[Libreoffice-commits] core.git: vcl/inc vcl/unx

[Stephan Bergmann]

 vcl/inc/unx/gtk/gtkinst.hxx |    2 +-
 vcl/unx/gtk/gtkinst.cxx     |   23 +++++++++--------------
 2 files changed, 10 insertions(+), 15 deletions(-)

New commits:
commit 07157e644fa9666850767ff6bd54c1511167a0a2
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Thu Feb 11 17:34:35 2016 +0100

    Keep track of ThreadsEnter/Leave acquire counts per thread
    
    Appears that gtk_clipboard_wait_for_targets (called from
    GtkClipboardTransferable::getTransferDataFlavorsAsVector,
    vcl/unx/gtk3/gtk3gtkinst.cxx) internally calls gdk_threads_leave/enter, so if
    two threads concurrently try to call into getTransferDataFlavorsAsVector, each
    with the SolarMutex recursively acquired a different number of times, the
    GtkYieldMutex::ThreadsLeave/Enter calls can pair up so that the threads' acquire
    counts get swapped.  Then eventually causing one thread to prematurely get the
    SolarMutex unlocked.
    
    This was fairly well reproducible when calling an --enable-dbgutil's soffice
    with an .oxt file on the command line, causing both the Extension Manager dialog
    and an empty Writer document to open simultaneously (which apparently both run
    into the above getTransferDataFlavorsAsVector), and then running into some
    DBG_TESTSOLARMUTEX assert firing when the SolarMutex had been unlocked
    prematurely.
    
    According to <http://en.cppreference.com/w/cpp/compiler_support>, support for
    thread_local is available partially since GCC 4.4 and fully only since GCC 4.8,
    so hope the partial support in GCC 4.7 (our current baseline) is good enough for
    what is used here.
    
    Change-Id: I9fad318078be5f31111a7cbd15d0e1349e7ddd3e
    Reviewed-on: https://gerrit.libreoffice.org/22291
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/vcl/inc/unx/gtk/gtkinst.hxx b/vcl/inc/unx/gtk/gtkinst.hxx
index 3122956..6f1f999 100644
--- a/vcl/inc/unx/gtk/gtkinst.hxx
+++ b/vcl/inc/unx/gtk/gtkinst.hxx
@@ -42,7 +42,7 @@ class GtkPrintWrapper;
 class GenPspGraphics;
 class GtkYieldMutex : public SalYieldMutex
 {
-    std::list<sal_uLong> aYieldStack;
+    thread_local static sal_uIntPtr yieldCount;
 
 public:
          GtkYieldMutex() {}
diff --git a/vcl/unx/gtk/gtkinst.cxx b/vcl/unx/gtk/gtkinst.cxx
index 0b3ef7b..a1b7522 100644
--- a/vcl/unx/gtk/gtkinst.cxx
+++ b/vcl/unx/gtk/gtkinst.cxx
@@ -288,29 +288,24 @@ SalPrinter* GtkInstance::CreatePrinter( SalInfoPrinter* pInfoPrinter )
  * for each pair, so we can accurately restore
  * it later.
  */
+thread_local sal_uIntPtr GtkYieldMutex::yieldCount;
+
 void GtkYieldMutex::ThreadsEnter()
 {
     acquire();
-    if( !aYieldStack.empty() )
-    { /* Previously called ThreadsLeave() */
-        sal_uLong nCount = aYieldStack.front();
-        aYieldStack.pop_front();
-        while( nCount-- > 1 )
-            acquire();
+    for (; yieldCount != 0; --yieldCount) {
+        acquire();
     }
 }
 
 void GtkYieldMutex::ThreadsLeave()
 {
-    aYieldStack.push_front( mnCount );
-
-    SAL_WARN_IF(
-        mnThreadId && mnThreadId != osl::Thread::getCurrentIdentifier(),
-        "vcl.gtk", "other thread " << mnThreadId << " owns the mutex");
-
-    while( mnCount > 1 )
+    assert(mnCount != 0);
+    assert(yieldCount == 0);
+    yieldCount = mnCount - 1;
+    for (sal_uIntPtr i = 0; i != yieldCount + 1; ++i) {
         release();
-    release();
+    }
 }
 
 SalVirtualDevice* GtkInstance::CreateVirtualDevice( SalGraphics *pG,