Race condition in Timer

General JUCE discussion
1

I just saw an assertion fire in the juce_Timer.cpp code. I have a potential explanation for what happened, below. I’m not sure what a proper fix might be but it does look like a race condition which could cause some problems so I’d appreciate it if someone could take a look.

The assertion is in resetTimerCounter():

jassert (timers[pos].timer == t);

which was being called by startTimer():

void Timer::startTimer (int interval) noexcept
{
    // If you're calling this before (or after) the MessageManager is
    // running, then you're not going to get any timer callbacks!
    JUCE_ASSERT_MESSAGE_MANAGER_EXISTS

    bool wasStopped = (timerPeriodMs == 0);
    timerPeriodMs = jmax (1, interval);

    if (wasStopped)
        timerThread->addTimer (this);
    else
        timerThread->resetTimerCounter (this);
}

When this assertion fired, startTimer was calling into resetTimerCounter and so wasStopped was false. However, the debugger showed that timerPeriodMs was 0.

One explanation for how that could happen is a race between the check for wasStopped and the call to resetTimerCounter. A second thread could slip in there and call stopTimer() which would race if the threads interleaved like this:

  1. Thread A calls startTimer. Gets past line that checks for wasStopped.
  2. Thread B calls stopTimer.
  3. Thread A continues, calls into resetTimerCounter for now-stopped-Timer and triggers assertion.

Note that stopTimer() does not reset the positionInQueue so I suspect when this race happens it will either modify the wrong timer’s state or if it was the only timer scheduled could result in an out of bounds access.

Thanks for taking a look,
Rob

1 Like
2

You may be right and maybe we can add some extra resilience there, are you calling start/stop timer on different threads?

3

It was actually the JUCE CoreAudio code where this happened, so might have been on a CoreAudio thread. Maybe CoreAudio has multiple threads, not sure the exact threading architecture there.

Here’s the full call stack of the assertion:

juce::Timer::TimerThread::resetTimerCounter(juce::Timer*) (/Users/rob/Documents/c74/dev/max/maxjuce/modules/juce_events/timers/juce_Timer.cpp:181)
juce::Timer::startTimer(int) (/Users/rob/Documents/c74/dev/max/maxjuce/modules/juce_events/timers/juce_Timer.cpp:324)
juce::CoreAudioClasses::CoreAudioInternal::deviceDetailsChanged() (/Users/rob/Documents/c74/dev/max/maxjuce/modules/juce_audio_devices/native/juce_CoreAudio_mac.cpp:839)
juce::CoreAudioClasses::CoreAudioIODevice::hardwareListenerProc(unsigned int, unsigned int, AudioObjectPropertyAddress const*, void*) (/Users/rob/Documents/c74/dev/max/maxjuce/modules/juce_audio_devices/native/juce_CoreAudio_mac.cpp:1428)
HALObject::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*) (@HALObject::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*):394)
HALSystem::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*) (@HALSystem::PropertiesChanged(unsigned int, AudioObjectPropertyAddress const*):106)
HALSystem::ObjectsPublishedAndDied(unsigned int, unsigned int const*, unsigned int, unsigned int const*) (@HALSystem::ObjectsPublishedAndDied(unsigned int, unsigned int const*, unsigned int, unsigned int const*):457)
HALSystem::AudioObjectsPublishedAndDied(AudioHardwarePlugInInterface**, unsigned int, unsigned int, unsigned int const*, unsigned int, unsigned int const*) (@HALSystem::AudioObjectsPublishedAndDied(AudioHardwarePlugInInterface**, unsigned int, unsigned int, unsigned int const*, unsigned int, unsigned int const*):55)
HALC_ShellPlugIn::ProxyObject_PropertiesChanged(unsigned int, unsigned int, AudioObjectPropertyAddress const*) (@HALC_ShellPlugIn::ProxyObject_PropertiesChanged(unsigned int, unsigned int, AudioObjectPropertyAddress const*):237)
HALC_ProxyNotifications::CallListener_f(void*) (@HALC_ProxyNotifications::CallListener_f(void*):26)
asan_dispatch_call_block_and_release (@asan_dispatch_call_block_and_release:60)
_dispatch_client_callout (@_dispatch_client_callout:8)
_dispatch_lane_serial_drain (@_dispatch_lane_serial_drain:170)
_dispatch_lane_invoke (@_dispatch_lane_invoke:112)
_dispatch_workloop_invoke (@_dispatch_workloop_invoke:444)
_dispatch_workloop_worker_thread (@_dispatch_workloop_worker_thread:165)
_pthread_wqthread (@_pthread_wqthread:75)
1 Like
4

Thanks and indeed that does call start/stop timer on different threads. I’ll try and get a fix up early next week.

2 Likes