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>

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>