From 9b44fa750bf6141cd70f97b6fe51935e24812bbb Mon Sep 17 00:00:00 2001 From: Anthony Nicholls Date: Mon, 20 May 2024 15:31:31 +0100 Subject: [PATCH] Timer: Stop the timer thread on shutdown to prevent a potential hang on windows This commit reverts commit: 9e9da250 as it introduced a regression. On windows when exit is called on a dll it forcibly kills any threads still running before destroying any static objects. This means if a Timer object is static the timer thread will be killed. Later when the static object is destroyed it will wait for the thread to exit, which because the OS forcibly killed it will never come true. --- modules/juce_events/timers/juce_Timer.cpp | 101 +++++++++++++++++----- modules/juce_events/timers/juce_Timer.h | 1 + 2 files changed, 78 insertions(+), 24 deletions(-) diff --git a/modules/juce_events/timers/juce_Timer.cpp b/modules/juce_events/timers/juce_Timer.cpp index e0ac72e8d9..fd863c5f88 100644 --- a/modules/juce_events/timers/juce_Timer.cpp +++ b/modules/juce_events/timers/juce_Timer.cpp @@ -35,20 +35,70 @@ namespace juce { +class ShutdownDetector : private DeletedAtShutdown +{ +public: + ShutdownDetector() = default; + + ~ShutdownDetector() override + { + getListeners().call (&Listener::applicationShuttingDown); + } + + struct Listener + { + virtual ~Listener() = default; + virtual void applicationShuttingDown() = 0; + }; + + static void addListener (Listener* listenerToAdd) + { + // Only try to create an instance of the ShutdownDetector when a listener is added + [[maybe_unused]] auto* instance = getInstance(); + getListeners().add (listenerToAdd); + } + + static void removeListener (Listener* listenerToRemove) + { + getListeners().remove (listenerToRemove); + } + +private: + using ListenerListType = ListenerList>; + + // By having a static ListenerList it can outlive the ShutdownDetector instance preventing + // issues for objects trying to remove themselves after the instance has been deleted + static ListenerListType& getListeners() + { + static ListenerListType listeners; + return listeners; + } + + JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ShutdownDetector) + JUCE_DECLARE_NON_MOVEABLE (ShutdownDetector) + JUCE_DECLARE_SINGLETON (ShutdownDetector, false) +}; + +JUCE_IMPLEMENT_SINGLETON (ShutdownDetector) + class Timer::TimerThread final : private Thread, - private DeletedAtShutdown + private ShutdownDetector::Listener { public: using LockType = CriticalSection; - JUCE_DECLARE_SINGLETON (TimerThread, true) + TimerThread() + : Thread ("JUCE Timer") + { + timers.reserve (32); + ShutdownDetector::addListener (this); + } ~TimerThread() override { - signalThreadShouldExit(); - callbackArrived.signal(); + stopThreadAsync(); + ShutdownDetector::removeListener (this); stopThread (-1); - clearSingletonInstance(); } void run() override @@ -215,8 +265,8 @@ private: void messageCallback() override { - if (auto* instance = TimerThread::getInstanceWithoutCreating()) - instance->callTimers(); + if (auto instance = SharedResourcePointer::getSharedObjectWithoutCreating()) + (*instance)->callTimers(); } }; @@ -285,13 +335,21 @@ private: } //============================================================================== - TimerThread() : Thread ("JUCE Timer") { timers.reserve (32); } + void applicationShuttingDown() final + { + stopThreadAsync(); + } + void stopThreadAsync() + { + signalThreadShouldExit(); + callbackArrived.signal(); + } + + //============================================================================== JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (TimerThread) }; -JUCE_IMPLEMENT_SINGLETON (Timer::TimerThread) - //============================================================================== Timer::Timer() noexcept {} Timer::Timer (const Timer&) noexcept {} @@ -315,16 +373,13 @@ void Timer::startTimer (int interval) noexcept // running, then you're not going to get any timer callbacks! JUCE_ASSERT_MESSAGE_MANAGER_EXISTS - if (auto* instance = TimerThread::getInstance()) - { - bool wasStopped = (timerPeriodMs == 0); - timerPeriodMs = jmax (1, interval); + bool wasStopped = (timerPeriodMs == 0); + timerPeriodMs = jmax (1, interval); - if (wasStopped) - instance->addTimer (this); - else - instance->resetTimerCounter (this); - } + if (wasStopped) + timerThread->addTimer (this); + else + timerThread->resetTimerCounter (this); } void Timer::startTimerHz (int timerFrequencyHz) noexcept @@ -339,17 +394,15 @@ void Timer::stopTimer() noexcept { if (timerPeriodMs > 0) { - if (auto* instance = TimerThread::getInstanceWithoutCreating()) - instance->removeTimer (this); - + timerThread->removeTimer (this); timerPeriodMs = 0; } } void JUCE_CALLTYPE Timer::callPendingTimersSynchronously() { - if (auto* instance = TimerThread::getInstanceWithoutCreating()) - instance->callTimersSynchronously(); + if (auto instance = SharedResourcePointer::getSharedObjectWithoutCreating()) + (*instance)->callTimersSynchronously(); } struct LambdaInvoker final : private Timer, diff --git a/modules/juce_events/timers/juce_Timer.h b/modules/juce_events/timers/juce_Timer.h index 77a6dfdadc..b2c42e21e0 100644 --- a/modules/juce_events/timers/juce_Timer.h +++ b/modules/juce_events/timers/juce_Timer.h @@ -141,6 +141,7 @@ private: class TimerThread; size_t positionInQueue = (size_t) -1; int timerPeriodMs = 0; + SharedResourcePointer timerThread; Timer& operator= (const Timer&) = delete; };