[Libreoffice-commits] core.git: Branch 'feature/cib_contract57' - vcl/inc vcl/win

Michael Stahl mstahl at redhat.com
Fri Jun 2 10:43:36 UTC 2017


 vcl/inc/win/saldata.hxx         |    2 ++
 vcl/inc/win/salinst.h           |    1 +
 vcl/win/source/app/salinst.cxx  |    4 ++++
 vcl/win/source/app/saltimer.cxx |   38 ++++++++++++++++++++++++--------------
 4 files changed, 31 insertions(+), 14 deletions(-)

New commits:
commit d4df751f89c5372eaa240769e5d275e91d2acc93
Author: Michael Stahl <mstahl at redhat.com>
Date:   Mon Apr 11 23:49:12 2016 +0200

    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.
    
    Reviewed-on: https://gerrit.libreoffice.org/24024
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>
    Reviewed-on: https://gerrit.libreoffice.org/26259
    Reviewed-by: Tor Lillqvist <tml at collabora.com>
    Tested-by: Tor Lillqvist <tml at collabora.com>
    (cherry picked from commit 4d8924806102681f8d807fd82cdd3db786ddfac0)
    
    Change-Id: I816bd3fa5fbfbea4f26be8ff680a1c916618d3f9

diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx
index a715e3a6aef1..9b071525125d 100644
--- a/vcl/inc/win/saldata.hxx
+++ b/vcl/inc/win/saldata.hxx
@@ -279,6 +279,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 769281ce7ce1..dc077ad786e2 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -77,6 +77,7 @@ SalFrame* ImplSalCreateFrame( WinSalInstance* pInst, HWND hWndParent, sal_uIntPt
 SalObject* ImplSalCreateObject( WinSalInstance* pInst, WinSalFrame* pParent );
 HWND ImplSalReCreateHWND( HWND hWndParent, HWND oldhWnd, bool bAsChild );
 void ImplSalStartTimer( sal_uIntPtr nMS, bool bMutex = false );
+void ImplSalStopTimer();
 void ImplSalPrinterAbortJobAsync( HDC hPrnDC );
 
 #endif // INCLUDED_VCL_INC_WIN_SALINST_H
diff --git a/vcl/win/source/app/salinst.cxx b/vcl/win/source/app/salinst.cxx
index 9e238bf02429..c4dab76e89b3 100644
--- a/vcl/win/source/app/salinst.cxx
+++ b/vcl/win/source/app/salinst.cxx
@@ -680,6 +680,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, (sal_uLong)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


More information about the Libreoffice-commits mailing list