[Libreoffice-commits] core.git: vcl/inc vcl/osx
Stephan Bergmann
sbergman at redhat.com
Thu Nov 9 07:11:09 UTC 2017
vcl/inc/osx/runinmain.hxx | 79 +++++++++++++++++++++++++++++-----------------
vcl/inc/osx/salinst.h | 4 ++
vcl/osx/salinst.cxx | 53 ++++++++++++++++++++----------
3 files changed, 89 insertions(+), 47 deletions(-)
New commits:
commit b5b78703b5d801c2a9a5e45126f8e87bf5ebd4b4
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Wed Nov 8 19:01:10 2017 +0100
Avoid races when using OSX_RUNINMAIN_MEMBERS
...so that e.g. main thread in SalYieldMutex::doAcquire could reset
m_aInMainTHreadCondition and then block waiting on it only after another thread
had set it. (Saw such a deadlock in some 'make check' CppunitTest.)
Change-Id: I5c676956a2bec6bf8f94d7dbeee64f100db39bd3
Reviewed-on: https://gerrit.libreoffice.org/44501
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/vcl/inc/osx/runinmain.hxx b/vcl/inc/osx/runinmain.hxx
index a57b7908214d..8f8559dac5c8 100644
--- a/vcl/inc/osx/runinmain.hxx
+++ b/vcl/inc/osx/runinmain.hxx
@@ -57,8 +57,11 @@ union RuninmainResult
};
#define OSX_RUNINMAIN_MEMBERS \
- osl::Condition m_aInMainCondition; \
- osl::Condition m_aResultCondition; \
+ std::mutex m_runInMainMutex; \
+ std::condition_variable m_aInMainCondition; \
+ std::condition_variable m_aResultCondition; \
+ bool m_wakeUpMain = false; \
+ bool m_resultReady = false; \
RuninmainBlock m_aCodeBlock; \
RuninmainResult m_aResult;
@@ -67,18 +70,24 @@ union RuninmainResult
{ \
DBG_TESTSOLARMUTEX(); \
SalYieldMutex *aMutex = static_cast<SalYieldMutex*>(instance->GetYieldMutex()); \
- assert( !aMutex->m_aCodeBlock ); \
- aMutex->m_aCodeBlock = Block_copy(^{ \
- command; \
- }); \
- aMutex->m_aResultCondition.reset(); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ assert( !aMutex->m_aCodeBlock ); \
+ aMutex->m_aCodeBlock = Block_copy(^{ \
+ command; \
+ }); \
+ aMutex->m_wakeUpMain = true; \
+ aMutex->m_aInMainCondition.notify_all(); \
+ } \
dispatch_async(dispatch_get_main_queue(),^{ \
ImplNSAppPostEvent( AquaSalInstance::YieldWakeupEvent, NO ); \
}); \
- aMutex->m_aInMainCondition.set(); \
- osl::Condition::Result res = aMutex->m_aResultCondition.wait(); \
- (void) res; \
- assert(osl::Condition::Result::result_ok == res); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ aMutex->m_aResultCondition.wait( \
+ g, [&aMutex]() { return aMutex->m_resultReady; }); \
+ aMutex->m_resultReady = false; \
+ } \
return; \
}
@@ -87,18 +96,24 @@ union RuninmainResult
{ \
DBG_TESTSOLARMUTEX(); \
SalYieldMutex *aMutex = static_cast<SalYieldMutex*>(instance->GetYieldMutex()); \
- assert( !aMutex->m_aCodeBlock ); \
- aMutex->m_aCodeBlock = Block_copy(^{ \
- aMutex->m_aResult.pointer = static_cast<void*>( command ); \
- }); \
- aMutex->m_aResultCondition.reset(); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ assert( !aMutex->m_aCodeBlock ); \
+ aMutex->m_aCodeBlock = Block_copy(^{ \
+ aMutex->m_aResult.pointer = static_cast<void*>( command ); \
+ }); \
+ aMutex->m_wakeUpMain = true; \
+ aMutex->m_aInMainCondition.notify_all(); \
+ } \
dispatch_async(dispatch_get_main_queue(),^{ \
ImplNSAppPostEvent( AquaSalInstance::YieldWakeupEvent, NO ); \
}); \
- aMutex->m_aInMainCondition.set(); \
- osl::Condition::Result res = aMutex->m_aResultCondition.wait(); \
- (void) res; \
- assert(osl::Condition::Result::result_ok == res); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ aMutex->m_aResultCondition.wait( \
+ g, [&aMutex]() { return aMutex->m_resultReady; }); \
+ aMutex->m_resultReady = false; \
+ } \
return static_cast<type>( aMutex->m_aResult.pointer ); \
}
@@ -107,18 +122,24 @@ union RuninmainResult
{ \
DBG_TESTSOLARMUTEX(); \
SalYieldMutex *aMutex = static_cast<SalYieldMutex*>(instance->GetYieldMutex()); \
- assert( !aMutex->m_aCodeBlock ); \
- aMutex->m_aCodeBlock = Block_copy(^{ \
- aMutex->m_aResult.member = command; \
- }); \
- aMutex->m_aResultCondition.reset(); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ assert( !aMutex->m_aCodeBlock ); \
+ aMutex->m_aCodeBlock = Block_copy(^{ \
+ aMutex->m_aResult.member = command; \
+ }); \
+ aMutex->m_wakeUpMain = true; \
+ aMutex->m_aInMainCondition.notify_all(); \
+ } \
dispatch_async(dispatch_get_main_queue(),^{ \
ImplNSAppPostEvent( AquaSalInstance::YieldWakeupEvent, NO ); \
}); \
- aMutex->m_aInMainCondition.set(); \
- osl::Condition::Result res = aMutex->m_aResultCondition.wait(); \
- (void) res; \
- assert(osl::Condition::Result::result_ok == res); \
+ { \
+ std::unique_lock<std::mutex> g(aMutex->m_runInMainMutex); \
+ aMutex->m_aResultCondition.wait( \
+ g, [&aMutex]() { return aMutex->m_resultReady; }); \
+ aMutex->m_resultReady = false; \
+ } \
return std::move( aMutex->m_aResult.member ); \
}
diff --git a/vcl/inc/osx/salinst.h b/vcl/inc/osx/salinst.h
index 95dd4db26c24..f96eefaa8950 100644
--- a/vcl/inc/osx/salinst.h
+++ b/vcl/inc/osx/salinst.h
@@ -21,7 +21,11 @@
#ifndef INCLUDED_VCL_INC_OSX_SALINST_H
#define INCLUDED_VCL_INC_OSX_SALINST_H
+#include <sal/config.h>
+
+#include <condition_variable>
#include <list>
+#include <mutex>
#include <comphelper/solarmutex.hxx>
#include <osl/conditn.hxx>
diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx
index f9254d873227..80b6fec66cc0 100644
--- a/vcl/osx/salinst.cxx
+++ b/vcl/osx/salinst.cxx
@@ -17,6 +17,12 @@
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
*/
+#include <sal/config.h>
+
+#include <condition_variable>
+#include <mutex>
+#include <utility>
+
#include <config_features.h>
#include <stdio.h>
@@ -277,24 +283,31 @@ void SalYieldMutex::doAcquire( sal_uInt32 nLockCount )
if ( pInst->mbNoYieldLock )
return;
do {
- m_aInMainCondition.reset();
- if ( m_aCodeBlock )
+ RuninmainBlock block = nullptr;
+ {
+ std::unique_lock<std::mutex> g(m_runInMainMutex);
+ if (m_aMutex.tryToAcquire()) {
+ assert(m_aCodeBlock == nullptr);
+ m_wakeUpMain = false;
+ break;
+ }
+ // wait for doRelease() or RUNINMAIN_* to set the condition
+ m_aInMainCondition.wait(g, [this]() { return m_wakeUpMain; });
+ m_wakeUpMain = false;
+ std::swap(block, m_aCodeBlock);
+ }
+ if ( block )
{
assert( !pInst->mbNoYieldLock );
pInst->mbNoYieldLock = true;
- m_aCodeBlock();
+ block();
pInst->mbNoYieldLock = false;
- Block_release( m_aCodeBlock );
- m_aCodeBlock = nullptr;
- m_aResultCondition.set();
+ Block_release( block );
+ std::unique_lock<std::mutex> g(m_runInMainMutex);
+ assert(!m_resultReady);
+ m_resultReady = true;
+ m_aResultCondition.notify_all();
}
- // reset condition *before* acquiring!
- if (m_aMutex.tryToAcquire())
- break;
- // wait for doRelease() or RUNINMAIN_* to set the condition
- osl::Condition::Result res = m_aInMainCondition.wait();
- (void) res;
- assert(osl::Condition::Result::result_ok == res);
}
while ( true );
}
@@ -311,11 +324,15 @@ sal_uInt32 SalYieldMutex::doRelease( const bool bUnlockAll )
AquaSalInstance *pInst = GetSalData()->mpInstance;
if ( pInst->mbNoYieldLock && pInst->IsMainThread() )
return 1;
- sal_uInt32 nCount = comphelper::GenericSolarMutex::doRelease( bUnlockAll );
-
- if ( 0 == m_nCount && !pInst->IsMainThread() )
- m_aInMainCondition.set();
-
+ sal_uInt32 nCount;
+ {
+ std::unique_lock<std::mutex> g(m_runInMainMutex);
+ nCount = comphelper::GenericSolarMutex::doRelease( bUnlockAll );
+ if ( 0 == m_nCount && !pInst->IsMainThread() ) {
+ m_wakeUpMain = true;
+ m_aInMainCondition.notify_all();
+ }
+ }
return nCount;
}
More information about the Libreoffice-commits
mailing list