Bug 92009 – Don’t use currentTime() in places where it’s inappropriate

• Status: NEW
• Component: WebKit Misc.

[Yong Li]

WebKit condition variable should use monotonic clock, and its consumers should use monotonic time as absolution time. Otherwise, if system date is changed back for a month, some threads could sleep too long...

It seems to me only WebKit is using condition variables only in 2 cases:

1. BlockFreeingThread
2. Timers in Web Worker threads.

The latter uses monotonic time as absolute wait time (see Timer.cpp), but the former uses currentTime.

The solution in my mind is:

1. Make condition variable use absolute time
2. Let BlockFreeingThread uses monotonicallyIncreasingTime()

[Yong Li]

"Make condition variable use absolute time"

should be "Make condition variable use monotonic clock" with pthread_condattr_setclock

[Yong Li]

Even though monotonicallyIncreasingTime() has been introduced for a while, WebKit is still using currentTime at so many places where monotonic time is expected.

It is too hard for one to fix all of these without breaking any port. For example, changing DOM event's timestamp to use monotonicallyIncreasingTime() could break plugins. Making ThreadCondition use monotonic clock will break WebKit2.

BTW, Web Worker's timer is using currentTime(), so it works well with ThreadCondition currently.

But the problem is still there: what if user adjusts system date to 1 month earlier?

Any idea how we can get this thing done smoothly?

[Darin Adler]

(In reply to comment #2)
> But the problem is still there: what if user adjusts system date to 1 month earlier?

A good way to make further use of this insight is to do some testing. We can find out how real these problems are that could happen when the user changes the computer time.

We are experts so we can see the bugs that could exist, but testing makes them more concrete and helps motivate us better than just architectural insight.

[Yong Li]

(In reply to comment #3)
> (In reply to comment #2)
> > But the problem is still there: what if user adjusts system date to 1 month earlier?
> 
> A good way to make further use of this insight is to do some testing. We can find out how real these problems are that could happen when the user changes the computer time.
> 
> We are experts so we can see the bugs that could exist, but testing makes them more concrete and helps motivate us better than just architectural insight.

I've seen changing system date back when Safari win32 is loading a page (like cnn.com, bgr.com) caused the page frozen (cannot be scrolled) for a while (>20 seconds). Not sure if it is related to this issue. Such kind of issues usually take developers quite some time to finally realize it is timer that doesn't work.

One way to show real problems might be running a script that keeps changing system date on a testbot at same time when it is running layout tests. But bugs like BlockFreeingThread never wakes up or takes very long to wake up are very hard to be detected.

[Antonio Gomes]

Adding [e-mail] so it can be duplicated against the proper bug.

[Darin Adler]

We don’t plan to wipe out currentTime entirely, do we? Retitling assuming that is correct.

[Yong Li]

(In reply to comment #6)
> We don’t plan to wipe out currentTime entirely, do we? Retitling assuming that is correct.

Initially, I thought the only need for currentTime was JS date object, and all other references should be changed to use monotonic time. This assumption was probably wrong. I like the new title.