From a2095b151409f0fb57aa8feaa4c6282f84040245 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Tue, 26 Jul 2016 23:42:36 +0200 Subject: comphelper,vcl: let DeInitVCL() join some AsyncEventNotifier threads comphelper::AsyncEventNotifier is an amazing class that dispatches events in separate threads, no doubt implemented during times of exuberant optimism about the tractability of shared-state multi-threading. Unfortunately the authors forgot to think about how all those awesome threads will be joined, so if they are somehow blocked, then it may well happen that the events are dispatched when the main thread is already in DeInitVCL, and the objects required for the dispatching already smell somewhat funny. This happens quite reproducibly when changing dbaccess' ModelMethodGuard to lock the SolarMutex too, then CppunitTest_dbaccess_RowSetClones crashes in DeInitVCL() because one AsyncEventNotifier thread was blocked until then by SolarMutexGuard, and this test never Yields once its document is loaded. Try to fix this by joining the "DocumentEventNotifier" threads from DeInitVCL() itself. Since there's no rtl::WeakReference to go with rtl::Reference, refactor the AsyncEventNotifier and create a new AsyncEventNotifierAutoJoin that has to be used with std::shared_ptr and std::weak_ptr. Change-Id: I50a0749795acb04b0776e543f7125767b697ea35 Reviewed-on: https://gerrit.libreoffice.org/27581 Tested-by: Jenkins Reviewed-by: Michael Stahl --- comphelper/source/misc/asyncnotification.cxx | 128 +++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 7 deletions(-) (limited to 'comphelper') diff --git a/comphelper/source/misc/asyncnotification.cxx b/comphelper/source/misc/asyncnotification.cxx index 85643e2b3cf4..e13be286a5dd 100644 --- a/comphelper/source/misc/asyncnotification.cxx +++ b/comphelper/source/misc/asyncnotification.cxx @@ -21,10 +21,12 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -76,6 +78,9 @@ namespace comphelper ::osl::Condition aPendingActions; EventQueue aEvents; bool bTerminate; + // only used for AsyncEventNotifierAutoJoin + char const* name; + std::shared_ptr pKeepThisAlive; EventNotifierImpl() :bTerminate( false ) @@ -83,18 +88,18 @@ namespace comphelper } }; - AsyncEventNotifier::AsyncEventNotifier(char const * name): - Thread(name), m_xImpl(new EventNotifierImpl) + AsyncEventNotifierBase::AsyncEventNotifierBase() + : m_xImpl(new EventNotifierImpl) { } - AsyncEventNotifier::~AsyncEventNotifier() + AsyncEventNotifierBase::~AsyncEventNotifierBase() { } - void AsyncEventNotifier::removeEventsForProcessor( const ::rtl::Reference< IEventProcessor >& _xProcessor ) + void AsyncEventNotifierBase::removeEventsForProcessor( const ::rtl::Reference< IEventProcessor >& _xProcessor ) { ::osl::MutexGuard aGuard( m_xImpl->aMutex ); @@ -103,7 +108,7 @@ namespace comphelper } - void SAL_CALL AsyncEventNotifier::terminate() + void SAL_CALL AsyncEventNotifierBase::terminate() { ::osl::MutexGuard aGuard( m_xImpl->aMutex ); @@ -115,7 +120,7 @@ namespace comphelper } - void AsyncEventNotifier::addEvent( const AnyEventRef& _rEvent, const ::rtl::Reference< IEventProcessor >& _xProcessor ) + void AsyncEventNotifierBase::addEvent( const AnyEventRef& _rEvent, const ::rtl::Reference< IEventProcessor >& _xProcessor ) { ::osl::MutexGuard aGuard( m_xImpl->aMutex ); @@ -128,7 +133,7 @@ namespace comphelper } - void AsyncEventNotifier::execute() + void AsyncEventNotifierBase::execute() { for (;;) { @@ -160,6 +165,115 @@ namespace comphelper } } + AsyncEventNotifier::AsyncEventNotifier(char const* name) + : salhelper::Thread(name) + { + } + + AsyncEventNotifier::~AsyncEventNotifier() + { + } + + void AsyncEventNotifier::execute() + { + return AsyncEventNotifierBase::execute(); + } + + void AsyncEventNotifier::terminate() + { + return AsyncEventNotifierBase::terminate(); + } + + struct theNotifiersMutex : public rtl::Static {}; + static std::vector> g_Notifiers; + + void JoinAsyncEventNotifiers() + { + std::vector> notifiers; + { + ::osl::MutexGuard g(theNotifiersMutex::get()); + notifiers = g_Notifiers; + } + for (std::weak_ptr const& wNotifier : notifiers) + { + std::shared_ptr const pNotifier( + wNotifier.lock()); + if (pNotifier) + { + pNotifier->terminate(); + pNotifier->join(); + } + } + // note it's possible that g_Notifiers isn't empty now in case of leaks, + // particularly since the UNO service manager isn't disposed yet + } + + AsyncEventNotifierAutoJoin::AsyncEventNotifierAutoJoin(char const* name) + { + m_xImpl->name = name; + } + + AsyncEventNotifierAutoJoin::~AsyncEventNotifierAutoJoin() + { + ::osl::MutexGuard g(theNotifiersMutex::get()); + // note: this doesn't happen atomically with the refcount + // hence it's possible this deletes > 1 or 0 elements + g_Notifiers.erase( + std::remove_if(g_Notifiers.begin(), g_Notifiers.end(), + [](std::weak_ptr const& w) { + return w.expired(); + } ), + g_Notifiers.end()); + } + + std::shared_ptr + AsyncEventNotifierAutoJoin::newAsyncEventNotifierAutoJoin(char const* name) + { + std::shared_ptr const ret( + new AsyncEventNotifierAutoJoin(name)); + ::osl::MutexGuard g(theNotifiersMutex::get()); + g_Notifiers.push_back(ret); + return ret; + } + + void AsyncEventNotifierAutoJoin::terminate() + { + return AsyncEventNotifierBase::terminate(); + } + + void AsyncEventNotifierAutoJoin::launch(std::shared_ptr const& xThis) + { + // see salhelper::Thread::launch + xThis->m_xImpl->pKeepThisAlive = xThis; + try { + if (!xThis->create()) { + throw std::runtime_error("osl::Thread::create failed"); + } + } catch (...) { + xThis->m_xImpl->pKeepThisAlive.reset(); + throw; + } + } + + void AsyncEventNotifierAutoJoin::run() + { + // see salhelper::Thread::run + try { + setName(m_xImpl->name); + execute(); + } catch (...) { + onTerminated(); + throw; + } + } + + void AsyncEventNotifierAutoJoin::onTerminated() + { + // try to delete "this" + m_xImpl->pKeepThisAlive.reset(); + return osl::Thread::onTerminated(); + } + } // namespace comphelper -- cgit