From e8358d0a0f7c2c4b1ccf800fe9ec8a7f2bf7066a Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Thu, 25 Jul 2024 07:59:58 +0200 Subject: [PATCH] Keep around a single configmgr::Components::WriteThread instance ...instead of joining and later re-spawning one. This helps with the current Emscripten setup (see 5b1df7709d75c6dd993da5a272e7e37e55a70174 "Document the Emscripten threads issue"), and probably doesn't matter much either way on other platforms (so just get it in unconditionally). (Renaming the delay_ member variable to delayOrTerminate_, to make its overall role more clear.) (I ran into this when trying to turn the Emscripten build from starting up with a Writer document to starting up with the start center, and then manually opening a new Writer document. The resulting different usage scheme happened to exhaust our current -sPTHREAD_POOL_SIZE=4 pool quickly, so this is one of multiple commits to address that.) Change-Id: I1bd28604b4640d2afad982bfd82b1acee8200e26 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170993 Reviewed-by: Stephan Bergmann Tested-by: Jenkins --- configmgr/source/components.cxx | 56 +++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/configmgr/source/components.cxx b/configmgr/source/components.cxx index d2b709e9c337..4790cdd4bd3c 100644 --- a/configmgr/source/components.cxx +++ b/configmgr/source/components.cxx @@ -21,6 +21,8 @@ #include #include +#include +#include #include #include #include @@ -153,7 +155,16 @@ public: rtl::Reference< WriteThread > * reference, Components & components, OUString url, Data const & data); - void flush() { delay_.set(); } + void trigger() { + std::scoped_lock l(triggerMutex_); + triggered_ = true; + triggerCondition_.notify_all(); + } + + void flush() { + delayOrTerminate_.set(); + trigger(); + } private: virtual ~WriteThread() override {} @@ -164,7 +175,10 @@ private: Components & components_; OUString url_; Data const & data_; - osl::Condition delay_; + osl::Condition delayOrTerminate_; + std::mutex triggerMutex_; + std::condition_variable triggerCondition_; + bool triggered_; std::shared_ptr lock_; }; @@ -173,26 +187,41 @@ Components::WriteThread::WriteThread( OUString url, Data const & data): Thread("configmgrWriter"), reference_(reference), components_(components), url_(std::move(url)), data_(data), + triggered_(false), lock_( lock() ) { assert(reference != nullptr); } void Components::WriteThread::execute() { - delay_.wait(std::chrono::seconds(1)); // must not throw; result_error is harmless and ignored - osl::MutexGuard g(*lock_); // must not throw - try { - try { - writeModFile(components_, url_, data_); - } catch (css::uno::RuntimeException &) { - // Ignore write errors, instead of aborting: - TOOLS_WARN_EXCEPTION("configmgr", "error writing modifications"); + for (;;) { + { + std::unique_lock l(triggerMutex_); + while (!triggered_) { + triggerCondition_.wait(l); + } + triggered_ = false; + } + delayOrTerminate_.wait(std::chrono::seconds(1)); + // must not throw; result_error is harmless and ignored + osl::MutexGuard g(*lock_); // must not throw + try { + try { + writeModFile(components_, url_, data_); + } catch (css::uno::RuntimeException &) { + // Ignore write errors, instead of aborting: + TOOLS_WARN_EXCEPTION("configmgr", "error writing modifications"); + } + } catch (...) { + reference_->clear(); + throw; + } + if (!delayOrTerminate_.check()) { + continue; } - } catch (...) { reference_->clear(); - throw; + break; } - reference_->clear(); } Components & Components::getSingleton( @@ -284,6 +313,7 @@ void Components::writeModifications() { &writeThread_, *this, modificationFileUrl_, data_); writeThread_->launch(); } + writeThread_->trigger(); break; case ModificationTarget::Dconf: #if ENABLE_DCONF