Reapply "Introduce a guard to delay processing of idles"

This reverts commit 330c99dd7525cf1fd20b9b8fe388e02468ee2842.

7 files changed, 74 insertions, 11 deletions

diff --git a/embeddedobj/source/inc/oleembobj.hxx b/embeddedobj/source/inc/oleembobj.hxx
index 274ecfaf8847..cf7c5ebe4ab4 100644
--- a/embeddedobj/source/inc/oleembobj.hxx
+++ b/embeddedobj/source/inc/oleembobj.hxx
@@ -249,7 +249,8 @@ protected:
                             const css::uno::Reference< css::embed::XStorage >& xStorage,
                             const OUString& sEntName,
                             const css::uno::Sequence< css::beans::PropertyValue >& lObjArgs,
-                            bool bSaveAs );
+                            bool bSaveAs,
+                            osl::ResettableMutexGuard& rGuard);
 #ifdef _WIN32
     /// @throws css::uno::Exception
     void StoreObjectToStream( css::uno::Reference< css::io::XOutputStream > const & xOutStream );
diff --git a/embeddedobj/source/msole/olepersist.cxx b/embeddedobj/source/msole/olepersist.cxx
index 86403f41bb3e..381fc7b0d68c 100644
--- a/embeddedobj/source/msole/olepersist.cxx
+++ b/embeddedobj/source/msole/olepersist.cxx
@@ -58,6 +58,17 @@
 using namespace ::com::sun::star;
 using namespace ::comphelper;
 
+namespace
+{
+#if defined(_WIN32)
+template <class Proc> auto ExecUnlocked(Proc proc, osl::ResettableMutexGuard& guard)
+{
+    ClearedMutexArea area(guard);
+    return proc();
+}
+#endif
+}
+
 
 bool KillFile_Impl( const OUString& aURL, const uno::Reference< uno::XComponentContext >& xContext )
 {
@@ -1059,8 +1070,11 @@ void OleEmbeddedObject::StoreToLocation_Impl(
                             const uno::Reference< embed::XStorage >& xStorage,
                             const OUString& sEntName,
                             const uno::Sequence< beans::PropertyValue >& lObjArgs,
-                            bool bSaveAs )
+                            bool bSaveAs, osl::ResettableMutexGuard& rGuard)
 {
+#ifndef _WIN32
+    (void)rGuard;
+#endif
     // TODO: use lObjArgs
     // TODO: exchange StoreVisualReplacement by SO file format version?
 
@@ -1110,7 +1124,7 @@ void OleEmbeddedObject::StoreToLocation_Impl(
 #ifdef _WIN32
         // if the object was NOT modified after storing it can be just copied
         // as if it was in loaded state
-      || ( m_pOleComponent && !m_pOleComponent->IsDirty() )
+        || (m_pOleComponent && !ExecUnlocked([this] { return m_pOleComponent->IsDirty(); }, rGuard))
 #endif
     )
     {
@@ -1482,13 +1496,13 @@ void SAL_CALL OleEmbeddedObject::storeToEntry( const uno::Reference< embed::XSto
     }
     // end wrapping related part ====================
 
-    ::osl::MutexGuard aGuard( m_aMutex );
+    ::osl::ResettableMutexGuard aGuard( m_aMutex );
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
     VerbExecutionControllerGuard aVerbGuard( m_aVerbExecutionController );
 
-    StoreToLocation_Impl( xStorage, sEntName, lObjArgs, false );
+    StoreToLocation_Impl( xStorage, sEntName, lObjArgs, false, aGuard );
 
     // TODO: should the listener notification be done?
 }
@@ -1509,13 +1523,13 @@ void SAL_CALL OleEmbeddedObject::storeAsEntry( const uno::Reference< embed::XSto
     }
     // end wrapping related part ====================
 
-    ::osl::MutexGuard aGuard( m_aMutex );
+    ::osl::ResettableMutexGuard aGuard( m_aMutex );
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
     VerbExecutionControllerGuard aVerbGuard( m_aVerbExecutionController );
 
-    StoreToLocation_Impl( xStorage, sEntName, lObjArgs, true );
+    StoreToLocation_Impl( xStorage, sEntName, lObjArgs, true, aGuard );
 
     // TODO: should the listener notification be done here or in saveCompleted?
 }
