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

[Stephan Bergmann (via logerrit)]

 cppu/source/threadpool/jobqueue.cxx |    7 +++++++
 1 file changed, 7 insertions(+)

New commits:
commit 78dc7d982b65c1843a288b80da83f8766e85d0cf
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Jun 15 13:22:58 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Jun 15 16:07:26 2020 +0200

    Remove a potentially already enqueued response when a bridge is disposed
    
    ...while a remote call is in progress.  Otherwise, if the same thread later
    makes another remote call (through another bridge), it would erroneously pair
    the repsonse for the disposed call with the second call.
    
    UITest_calc_demo started to fail that way occasionally, presumably after
    77445e201c45e5593761e8399c32f80eea2178a4 "Make Chart Creation Wizard async", but
    for reasons rather unrelated to the contents of that commit.  What apparently
    happened is that in one test's tearDown, the bridge between the Python and the
    soffice process happened to be disposed during the XDesktop::terminate call from
    Python's main thread, so that the response (of type BOOLEAN) remained in the
    JobQueue.  Then during the next test's setUp, XUnoUrlResolver::resolve from
    Python's main thread would internally make a remote queryInterface call for the
    initial StarOffice.ComponentContext object, which returns an ANY of either VOID
    or XInterface type.  But that call was then mis-matched with the leftover
    BOOLEAN response, causing failure.
    
    I was able to reproduce that reliably on Linux with a local hack of
    
    > diff --git a/cppu/source/threadpool/jobqueue.cxx b/cppu/source/threadpool/jobqueue.cxx
    > index 6c9324521f40..a87770bf8935 100644
    > --- a/cppu/source/threadpool/jobqueue.cxx
    > +++ b/cppu/source/threadpool/jobqueue.cxx
    > @@ -71,6 +71,7 @@ namespace cppu_threadpool {
    >              }
    >
    >              m_cndWait.wait();
    > +            timespec ms{0, 1000000}; nanosleep(&ms, nullptr);
    >
    >              struct Job job={nullptr,nullptr};
    >              {
    
    introducing a sleep, so that other threads have a higher chance to dispose the
    bridge (when the call being processed here is that XDesktop::dispose) after the
    successful wait() but before the response is processed.  UITest_calc_demo then
    failed with
    
    [...]
    > Execution time for create_chart.CalcChartUIDemo.test_activate_chart: 6.520
    > tearDown: calling terminate()...
    > caught while TearDown:
    >  Traceback (most recent call last):
    >   File "uitest/libreoffice/connection.py", line 127, in tearDown
    >     xDesktop.terminate()
    > libreoffice.connection.com.sun.star.lang.DisposedException: Binary URP bridge disposed during call binaryurp/source/bridge.cxx:611
    >
    > ok
    > test_cancel_immediately (create_chart.CalcChartUIDemo) ...
    [...]
    > warn:sal.osl.pipe:423851:425076:sal/osl/unx/pipe.cxx:442: recv() failed: ECONNRESET
    > warn:binaryurp:423851:425076:binaryurp/source/bridge.cxx:843: undisposed bridge "" in state 2, potential deadlock ahead
    [...]
    > ======================================================================
    > ERROR: test_cancel_immediately (create_chart.CalcChartUIDemo)
    > ----------------------------------------------------------------------
    > Traceback (most recent call last):
    >   File "uitest/uitest/framework.py", line 25, in setUp
    >     self.connection.setUp()
    >   File "uitest/libreoffice/connection.py", line 176, in setUp
    >     conn.setUp()
    >   File "uitest/libreoffice/connection.py", line 57, in setUp
    >     self.xContext = self.connect(socket)
    >   File "uitest/libreoffice/connection.py", line 107, in connect
    >     xContext = xUnoResolver.resolve(url)
    > uno.com.sun.star.uno.RuntimeException: initial object queryInterface for OID "StarOffice.ComponentContext" returned ANY of type boolean binaryurp/source/bridge.cxx:883
    [...]
    
    Change-Id: Icf9aadbb38e7aafffff844fe8e9ae99e165c1f33
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96326
    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 6c9324521f40..b4974fdc1724 100644
--- a/cppu/source/threadpool/jobqueue.cxx
+++ b/cppu/source/threadpool/jobqueue.cxx
@@ -80,6 +80,13 @@ namespace cppu_threadpool {
                 if( 0 == m_lstCallstack.front() )
                 {
                     // disposed !
+                    if (!m_lstJob.empty() && m_lstJob.front().doRequest == nullptr) {
+                        // If this thread was waiting for a remote response, that response may or
+                        // may not have been enqueued; if it has not been enqueued, there cannot be
+                        // another enqueued response, so it is always correct to remove any enqueued
+                        // response here:
+                        m_lstJob.pop_front();
+                    }
                     if( m_lstJob.empty()
                         && (m_lstCallstack.empty()
                             || m_lstCallstack.front() != 0) )