[Libreoffice-commits] core.git: cppu/source
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Tue Jun 16 08:02:59 UTC 2020
cppu/source/threadpool/jobqueue.cxx | 76 +++++++++++++---------------------
cppu/source/threadpool/jobqueue.hxx | 12 +++--
cppu/source/threadpool/threadpool.hxx | 2
3 files changed, 39 insertions(+), 51 deletions(-)
New commits:
commit a0bc388b38859ab67067340f836f26fa8f850cbf
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Jun 16 08:25:36 2020 +0200
Commit: Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Jun 16 10:02:24 2020 +0200
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 at redhat.com>
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>
More information about the Libreoffice-commits
mailing list