@@ -1691,7 +1705,7 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
     // ask container to store the object, the container has to make decision
     // to do so or not
 
-    osl::ClearableMutexGuard aGuard(m_aMutex);
+    osl::ResettableMutexGuard aGuard(m_aMutex);
     if ( m_bDisposed )
         throw lang::DisposedException(); // TODO
 
@@ -1717,7 +1731,7 @@ void SAL_CALL OleEmbeddedObject::storeOwn()
     bool bStoreLoaded = true;
 
 #ifdef _WIN32
-    if ( m_nObjectState != embed::EmbedStates::LOADED && m_pOleComponent && m_pOleComponent->IsDirty() )
+    if ( m_nObjectState != embed::EmbedStates::LOADED && m_pOleComponent && ExecUnlocked([this] { return m_pOleComponent->IsDirty(); }, aGuard) )
     {
         bStoreLoaded = false;
 
diff --git a/include/vcl/scheduler.hxx b/include/vcl/scheduler.hxx
index 1b63404139bf..0181e52c33d2 100644
--- a/include/vcl/scheduler.hxx
+++ b/include/vcl/scheduler.hxx
@@ -81,6 +81,13 @@ public:
     static void       SetDeterministicMode(bool bDeterministic);
     /// Return the current state of deterministic mode.
     static bool       GetDeterministicMode();
+
+    // Makes sure that idles are not processed, until the guard is destroyed
+    struct VCL_DLLPUBLIC IdlesLockGuard final
+    {
+        IdlesLockGuard();
+        ~IdlesLockGuard();
+    };
 };
 
 #endif // INCLUDED_VCL_SCHEDULER_HXX
diff --git a/sw/source/core/doc/DocumentLayoutManager.cxx b/sw/source/core/doc/DocumentLayoutManager.cxx
index 75c8e86acdb2..425729833754 100644
--- a/sw/source/core/doc/DocumentLayoutManager.cxx
+++ b/sw/source/core/doc/DocumentLayoutManager.cxx
@@ -43,6 +43,7 @@
 #include <svx/svdobj.hxx>
 #include <svx/svdpage.hxx>
 #include <osl/diagnose.h>
+#include <vcl/scheduler.hxx>
 
 using namespace ::com::sun::star;
 
@@ -191,6 +192,9 @@ SwFrameFormat *DocumentLayoutManager::MakeLayoutFormat( RndStdIds eRequest, cons
 /// Deletes the denoted format and its content.
 void DocumentLayoutManager::DelLayoutFormat( SwFrameFormat *pFormat )
 {
+    // Do not paint, until the destruction is complete. Paint may access the layout and nodes
+    // while it's in inconsistent state, and crash.
+    Scheduler::IdlesLockGuard g;
     // A chain of frames needs to be merged, if necessary,
     // so that the Frame's contents are adjusted accordingly before we destroy the Frames.
     const SwFormatChain &rChain = pFormat->GetChain();
diff --git a/vcl/inc/svdata.hxx b/vcl/inc/svdata.hxx
index 06d0aeb9b9af..725f0cda4260 100644
--- a/vcl/inc/svdata.hxx
+++ b/vcl/inc/svdata.hxx
@@ -24,6 +24,7 @@
 
 #include <o3tl/lru_map.hxx>
 #include <o3tl/hash_combine.hxx>
+#include <osl/conditn.hxx>
 #include <tools/fldunit.hxx>
 #include <unotools/options.hxx>
 #include <vcl/bitmapex.hxx>
@@ -383,6 +384,7 @@ struct ImplSchedulerContext
     std::mutex              maMutex;                        ///< the "scheduler mutex" (see
                                                             ///< vcl/README.scheduler)
     bool                    mbActive = true;                ///< is the scheduler active?
+    oslInterlockedCount     mnIdlesLockCount = 0;           ///< temporary ignore idles
 };
 
 struct ImplSVData
@@ -424,6 +426,9 @@ struct ImplSVData
     css::uno::Reference<css::datatransfer::clipboard::XClipboard> m_xSystemClipboard;
 #endif
 
+    osl::Condition m_inExecuteCondtion; // Set when code returns to Application::Execute,
+                                        // i.e. no nested message loops run
+
     Link<LinkParamNone*,void> maDeInitHook;
 
     // LOK & headless backend specific hooks
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index 7afdfd0846e6..c0fc79d4ac5c 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -272,6 +272,32 @@ bool Scheduler::GetDeterministicMode()
     return g_bDeterministicMode;
 }
 
