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>
Reviewed-on: https://gerrit.libreoffice.org/26257
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>

4 files changed, 31 insertions, 14 deletions

diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx
index 66b8a7fd7179..83e853f6b051 100644
--- a/vcl/inc/win/saldata.hxx
+++ b/vcl/inc/win/saldata.hxx
@@ -277,6 +277,8 @@ int ImplSalWICompareAscii( const wchar_t* pStr1, const char* pStr2 );
 
 // Call the Timer's callback from the main thread
 #define SAL_MSG_TIMER_CALLBACK      (WM_USER+162)
+// Stop the timer from the main thread; wParam = 0, lParam = 0
+#define SAL_MSG_STOPTIMER           (WM_USER+163)
 
 inline void SetWindowPtr( HWND hWnd, WinSalFrame* pThis )
 {
diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h
index fe2c3414593e..43c0313efa7d 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -83,6 +83,7 @@ SalFrame* ImplSalCreateFrame( WinSalInstance* pInst, HWND hWndParent, SalFrameSt
 SalObject* ImplSalCreateObject( WinSalInstance* pInst, WinSalFrame* pParent );
 HWND ImplSalReCreateHWND( HWND hWndParent, HWND oldhWnd, bool bAsChild );
 void ImplSalStartTimer( sal_uIntPtr nMS, bool bMutex = false );
+void ImplSalStopTimer();
 
 #endif // INCLUDED_VCL_INC_WIN_SALINST_H
 
diff --git a/vcl/win/source/app/salinst.cxx b/vcl/win/source/app/salinst.cxx
index c76fa44c1a8a..b229d4647431 100644
--- a/vcl/win/source/app/salinst.cxx
+++ b/vcl/win/source/app/salinst.cxx
@@ -689,6 +689,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/source/app/saltimer.cxx b/vcl/win/source/app/saltimer.cxx
index c6f04be986fe..641d2eb7de7b 100644
--- a/vcl/win/source/app/saltimer.cxx
+++ b/vcl/win/source/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;
 }
@@ -93,12 +105,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.
@@ -158,9 +166,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