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

[Jan-Marek Glogowski (via logerrit)]

 include/vcl/scheduler.hxx    |    1 -
 vcl/README.scheduler         |   11 +++++++++++
 vcl/inc/schedulerimpl.hxx    |   19 -------------------
 vcl/source/app/scheduler.cxx |    3 +++
 vcl/source/app/svapp.cxx     |    1 -
 5 files changed, 14 insertions(+), 21 deletions(-)

New commits:
commit e4b702718e83c1ad76f001e4d3641cc2dd17cd7b
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Dec 4 21:05:28 2020 +0100
Commit:     Jan-Marek Glogowski <glogow at fbihome.de>
CommitDate: Mon Dec 7 14:23:57 2020 +0100

    Unlock scheduler in deinit for ProcessEventsToIdle
    
    The scheduler is normally never locked when not processing its own
    task lists. So while the fix is correct, that ProcessEventsToIdle
    shouldn't run with a locked scheduler, just unlock it in the
    actual shutdown case, where the deadlock occurred.
    
    This also reverts commit 46f6b39c6d8acd064bafb2416feba757ba0d0fbc.
    
    Change-Id: If1603c1563772dd1d156dc6d9bcddbf58aa721c1
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107241
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>

diff --git a/include/vcl/scheduler.hxx b/include/vcl/scheduler.hxx
index 955043116b50..9318b2109641 100644
--- a/include/vcl/scheduler.hxx
+++ b/include/vcl/scheduler.hxx
@@ -27,7 +27,6 @@ struct ImplSchedulerContext;
 class VCL_DLLPUBLIC Scheduler final
 {
     friend class SchedulerGuard;
-    friend class SchedulerGuardReleaser;
     friend class Task;
     Scheduler() = delete;
 
diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 52c76dac5b85..718509033e79 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -58,6 +58,17 @@ Task::mpSchedulerData, which is actually a part of the scheduler.
 Before invoking the task, we have to release the lock, so others can
 Start new Tasks.
 
+The Scheduler just processes it's own Tasks in the main thread and needs
+the SolarMutex for it and for DeInit (tested by DBG_TESTSOLARMUTEX). All
+the other interaction just take the SchedulerMutex or don't need locking
+at all.
+
+There is a "workaround" for static Task objecs, which would crash LO on
+destruction, because Task::~Task would try to de-register itself in the
+Scheduler, while the SchedulerLock would be long gone. OTOH this makes
+Task handling less error-prone, than doing "special" manual cleanup.
+There is also a "= TODOs and ideas =" to get rid if static Tasks.
+
 
 = Lifecycle / thread-safety of Scheduler-based objects =
 
diff --git a/vcl/inc/schedulerimpl.hxx b/vcl/inc/schedulerimpl.hxx
index d95fb0157880..ffb08084252c 100644
--- a/vcl/inc/schedulerimpl.hxx
+++ b/vcl/inc/schedulerimpl.hxx
@@ -65,25 +65,6 @@ public:
     }
 };
 
-class SchedulerGuardReleaser final
-{
-public:
-    SchedulerGuardReleaser()
-    {
-        Scheduler::Lock();
-        m_nLockCount = Scheduler::Unlock(true) - 1;
-    }
-
-    ~SchedulerGuardReleaser()
-    {
-        if (m_nLockCount)
-            Scheduler::Lock(m_nLockCount);
-    }
-
-private:
-    sal_uInt32 m_nLockCount;
-};
-
 #endif // INCLUDED_VCL_INC_SCHEDULERIMPL_HXX
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index f9fc3dc01fc6..9ae60f3f344d 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -124,7 +124,10 @@ void Scheduler::ImplDeInitScheduler()
               "DeInit the scheduler - pending tasks: " << nTasks );
 
     // clean up all the sfx::SfxItemDisruptor_Impl Idles
+    sal_uInt32 nLockCount = Unlock(true);
+    assert(1 == nLockCount);
     ProcessEventsToIdle();
+    Lock(nLockCount);
 #endif
     rSchedCtx.mbActive = false;
 
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index 0b30a0b6b688..f91375e77edd 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -462,7 +462,6 @@ bool Application::Reschedule( bool i_bAllEvents )
 
 void Scheduler::ProcessEventsToIdle()
 {
-    SchedulerGuardReleaser aReleaser;
     int nSanity = 1;
     while( Application::Reschedule( true ) )
     {