+Scheduler::IdlesLockGuard::IdlesLockGuard()
+{
+    ImplSVData* pSVData = ImplGetSVData();
+    ImplSchedulerContext& rSchedCtx = pSVData->maSchedCtx;
+    osl_atomic_increment(&rSchedCtx.mnIdlesLockCount);
+    if (!Application::IsMainThread())
+    {
+        // Make sure that main thread has reached the main message loop, so no idles are executing.
+        // It is important to ensure this, because e.g. ProcessEventsToIdle could be executed in a
+        // nested event loop, while an active processed idle in the main thread is waiting for some
+        // condition to proceed. Only main thread returning to Application::Execute guarantees that
+        // the flag really took effect.
+        pSVData->m_inExecuteCondtion.reset();
+        std::optional<SolarMutexReleaser> releaser;
+        if (pSVData->mpDefInst->GetYieldMutex()->IsCurrentThread())
+            releaser.emplace();
+        pSVData->m_inExecuteCondtion.wait();
+    }
+}
+
+Scheduler::IdlesLockGuard::~IdlesLockGuard()
+{
+    ImplSchedulerContext& rSchedCtx = ImplGetSVData()->maSchedCtx;
+    osl_atomic_decrement(&rSchedCtx.mnIdlesLockCount);
+}
+
 inline void Scheduler::UpdateSystemTimer( ImplSchedulerContext &rSchedCtx,
                                           const sal_uInt64 nMinPeriod,
                                           const bool bForce, const sal_uInt64 nTime )
@@ -458,8 +484,10 @@ void Scheduler::CallbackTaskScheduling()
     // Delay invoking tasks with idle priorities as long as there are user input or repaint events
     // in the OS event queue. This will often effectively compress such events and repaint only
     // once at the end, improving performance in cases such as repeated zooming with a complex document.
-    bool bDelayInvoking = bIsHighPriorityIdle &&
-        Application::AnyInput( VclInputFlags::MOUSE | VclInputFlags::KEYBOARD | VclInputFlags::PAINT );
+    bool bDelayInvoking
+        = bIsHighPriorityIdle
+          && (rSchedCtx.mnIdlesLockCount > 0
+              || Application::AnyInput(VclInputFlags::MOUSE | VclInputFlags::KEYBOARD | VclInputFlags::PAINT));
 
     /*
     * Current policy is that scheduler tasks aren't allowed to throw an exception.
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index d2c6294917f0..1f7831ea060b 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -450,7 +450,11 @@ void Application::Execute()
             std::abort();
         }
         while (!pSVData->maAppData.mbAppQuit)
+        {
             Application::Yield();
+            SolarMutexReleaser releaser; // Give a chance for the waiting threads to lock the mutex
+            pSVData->m_inExecuteCondtion.set();
+        }
     }
 
     pSVData->maAppData.mbInAppExecute = false;