Bug 141554 – TimerBase::m_heapInsertionOrder calculation is racy

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Alexey Proskuryakov]

TimerBase is used from multiple threads, and insertion order can get incorrect if the threads start overwriting the value.

[Alexey Proskuryakov]

Created attachment 246510 [details]
proposed fix

Fingers crossed that this won't trigger a static destructor warning anywhere (it did build fine locally for me).

I'm not sure if it is legitimate to use uint64_t here, that's not one of the types listed for specializations at <http://en.cppreference.com/w/cpp/atomic/atomic>.

[Anders Carlsson]

Comment on attachment 246510 [details]
proposed fix

Just use uint64_t instead of uint_fast64_t.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r180058>.

Anders also noted that the static object doesn't need to be static, it can be per-thread (perhaps part of ThreadTimers). Looking at the code, it seems like it could use a bit of refactoring for which functions are in TimerBase, and which are in ThreadTimers.

I noticed that the counter doesn't actually have a problem with overflow, so I kept it as unsigned.

     // We need to look at the difference of the insertion orders instead of comparing the two 
     // outright in case of overflow.