drawinglayer/source/primitive2d/BufferedDecompositionFlusher.cxx | 121 +++++----- include/drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx | 11 vcl/source/app/svmain.cxx | 7 3 files changed, 75 insertions(+), 64 deletions(-)
New commits: commit 6fd5570cc9ecfd44c104f734e348b42bcea88f8e Author: Noel Grandin <[email protected]> AuthorDate: Wed Mar 26 08:17:52 2025 +0200 Commit: Noel Grandin <[email protected]> CommitDate: Wed Mar 26 12:48:33 2025 +0100 fix BufferedDecompositionFlusher some more if the onShot() method is processing a lot of work, and at the same time teardown happens, it might managed to restart itself after teardown. What ended up being easiest is making this its own thread, and calling shutdown() on that thread from DeInitVCL. See notes in BufferedDecompositionFlusher.cxx. Change-Id: I70fecb259c6e74d2c102065696f7ae2bed15cab6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183320 Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins diff --git a/drawinglayer/source/primitive2d/BufferedDecompositionFlusher.cxx b/drawinglayer/source/primitive2d/BufferedDecompositionFlusher.cxx index fd19f398ae8b..97f7696a7575 100644 --- a/drawinglayer/source/primitive2d/BufferedDecompositionFlusher.cxx +++ b/drawinglayer/source/primitive2d/BufferedDecompositionFlusher.cxx @@ -21,7 +21,6 @@ #include <sal/log.hxx> #include <drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx> -#include <tools/lazydelete.hxx> namespace drawinglayer::primitive2d { @@ -38,103 +37,104 @@ namespace drawinglayer::primitive2d It is very simple, scales to lots and lots of primitives without needing lots of timers, and performs very little work in the common case. -*/ -namespace -{ -class FlusherDeinit : public tools::DeleteOnDeinitBase -{ - rtl::Reference<BufferedDecompositionFlusher> m_xTimer; - virtual void doCleanup() override - { - SAL_WARN("drawinglayer", "tearing down BufferedDecompositionFlusher"); - m_xTimer->stop(); - m_xTimer->onTeardown(); - m_xTimer = nullptr; - } -public: - FlusherDeinit() - { - m_xTimer = new BufferedDecompositionFlusher; - addDeinitContainer(this); - } - - BufferedDecompositionFlusher* get() { return m_xTimer.get(); } -}; -} + Shutdown notes + -------------------- + The process of handling shutdown is more complicated here than it should be, because we are interacting with + various vcl-level things (by virtue of calling into drawinglayer primitives that use vcl facilities), but we + do not have access to vcl-level API here (like SolarMutexReleaser and vcl::Timer). +*/ static BufferedDecompositionFlusher* getInstance() { - static FlusherDeinit gaTimer; + static std::unique_ptr<BufferedDecompositionFlusher> gaTimer(new BufferedDecompositionFlusher); return gaTimer.get(); } +// static +void BufferedDecompositionFlusher::shutdown() +{ + SAL_WARN("drawinglayer", "tearing down BufferedDecompositionFlusher"); + BufferedDecompositionFlusher* pFlusher = getInstance(); + pFlusher->onTeardown(); + // We have to wait for the thread to exit, otherwise we might end up with the background thread + // trying to process stuff while it has things ripped out underneath it. + pFlusher->join(); +} + // static void BufferedDecompositionFlusher::update(const BufferedDecompositionPrimitive2D* p) { - if (auto flusher = getInstance()) - flusher->updateImpl(p); + getInstance()->updateImpl(p); } // static void BufferedDecompositionFlusher::update(const BufferedDecompositionGroupPrimitive2D* p) { - if (auto flusher = getInstance()) - flusher->updateImpl(p); + getInstance()->updateImpl(p); } BufferedDecompositionFlusher::BufferedDecompositionFlusher() { - setRemainingTime(salhelper::TTimeValue(2, 0)); - start(); + setName("BufferedDecompositionFlusher"); + create(); } void BufferedDecompositionFlusher::updateImpl(const BufferedDecompositionPrimitive2D* p) { std::unique_lock l(maMutex); - maRegistered1.insert(const_cast<BufferedDecompositionPrimitive2D*>(p)); + if (!mbShutdown) + maRegistered1.insert(const_cast<BufferedDecompositionPrimitive2D*>(p)); } void BufferedDecompositionFlusher::updateImpl(const BufferedDecompositionGroupPrimitive2D* p) { std::unique_lock l(maMutex); - maRegistered2.insert(const_cast<BufferedDecompositionGroupPrimitive2D*>(p)); + if (!mbShutdown) + maRegistered2.insert(const_cast<BufferedDecompositionGroupPrimitive2D*>(p)); } -void SAL_CALL BufferedDecompositionFlusher::onShot() +void SAL_CALL BufferedDecompositionFlusher::run() { - auto aNow = std::chrono::steady_clock::now(); - std::vector<rtl::Reference<BufferedDecompositionPrimitive2D>> aRemoved1; - std::vector<rtl::Reference<BufferedDecompositionGroupPrimitive2D>> aRemoved2; + for (;;) { - std::unique_lock l(maMutex); - for (auto it = maRegistered1.begin(); it != maRegistered1.end();) + auto aNow = std::chrono::steady_clock::now(); + std::vector<rtl::Reference<BufferedDecompositionPrimitive2D>> aRemoved1; + std::vector<rtl::Reference<BufferedDecompositionGroupPrimitive2D>> aRemoved2; { - if (aNow - (*it)->maLastAccess > std::chrono::seconds(10)) + std::unique_lock l1(maMutex); + // exit if we have been shutdown + if (mbShutdown) + break; + for (auto it = maRegistered1.begin(); it != maRegistered1.end();) { - aRemoved1.push_back(*it); - it = maRegistered1.erase(it); + if (aNow - (*it)->maLastAccess > std::chrono::seconds(10)) + { + aRemoved1.push_back(*it); + it = maRegistered1.erase(it); + } + else + ++it; } - else - ++it; - } - for (auto it = maRegistered2.begin(); it != maRegistered2.end();) - { - if (aNow - (*it)->maLastAccess > std::chrono::seconds(10)) + for (auto it = maRegistered2.begin(); it != maRegistered2.end();) { - aRemoved2.push_back(*it); - it = maRegistered2.erase(it); + if (aNow - (*it)->maLastAccess > std::chrono::seconds(10)) + { + aRemoved2.push_back(*it); + it = maRegistered2.erase(it); + } + else + ++it; } - else - ++it; } + + for (const auto& r : aRemoved1) + r->setBuffered2DDecomposition(nullptr); + for (const auto& r : aRemoved2) + r->setBuffered2DDecomposition(Primitive2DContainer{}); + + wait(TimeValue(2, 0)); } - for (const auto& r : aRemoved1) - r->setBuffered2DDecomposition(nullptr); - for (const auto& r : aRemoved2) - r->setBuffered2DDecomposition(Primitive2DContainer{}); - setRemainingTime(salhelper::TTimeValue(2, 0)); - start(); } /// Only called by FlusherDeinit @@ -143,11 +143,12 @@ void BufferedDecompositionFlusher::onTeardown() std::unordered_set<rtl::Reference<BufferedDecompositionPrimitive2D>> aRemoved1; std::unordered_set<rtl::Reference<BufferedDecompositionGroupPrimitive2D>> aRemoved2; { - std::unique_lock l(maMutex); + std::unique_lock l2(maMutex); + mbShutdown = true; aRemoved1 = std::move(maRegistered1); aRemoved2 = std::move(maRegistered2); } - // let them destruct outside the lock + // let the destruction happen outside the lock } } // end of namespace drawinglayer::primitive2d diff --git a/include/drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx b/include/drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx index e1afcb75d31d..9c0f5b005b1c 100644 --- a/include/drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx +++ b/include/drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx @@ -21,12 +21,12 @@ #include <drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx> #include <drawinglayer/primitive2d/BufferedDecompositionPrimitive2D.hxx> -#include <salhelper/timer.hxx> +#include <osl/thread.hxx> #include <unordered_set> namespace drawinglayer::primitive2d { -class BufferedDecompositionFlusher : public salhelper::Timer +class BufferedDecompositionFlusher : public osl::Thread { public: static void update(const BufferedDecompositionPrimitive2D*); @@ -34,17 +34,20 @@ public: BufferedDecompositionFlusher(); + static DRAWINGLAYERCORE_DLLPUBLIC void shutdown(); + /// Only called by FlusherDeinit void onTeardown(); private: - virtual void SAL_CALL onShot() override; + virtual void SAL_CALL run() override; void updateImpl(const BufferedDecompositionPrimitive2D*); void updateImpl(const BufferedDecompositionGroupPrimitive2D*); - std::mutex maMutex; std::unordered_set<rtl::Reference<BufferedDecompositionPrimitive2D>> maRegistered1; std::unordered_set<rtl::Reference<BufferedDecompositionGroupPrimitive2D>> maRegistered2; + std::mutex maMutex; + bool mbShutdown{ false }; }; } // end of namespace drawinglayer::primitive2d diff --git a/vcl/source/app/svmain.cxx b/vcl/source/app/svmain.cxx index 4c81138c117f..0df09b05c69d 100644 --- a/vcl/source/app/svmain.cxx +++ b/vcl/source/app/svmain.cxx @@ -30,6 +30,7 @@ #include <comphelper/accessibleeventnotifier.hxx> #include <comphelper/processfactory.hxx> #include <comphelper/asyncnotification.hxx> +#include <drawinglayer/primitive2d/BufferedDecompositionFlusher.hxx> #include <i18nlangtag/mslangid.hxx> #include <unotools/syslocale.hxx> #include <unotools/syslocaleoptions.hxx> @@ -470,6 +471,12 @@ void DeInitVCL() // Some events may need to access objects destroyed in ImplDeleteOnDeInit, so process them first Scheduler::ProcessEventsToIdle(); + { + // unblock we will wait for things that want to take the SolarMutex on another thread + SolarMutexReleaser aReleaser; + drawinglayer::primitive2d::BufferedDecompositionFlusher::shutdown(); + } + tools::DeleteOnDeinitBase::ImplDeleteOnDeInit(); #if OSL_DEBUG_LEVEL > 0
