tdf#96887 vcl: stop using periodic timers on WNT

Every time the periodic timer fires, it does a PostMessage() to the
main thread.  The main thread will only process the first message and
discard the rest anyway, but with a short enough timer and other
threads hogging the SolarMutex it's possible that the message queue
overflows and other PostMessage calls fail with ERROR_NOT_ENOUGH_QUOTA.

Try to avoid the problem by having the WinSalTimer always be a one-shot
timer; when it fires and the main thread processes the posted message,
it is restarted with the new due time.

This requires creating a new TimerQueueTimer because
ChangeTimerQueueTimer only works on periodic timers.

Change-Id: I816bd3fa5fbfbea4f26be8ff680a1c916618d3f9
Reviewed-on: https://gerrit.libreoffice.org/24024
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Jan Holesovsky <kendy@collabora.com>
Reviewed-by: Michael Stahl <mstahl@redhat.com>

2 files changed, 28 insertions, 14 deletions

diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index dd687d813b2b..bd5b49e1d219 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -672,6 +672,10 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
             ImplSalStartTimer( (sal_uLong) lParam, FALSE );
             rDef = FALSE;
             break;
+        case SAL_MSG_STOPTIMER:
+            ImplSalStopTimer();
+            rDef = FALSE;
+            break;
         case SAL_MSG_CREATEFRAME:
             nRet = (LRESULT)ImplSalCreateFrame( GetSalData()->mpFirstInstance, (HWND)lParam, (SalFrameStyleFlags)wParam );
             rDef = FALSE;
diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx
index 30aa816745c1..ff230c2c2c61 100644
--- a/vcl/win/app/saltimer.cxx
+++ b/vcl/win/app/saltimer.cxx
@@ -34,12 +34,22 @@ void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired);
 // See http://msdn.microsoft.com/en-us/library/windows/desktop/ms687003%28v=vs.85%29.aspx
 // (and related pages) for details about the Timer Queues.
 
-void ImplSalStopTimer(SalData* pSalData)
+// in order to prevent concurrent execution of ImplSalStartTimer and double
+// deletion of timer (which is extremely likely, given that
+// INVALID_HANDLE_VALUE waits for the callback to run on the main thread),
+// this must run on the main thread too
+void ImplSalStopTimer()
 {
+    SalData *const pSalData = GetSalData();
     HANDLE hTimer = pSalData->mnTimerId;
-    pSalData->mnTimerId = 0;
-    DeleteTimerQueueTimer(NULL, hTimer, INVALID_HANDLE_VALUE);
+    if (hTimer)
+    {
+        pSalData->mnTimerId = 0; // reset so it doesn't restart
+        DeleteTimerQueueTimer(NULL, hTimer, INVALID_HANDLE_VALUE);
+        pSalData->mnNextTimerTime = 0;
+    }
     MSG aMsg;
+    // this needs to run on the main thread
     while (PeekMessageW(&aMsg, 0, SAL_MSG_TIMER_CALLBACK, SAL_MSG_TIMER_CALLBACK, PM_REMOVE))
     {
         // just remove all the SAL_MSG_TIMER_CALLBACKs
@@ -61,11 +71,13 @@ void ImplSalStartTimer( sal_uLong nMS, bool bMutex )
     if (nMS > MAX_SYSPERIOD)
         nMS = MAX_SYSPERIOD;
 
-    // change if it exists, create if not
+    // cannot change a one-shot timer, so delete it and create new one
     if (pSalData->mnTimerId)
-        ChangeTimerQueueTimer(NULL, pSalData->mnTimerId, nMS, nMS);
-    else
-        CreateTimerQueueTimer(&pSalData->mnTimerId, NULL, SalTimerProc, NULL, nMS, nMS, WT_EXECUTEINTIMERTHREAD);
+    {
+        DeleteTimerQueueTimer(NULL, pSalData->mnTimerId, INVALID_HANDLE_VALUE);
+        pSalData->mnTimerId = 0;
+    }
+    CreateTimerQueueTimer(&pSalData->mnTimerId, NULL, SalTimerProc, NULL, nMS, 0, WT_EXECUTEINTIMERTHREAD);
 
     pSalData->mnNextTimerTime = pSalData->mnLastEventTime + nMS;
 }
@@ -96,12 +108,8 @@ void WinSalTimer::Stop()
 {
     SalData* pSalData = GetSalData();
 
-    // If we have a timer, than
-    if ( pSalData->mnTimerId )
-    {
-        ImplSalStopTimer(pSalData);
-        pSalData->mnNextTimerTime = 0;
-    }
+    assert(pSalData->mpFirstInstance);
+    SendMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STOPTIMER, 0, 0);
 }
 
 /** This gets invoked from a Timer Queue thread.
@@ -165,9 +173,11 @@ void EmitTimerCallback()
         pSVData->mpSalTimer->CallCallback( idle );
         ImplSalYieldMutexRelease();
 
+        // Run the timer again if it was started before, and also
         // Run the timer in the correct time, if we started this
         // with a small timeout, because we didn't get the mutex
-        if (pSalData->mnTimerId && (pSalData->mnTimerMS != pSalData->mnTimerOrgMS))
+        // - but not if mnTimerId is 0, which is set by ImplSalStopTimer()
+        if (pSalData->mnTimerId)
             ImplSalStartTimer(pSalData->mnTimerOrgMS, false);
     }
     else