[Libreoffice-commits] core.git: include/vcl svx/source vcl/README.scheduler vcl/source vcl/win

[Jan-Marek Glogowski]

 include/vcl/task.hxx         |   10 ++++++++++
 svx/source/svdraw/svdetc.cxx |    1 +
 vcl/README.scheduler         |    7 +++++++
 vcl/source/app/scheduler.cxx |   14 +++++++++++---
 vcl/win/gdi/salbmp.cxx       |    2 +-
 5 files changed, 30 insertions(+), 4 deletions(-)

New commits:
commit 06ce312f79cb0871c0b110ba4bff16f5aaa0f538
Author: Jan-Marek Glogowski <glogow at fbihome.de>
Date:   Wed Aug 23 16:07:50 2017 +0200

    Workaround static Task destruction error
    
    A task has to get the SchedulerLock to remove itself from the
    Scheduler list. This doesn't work, if the Task is static, as the
    static Scheduler might be destroyed earlier. In this case we fail
    with the following backtrace:
    
     #0 SchedulerMutex::acquire
     #1 Task::~Task
     #2 __run_exit_handlers
    
    Thanks to Michael Stahl to catching this backtrace.
    
    As a workaround this marks static tasks, so they ignore the
    SchedulerMutex in the destructor, We also mark all scheduled Tasks
    as "static" in DeInitScheduler, as their cleanup was already done.
    
    In the end all Tasks should be removed from static objects.
    
    Change-Id: I38be3206378b9449193efaccbc96896ac8de9478
    Reviewed-on: https://gerrit.libreoffice.org/42574
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/include/vcl/task.hxx b/include/vcl/task.hxx
index e45fe3f5ae73..2711d4343932 100644
--- a/include/vcl/task.hxx
+++ b/include/vcl/task.hxx
@@ -47,6 +47,7 @@ class VCL_DLLPUBLIC Task
     const sal_Char    *mpDebugName;     ///< Useful for debugging
     TaskPriority       mePriority;      ///< Task priority
     bool               mbActive;        ///< Currently in the scheduler
+    bool               mbStatic;        ///< Is a static object
 
 protected:
     static void StartTimer( sal_uInt64 nMS );
@@ -88,6 +89,15 @@ public:
     void            Stop();
 
     bool            IsActive() const { return mbActive; }
+
+    /**
+     * This function must be called for static tasks, so the Task destructor
+     * ignores the SchedulerMutex, as it may not be available anymore.
+     * The cleanup is still correct, as it has already happened in
+     * DeInitScheduler call well before the static destructor calls.
+     */
+    void            SetStatic() { mbStatic = true; }
+    bool            IsStatic() const { return mbStatic; }
 };
 
 #endif // INCLUDED_VCL_TASK_HXX
diff --git a/svx/source/svdraw/svdetc.cxx b/svx/source/svdraw/svdetc.cxx
index 89c17590984c..3cc369a49be5 100644
--- a/svx/source/svdraw/svdetc.cxx
+++ b/svx/source/svdraw/svdetc.cxx
@@ -112,6 +112,7 @@ OLEObjCache::OLEObjCache()
     pTimer = new AutoTimer( "svx OLEObjCache pTimer UnloadCheck" );
     pTimer->SetInvokeHandler( LINK(this, OLEObjCache, UnloadCheckHdl) );
     pTimer->SetTimeout(20000);
+    pTimer->SetStatic();
 }
 
 OLEObjCache::~OLEObjCache()
diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 271ce1949695..7e80c1f04eb8 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -172,6 +172,13 @@ Since the Scheduler is always handled by the system message queue, there is
 really no more reasoning to stop after 100 events to prevent LO Scheduler
 starvation.
 
+== Drop static inherited or composed Task objects ==
+
+The sequence of destruction of static objects is not defined. So a static Task
+can not be guaranteed to happen before the Scheduler. When dynamic unloading
+is involved, this becomes an even worse problem. This way we could drop the
+mbStatic workaround from the Task class.
+
 == Run the LO application in its own thread ==
 
 This would probably get rid of most of the MacOS and Windows implementation
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index f20c3ae41677..18e9edb8132c 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -160,6 +160,7 @@ void Scheduler::ImplDeInitScheduler()
                 pTask->mbActive = false;
             }
             pTask->mpSchedulerData = nullptr;
+            pTask->SetStatic();
         }
         ImplSchedulerData* pDeleteSchedulerData = pSchedulerData;
         pSchedulerData = pSchedulerData->mpNext;
@@ -548,6 +549,7 @@ Task::Task( const sal_Char *pDebugName )
     , mpDebugName( pDebugName )
     , mePriority( TaskPriority::DEFAULT )
     , mbActive( false )
+    , mbStatic( false )
 {
 }
 
@@ -556,6 +558,7 @@ Task::Task( const Task& rTask )
     , mpDebugName( rTask.mpDebugName )
     , mePriority( rTask.mePriority )
     , mbActive( false )
+    , mbStatic( false )
 {
     if ( rTask.IsActive() )
         Start();
@@ -563,9 +566,14 @@ Task::Task( const Task& rTask )
 
 Task::~Task() COVERITY_NOEXCEPT_FALSE
 {
-    SchedulerGuard aSchedulerGuard;
-    if ( mpSchedulerData )
-        mpSchedulerData->mpTask = nullptr;
+    if ( !IsStatic() )
+    {
+        SchedulerGuard aSchedulerGuard;
+        if ( mpSchedulerData )
+            mpSchedulerData->mpTask = nullptr;
+    }
+    else
+        assert( nullptr == mpSchedulerData );
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/gdi/salbmp.cxx b/vcl/win/gdi/salbmp.cxx
index c9216bfd0109..fb8fb10d8055 100644
--- a/vcl/win/gdi/salbmp.cxx
+++ b/vcl/win/gdi/salbmp.cxx
@@ -80,7 +80,7 @@ public:
         maEntries()
     {
         SetTimeout(1000);
-        Stop();
+        SetStatic();
     }
 
     ~GdiPlusBuffer() override