Delete the previous timer-queue timer in the main thread, just prior to sched...
authorsfalken@apple.com <sfalken@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2008 18:10:52 +0000 (18:10 +0000)
committersfalken@apple.com <sfalken@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2008 18:10:52 +0000 (18:10 +0000)
        The code previously called DeleteTimerQueueTimer in the timer callback proc.

        The new technique simplifies the code, since we now create and delete timers on the
        same thread, and don't access the timer queue or timer handles in the callback.
        This allows us to remove some mutex use, and more importantly, it solves a race
        condition that was occuring between ChangeTimerQueueTimer and DeleteTimerQueueTimer.

        Since the timer callback isn't passed the timer handle, we were retrieving that handle
        via a global. If the timer callback code was entered, but then a new timer was immediately
        scheduled (prior to the callback acquiring the mutex and calling DeleteTimerQueueTimer),
        there was a small window where the timer could be re-scheduled via ChangeTimerQueueTimer
        and then immediately deleted once the already running callback acquired the mutex and
        then called DeleteTimerQueueTimer. This resulted in the newly scheduled timer never firing.

        Reviewed by Oliver Hunt.

        * platform/win/SharedTimerWin.cpp:
        (WebCore::queueTimerProc): Don't delete the timer in the callback.
        (WebCore::setSharedTimerFireTime): Always delete and create the timer instead of using ChangeTimerQueueTimer.
        (WebCore::stopSharedTimer): Call DeleteTimerQueueTimer directly.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39247 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/platform/win/SharedTimerWin.cpp

index ccb1583..b69c64f 100644 (file)
@@ -1,3 +1,27 @@
+2008-12-11  Steve Falkenburg  <sfalken@apple.com>
+
+        Delete the previous timer-queue timer in the main thread, just prior to scheduling a new timer.
+        The code previously called DeleteTimerQueueTimer in the timer callback proc.
+        
+        The new technique simplifies the code, since we now create and delete timers on the
+        same thread, and don't access the timer queue or timer handles in the callback.
+        This allows us to remove some mutex use, and more importantly, it solves a race
+        condition that was occuring between ChangeTimerQueueTimer and DeleteTimerQueueTimer.
+        
+        Since the timer callback isn't passed the timer handle, we were retrieving that handle
+        via a global. If the timer callback code was entered, but then a new timer was immediately
+        scheduled (prior to the callback acquiring the mutex and calling DeleteTimerQueueTimer),
+        there was a small window where the timer could be re-scheduled via ChangeTimerQueueTimer
+        and then immediately deleted once the already running callback acquired the mutex and
+        then called DeleteTimerQueueTimer. This resulted in the newly scheduled timer never firing.
+        
+        Reviewed by Oliver Hunt.
+
+        * platform/win/SharedTimerWin.cpp:
+        (WebCore::queueTimerProc): Don't delete the timer in the callback.
+        (WebCore::setSharedTimerFireTime): Always delete and create the timer instead of using ChangeTimerQueueTimer.
+        (WebCore::stopSharedTimer): Call DeleteTimerQueueTimer directly.
+
 2008-12-12  Kai BrĂ¼ning  <kai@granus.net>
 
         Reviewed and tweaked by Darin Adler.
index b611659..bafccd2 100644 (file)
 #include <windows.h>
 #include <mmsystem.h>
 
+#if PLATFORM(WIN)
+#include "PluginView.h"
+#endif
+
 // These aren't in winuser.h with the MSVS 2003 Platform SDK, 
 // so use default values in that case.
 #ifndef USER_TIMER_MINIMUM
 #define QS_RAWINPUT         0x0400
 #endif
 
-#if PLATFORM(WIN)
-#include "PluginView.h"
-#endif
-
 namespace WebCore {
 
 static UINT timerID;
@@ -68,7 +68,6 @@ static HWND timerWindowHandle = 0;
 static UINT timerFiredMessage = 0;
 static HANDLE timerQueue;
 static HANDLE timer;
-static Mutex timerMutex;
 static bool highResTimerActive;
 static bool processingCustomTimerMessage = false;
 static LONG pendingTimers;
@@ -139,17 +138,8 @@ void setSharedTimerFiredFunction(void (*f)())
     sharedTimerFiredFunction = f;
 }
 
-static void clearTimer()
-{
-    MutexLocker locker(timerMutex);
-    if (timerQueue && timer)
-        DeleteTimerQueueTimer(timerQueue, timer, 0);
-    timer = 0;
-}
-
 static void NTAPI queueTimerProc(PVOID, BOOLEAN)
 {
-    clearTimer();
     if (InterlockedIncrement(&pendingTimers) == 1)
         PostMessage(timerWindowHandle, timerFiredMessage, 0, 0);
 }
@@ -196,11 +186,9 @@ void setSharedTimerFireTime(double fireTime)
             // Otherwise, delay the PostMessage via a CreateTimerQueueTimer
             if (!timerQueue)
                 timerQueue = CreateTimerQueue();
-            MutexLocker locker(timerMutex);
             if (timer)
-                timerSet = ChangeTimerQueueTimer(timerQueue, timer, intervalInMS, 0);
-            else
-                timerSet = CreateTimerQueueTimer(&timer, timerQueue, queueTimerProc, 0, intervalInMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
+                DeleteTimerQueueTimer(timerQueue, timer, 0);
+            timerSet = CreateTimerQueueTimer(&timer, timerQueue, queueTimerProc, 0, intervalInMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
         }
     }
 
@@ -209,13 +197,19 @@ void setSharedTimerFireTime(double fireTime)
             KillTimer(timerWindowHandle, timerID);
             timerID = 0;
         }
-    } else
+    } else {
         timerID = SetTimer(timerWindowHandle, sharedTimerID, intervalInMS, 0);
+        timer = 0;
+    }
 }
 
 void stopSharedTimer()
 {
-    clearTimer();
+    if (timerQueue && timer) {
+        DeleteTimerQueueTimer(timerQueue, timer, 0);
+        timer = 0;
+    }
+
     if (timerID) {
         KillTimer(timerWindowHandle, timerID);
         timerID = 0;