[Libreoffice-commits] core.git: cppu/source

[Stephan Bergmann (via logerrit)]

 cppu/source/threadpool/threadpool.cxx |    8 ++++++--
 cppu/source/threadpool/threadpool.hxx |    3 ++-
 2 files changed, 8 insertions(+), 3 deletions(-)

New commits:
commit ad0779ed5ed14ab71cbf4247351827983f6393f7
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jun 26 07:58:40 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Fri Jun 26 10:43:40 2020 +0200

    Handle uno_threadpool_dispose in parallel with uno_threadpool_putJob
    
    While tracking down the issue discussed in the commit message of
    78dc7d982b65c1843a288b80da83f8766e85d0cf "Remove a potentially already enqueued
    response when a bridge is disposed", it occurred to me that there should be a
    race in those
    
      uno_threadpool_putJob(
        bridge_->getThreadPool(), ...);
    
    calls in binaryurp/source/reader.cxx, when the bridge gets disposed (through
    some other thread) between the time the bridge_->getThreadPool() call checks for
    the bridge being disposed (in which case it would throw a DisposedException) and
    the actual uno_threadpool_putJob call.
    
    I tried to catch that with a previous incarnation of this change
    (<https://gerrit.libreoffice.org/c/core/+/96120/1> "Jenkins Slides Through the
    Tiny Window"), but couldn't---presumably because this race would be very rare
    after all, and the issue I was chasing turned out to be caused by something
    different anyway.  Nevertheless, I wanted to address this potential race now.
    
    We can only reliably check for disposed'ness after having locked ThreadPool's
    m_mutex in uno_threadpool_putJob -> ThreadPool::addJob, but at which time we can
    no longer indicate this condition to the caller---uno_threapool_putJob is part
    of the stable URE interface, has a void return type, and should not throw any
    exceptions as it is a C function.  However, if the bridge gets disposed, any
    threads that would wait for this job (in cppu_threadpool::JobQueue::enter,
    either from cppu_threadpool::ORequestThread::run waiting to process new incoming
    calls, or from a bridge's call to uno_threadpool_enter waiting for a respose to
    an outgoing call) should already learn about the bridge being disposed by
    falling out of cppu_threadpool::JobQueue::enter with a null return value.  So it
    should be OK if uno_threadpool_putJob silently discards the job in that case.
    
    Change-Id: I36fe996436f55a93d84d66cc0b164e2e45a37e81
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96120
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/cppu/source/threadpool/threadpool.cxx b/cppu/source/threadpool/threadpool.cxx
index a9608fef3e1b..c5783dc19989 100644
--- a/cppu/source/threadpool/threadpool.cxx
+++ b/cppu/source/threadpool/threadpool.cxx
@@ -234,12 +234,16 @@ namespace cppu_threadpool
         const ByteSequence &aThreadId ,
         bool bAsynchron,
         void *pThreadSpecificData,
-        RequestFun * doRequest )
+        RequestFun * doRequest,
+        void const * disposeId )
     {
         bool bCreateThread = false;
         JobQueue *pQueue = nullptr;
         {
             MutexGuard guard( m_mutex );
+            if (m_DisposedCallerAdmin->isDisposed(disposeId)) {
+                return true;
+            }
 
             ThreadIdHashMap::iterator ii = m_mapQueue.find( aThreadId );
 
@@ -434,7 +438,7 @@ uno_threadpool_putJob(
     void ( SAL_CALL * doRequest ) ( void *pThreadSpecificData ),
     sal_Bool bIsOneway ) SAL_THROW_EXTERN_C()
 {
-    if (!getThreadPool(hPool)->addJob( pThreadId, bIsOneway, pJob ,doRequest ))
+    if (!getThreadPool(hPool)->addJob( pThreadId, bIsOneway, pJob ,doRequest, hPool ))
     {
         SAL_WARN(
             "cppu.threadpool",
diff --git a/cppu/source/threadpool/threadpool.hxx b/cppu/source/threadpool/threadpool.hxx
index c86e6575bb66..f473e2519348 100644
--- a/cppu/source/threadpool/threadpool.hxx
+++ b/cppu/source/threadpool/threadpool.hxx
@@ -126,7 +126,8 @@ namespace cppu_threadpool {
         bool addJob( const ::rtl::ByteSequence &aThreadId,
                      bool bAsynchron,
                      void *pThreadSpecificData,
-                     RequestFun * doRequest );
+                     RequestFun * doRequest,
+                     void const * disposeId );
 
         void prepare( const ::rtl::ByteSequence &aThreadId );
         void * enter( const ::rtl::ByteSequence &aThreadId, void const * nDisposeId );