[Libreoffice-commits] core.git: vcl/inc vcl/win
Armin Le Grand
Armin.Le.Grand at cib.de
Tue Apr 12 08:54:48 UTC 2016
vcl/inc/win/saldata.hxx | 2
vcl/inc/win/salinst.h | 8 ---
vcl/win/app/salinst.cxx | 117 +++++++++++++-----------------------------------
3 files changed, 34 insertions(+), 93 deletions(-)
New commits:
commit 9c9970952b0adec4a8c6de9a4cd54d0980cd47ec
Author: Armin Le Grand <Armin.Le.Grand at cib.de>
Date: Fri Apr 8 15:14:43 2016 +0200
tdf#96887 enhance SolarMutex AcquireWithWait for Windows
Currently the Windows-specific method ImplSalYieldMutexAcquireWithWait()
uses a messaging mechanism to learn about the SolarMutex being free again.
This is not reliable when the MessageQueue overflows (MS allows 10000
messages per queue). It is more safe to use MsgWaitForMultipleObjects.
This also allows to not only wait for the SolarMutex to be freed, but
also to detect when SendMessage() is used which needs to lead to a
reschedule to not block current Window handling.
Change-Id: Id317dda62aaa1fe7677d8d28929e6936e5a22705
Reviewed-on: https://gerrit.libreoffice.org/23921
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Armin Le Grand <Armin.Le.Grand at cib.de>
diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx
index ed8a2fd..66b8a7f 100644
--- a/vcl/inc/win/saldata.hxx
+++ b/vcl/inc/win/saldata.hxx
@@ -214,8 +214,6 @@ int ImplSalWICompareAscii( const wchar_t* pStr1, const char* pStr2 );
// wParam == bWait; lParam == 0
#define SAL_MSG_THREADYIELD (WM_USER+111)
-// wParam == 0; lParam == 0
-#define SAL_MSG_RELEASEWAITYIELD (WM_USER+112)
// wParam == 0; lParam == nMS
#define SAL_MSG_STARTTIMER (WM_USER+113)
// wParam == nFrameStyle; lParam == pParent; lResult == pFrame
diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h
index b6408ea..fe2c341 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -33,14 +33,6 @@ public:
HWND mhComWnd;
/// The Yield mutex ensures that only one thread calls into VCL
SalYieldMutex* mpSalYieldMutex;
- /// The Wait mutex ensures increment of mnYieldWaitCount and acquisition
- /// or release of mpSalYieldMutex is atomic
- osl::Mutex* mpSalWaitMutex;
- /// count main thread's pending ImplSalYieldMutexAcquireWithWait() calls
- /// (it's not clear to me if this will be > 1 in practice; it would be
- /// possible if main thread's handling of SAL_MSG_* sent by other threads
- /// via SendMessage() ends up calling ImplSalYieldMutexAcquireWithWait())
- sal_uInt16 mnYieldWaitCount;
public:
WinSalInstance();
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 1e37ece..a44e930 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -109,9 +109,9 @@ LRESULT CALLBACK SalComWndProcW( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lPa
class SalYieldMutex : public comphelper::SolarMutex
{
- osl::Mutex m_mutex;
-
-public: // for ImplSalYield()
+public: // for ImplSalYield() and ImplSalYieldMutexAcquireWithWait()
+ osl::Mutex m_mutex;
+ osl::Condition m_condition; /// for MsgWaitForMultipleObjects()
WinSalInstance* mpInstData;
sal_uLong mnCount;
DWORD mnThreadId;
@@ -147,10 +147,11 @@ void SalYieldMutex::release()
m_mutex.release();
else
{
- SalData* pSalData = GetSalData();
- if ( pSalData->mnAppThreadId != nThreadId )
+ bool isRelease(1 == mnCount);
+ if ( isRelease )
{
- if ( mnCount == 1 )
+ SalData* pSalData = GetSalData();
+ if ( pSalData->mnAppThreadId != nThreadId )
{
OpenGLContext::prepareForYield();
@@ -158,31 +159,21 @@ void SalYieldMutex::release()
// Java clients doesn't come in the right order
GdiFlush();
- // lock here to ensure that the test of mnYieldWaitCount and
- // m_mutex.release() is atomic
- mpInstData->mpSalWaitMutex->acquire();
- if ( mpInstData->mnYieldWaitCount )
- PostMessageW( mpInstData->mhComWnd, SAL_MSG_RELEASEWAITYIELD, 0, 0 );
mnThreadId = 0;
- mnCount--;
- m_mutex.release();
- mpInstData->mpSalWaitMutex->release();
}
else
{
- mnCount--;
- m_mutex.release();
- }
- }
- else
- {
- if ( mnCount == 1 )
- {
mnThreadId = 0;
OpenGLContext::prepareForYield();
}
- mnCount--;
- m_mutex.release();
+ }
+
+ mnCount--;
+ m_mutex.release();
+
+ if ( isRelease )
+ { // do this *after* release
+ m_condition.set(); // wake up ImplSalYieldMutexAcquireWithWait()
}
}
}
@@ -216,57 +207,33 @@ void ImplSalYieldMutexAcquireWithWait()
if ( !pInst )
return;
- // If this is the main thread, then we must wait with GetMessage(),
- // because if we don't reschedule, then we create deadlocks if a Window's
- // create/destroy is called via SendMessage() from another thread.
- // If this is not the main thread, call acquire directly.
DWORD nThreadId = GetCurrentThreadId();
SalData* pSalData = GetSalData();
+
if ( pSalData->mnAppThreadId == nThreadId )
{
- // wait till we get the Mutex
- bool bAcquire = false;
- do
+ // tdf#96887 If this is the main thread, then we must wait for two things:
+ // - the mpSalYieldMutex being freed
+ // - SendMessage() being triggered
+ // This can nicely be done using MsgWaitForMultipleObjects. The 2nd one is
+ // needed because if we don't reschedule, then we create deadlocks if a
+ // Window's create/destroy is called via SendMessage() from another thread.
+ // Have a look at the osl_waitCondition implementation for more info.
+ pInst->mpSalYieldMutex->m_condition.reset();
+ while (!pInst->mpSalYieldMutex->tryToAcquire())
{
- if ( pInst->mpSalYieldMutex->tryToAcquire() )
- bAcquire = true;
- else
- {
- pInst->mpSalWaitMutex->acquire();
- if ( pInst->mpSalYieldMutex->tryToAcquire() )
- {
- bAcquire = true;
- pInst->mpSalWaitMutex->release();
- }
- else
- {
- // other threads must not observe mnYieldWaitCount == 0
- // while main thread is blocked in GetMessage()
- pInst->mnYieldWaitCount++;
- pInst->mpSalWaitMutex->release();
- MSG aTmpMsg;
- // this call exists because it dispatches SendMessage() msg!
- GetMessageW( &aTmpMsg, pInst->mhComWnd, SAL_MSG_RELEASEWAITYIELD, SAL_MSG_RELEASEWAITYIELD );
- // it is possible that another thread acquires and releases
- // mpSalYieldMutex after the GetMessage call returns,
- // observes mnYieldWaitCount != 0 and sends an extra
- // SAL_MSG_RELEASEWAITYIELD - but that appears unproblematic
- // as it will just cause the next Yield to do an extra
- // iteration of the while loop here
- pInst->mnYieldWaitCount--;
- if ( pInst->mnYieldWaitCount )
- {
- // repeat the message so that the next instance of this
- // function further up the call stack is unblocked too
- PostMessageW( pInst->mhComWnd, SAL_MSG_RELEASEWAITYIELD, 0, 0 );
- }
- }
- }
+ // wait for SalYieldMutex::release() to set the condition
+ osl::Condition::Result res = pInst->mpSalYieldMutex->m_condition.wait();
+ assert(osl::Condition::Result::result_ok == res);
+ // reset condition *before* acquiring!
+ pInst->mpSalYieldMutex->m_condition.reset();
}
- while ( !bAcquire );
}
else
+ {
+ // If this is not the main thread, call acquire directly.
pInst->mpSalYieldMutex->acquire();
+ }
}
bool ImplSalYieldMutexTryToAcquire()
@@ -576,8 +543,6 @@ WinSalInstance::WinSalInstance()
{
mhComWnd = 0;
mpSalYieldMutex = new SalYieldMutex( this );
- mpSalWaitMutex = new osl::Mutex;
- mnYieldWaitCount = 0;
mpSalYieldMutex->acquire();
::comphelper::SolarMutex::setSolarMutex( mpSalYieldMutex );
}
@@ -587,7 +552,6 @@ WinSalInstance::~WinSalInstance()
::comphelper::SolarMutex::setSolarMutex( 0 );
mpSalYieldMutex->release();
delete mpSalYieldMutex;
- delete mpSalWaitMutex;
DestroyWindow( mhComWnd );
}
@@ -706,7 +670,7 @@ SalYieldResult WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents,
return eDidWork;
}
-LRESULT CALLBACK SalComWndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lParam, int& rDef )
+LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, int& rDef )
{
LRESULT nRet = 0;
@@ -716,19 +680,6 @@ LRESULT CALLBACK SalComWndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lPar
ImplSalYield( (bool)wParam, (bool)lParam );
rDef = FALSE;
break;
- // If we get this message, because another GetMessage() call
- // has received this message, we must post this message to
- // us again, because in the other case we wait forever.
- case SAL_MSG_RELEASEWAITYIELD:
- {
- WinSalInstance* pInst = GetSalData()->mpFirstInstance;
- // this test does not need mpSalWaitMutex locked because
- // it can only happen on the main thread
- if ( pInst && pInst->mnYieldWaitCount )
- PostMessageW( hWnd, SAL_MSG_RELEASEWAITYIELD, wParam, lParam );
- }
- rDef = FALSE;
- break;
case SAL_MSG_STARTTIMER:
ImplSalStartTimer( (sal_uLong) lParam, FALSE );
rDef = FALSE;
More information about the Libreoffice-commits
mailing list