summaryrefslogtreecommitdiff
path: root/cppu/source
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2020-06-16 08:25:36 +0200
committerStephan Bergmann <sbergman@redhat.com>2020-06-16 10:02:24 +0200
commita0bc388b38859ab67067340f836f26fa8f850cbf (patch)
tree722d34d5c0fe45693c2d5631890f9d0774bc1971 /cppu/source
parentd43e03e76e0bcbf1222584c1892d8377b8e09a61 (diff)
Use the saner std::condition_variable semantics for JobQueue::m_cndWait
This avoids the need for the tricky osl::Condition::reset calls. For example, the first one (in the "disposed !" case) had been added with 2a3ed89284890615ad4bce57be55ba558351cde1 "sb132: #i112448# proper clean up in JobQueue::enter (patch by olistraub)" as > if( 0 == m_lstCallstack.front() ) > { > // disposed ! > + if( m_lstJob.empty() ) > + { > + osl_resetCondition( m_cndWait ); > + } > break; > } > and then change to > if( 0 == m_lstCallstack.front() ) > { > // disposed ! > - if( m_lstJob.empty() ) > + if( m_lstJob.empty() > + && (m_lstCallstack.empty() > + || m_lstCallstack.front() != 0) ) > { > osl_resetCondition( m_cndWait ); > } with cba3ac1eab7acaf8e6efd7a00eee7c5e969fc49b "Avoid deadlocks when disposing recursive JobQueue::enter", which prevented the reset from ever hapening (because m_lstCallstack.front() cannot both be zero and non-zero here at the same time). The std::condition_variable semantics nicely avoid any reasoning whether or not a reset would be necessary here. Change-Id: Ic9b57b836bb6679829f4aa3b68679256726acf60 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96406 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'cppu/source')
-rw-r--r--cppu/source/threadpool/jobqueue.cxx76
-rw-r--r--cppu/source/threadpool/jobqueue.hxx12
-rw-r--r--cppu/source/threadpool/threadpool.hxx2
3 files changed, 39 insertions, 51 deletions
diff --git a/cppu/source/threadpool/jobqueue.cxx b/cppu/source/threadpool/jobqueue.cxx
index 748bc06a422e..1be424024d4b 100644
--- a/cppu/source/threadpool/jobqueue.cxx
+++ b/cppu/source/threadpool/jobqueue.cxx
@@ -17,12 +17,12 @@
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
*/
-#include "jobqueue.hxx"
-#include "threadpool.hxx"
+#include <sal/config.h>
-#include <osl/diagnose.h>
+#include <cassert>
-using namespace ::osl;
+#include "jobqueue.hxx"
+#include "threadpool.hxx"
namespace cppu_threadpool {
@@ -35,12 +35,12 @@ namespace cppu_threadpool {
void JobQueue::add( void *pThreadSpecificData, RequestFun * doRequest )
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
Job job = { pThreadSpecificData , doRequest };
m_lstJob.push_back( job );
if( ! m_bSuspended )
{
- m_cndWait.set();
+ m_cndWait.notify_all();
}
m_nToDo ++;
}
@@ -50,7 +50,7 @@ namespace cppu_threadpool {
void *pReturn = nullptr;
{
// synchronize with the dispose calls
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
if( m_DisposedCallerAdmin->isDisposed( nDisposeId ) )
{
return nullptr;
@@ -61,21 +61,16 @@ namespace cppu_threadpool {
while( true )
{
- if( bReturnWhenNoJob )
+ struct Job job={nullptr,nullptr};
{
- MutexGuard guard( m_mutex );
- if( m_lstJob.empty() )
+ std::unique_lock guard( m_mutex );
+
+ while (m_bSuspended
+ || (m_lstCallstack.front() != nullptr && !bReturnWhenNoJob
+ && m_lstJob.empty()))
{
- break;
+ m_cndWait.wait(guard);
}
- }
-
- m_cndWait.wait();
-
- struct Job job={nullptr,nullptr};
- {
- // synchronize with add and dispose calls
- MutexGuard guard( m_mutex );
if( nullptr == m_lstCallstack.front() )
{
@@ -87,38 +82,29 @@ namespace cppu_threadpool {
// response here:
m_lstJob.pop_front();
}
- if( m_lstJob.empty()
- && (m_lstCallstack.empty()
- || m_lstCallstack.front() != nullptr) )
- {
- m_cndWait.reset();
- }
break;
}
- OSL_ASSERT( ! m_lstJob.empty() );
- if( ! m_lstJob.empty() )
- {
- job = m_lstJob.front();
- m_lstJob.pop_front();
- }
- if( m_lstJob.empty()
- && (m_lstCallstack.empty() || m_lstCallstack.front() != nullptr) )
+ if( m_lstJob.empty() )
{
- m_cndWait.reset();
+ assert(bReturnWhenNoJob);
+ break;
}
+
+ job = m_lstJob.front();
+ m_lstJob.pop_front();
}
if( job.doRequest )
{
job.doRequest( job.pThreadSpecificData );
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
m_nToDo --;
}
else
{
pReturn = job.pThreadSpecificData;
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
m_nToDo --;
break;
}
@@ -126,7 +112,7 @@ namespace cppu_threadpool {
{
// synchronize with the dispose calls
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
m_lstCallstack.pop_front();
}
@@ -135,7 +121,7 @@ namespace cppu_threadpool {
void JobQueue::dispose( void const * nDisposeId )
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
for( auto& rId : m_lstCallstack )
{
if( rId == nDisposeId )
@@ -147,41 +133,41 @@ namespace cppu_threadpool {
if( !m_lstCallstack.empty() && ! m_lstCallstack.front() )
{
// The thread is waiting for a disposed pCallerId, let it go
- m_cndWait.set();
+ m_cndWait.notify_all();
}
}
void JobQueue::suspend()
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
m_bSuspended = true;
}
void JobQueue::resume()
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
m_bSuspended = false;
if( ! m_lstJob.empty() )
{
- m_cndWait.set();
+ m_cndWait.notify_all();
}
}
bool JobQueue::isEmpty() const
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
return m_lstJob.empty();
}
bool JobQueue::isCallstackEmpty() const
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
return m_lstCallstack.empty();
}
bool JobQueue::isBusy() const
{
- MutexGuard guard( m_mutex );
+ std::scoped_lock guard( m_mutex );
return m_nToDo > 0;
}
diff --git a/cppu/source/threadpool/jobqueue.hxx b/cppu/source/threadpool/jobqueue.hxx
index a0ccf5430385..5a5c80479d2b 100644
--- a/cppu/source/threadpool/jobqueue.hxx
+++ b/cppu/source/threadpool/jobqueue.hxx
@@ -20,12 +20,14 @@
#ifndef INCLUDED_CPPU_SOURCE_THREADPOOL_JOBQUEUE_HXX
#define INCLUDED_CPPU_SOURCE_THREADPOOL_JOBQUEUE_HXX
+#include <sal/config.h>
+
+#include <condition_variable>
#include <deque>
#include <memory>
-#include <sal/types.h>
+#include <mutex>
-#include <osl/conditn.hxx>
-#include <osl/mutex.hxx>
+#include <sal/types.h>
namespace cppu_threadpool
{
@@ -58,12 +60,12 @@ namespace cppu_threadpool
bool isBusy() const;
private:
- mutable ::osl::Mutex m_mutex;
+ mutable std::mutex m_mutex;
std::deque < struct Job > m_lstJob;
std::deque<void const *> m_lstCallstack;
sal_Int32 m_nToDo;
bool m_bSuspended;
- osl::Condition m_cndWait;
+ std::condition_variable m_cndWait;
DisposedCallerAdminHolder m_DisposedCallerAdmin;
};
}
diff --git a/cppu/source/threadpool/threadpool.hxx b/cppu/source/threadpool/threadpool.hxx
index acedbf628a03..c86e6575bb66 100644
--- a/cppu/source/threadpool/threadpool.hxx
+++ b/cppu/source/threadpool/threadpool.hxx
@@ -24,7 +24,7 @@
#include <unordered_map>
#include <osl/conditn.hxx>
-
+#include <osl/mutex.hxx>
#include <rtl/byteseq.hxx>
#include <rtl/ref.hxx>
#include <salhelper/simplereferenceobject.hxx>