juce::Timers are not thread safe. Bug?

General JUCE discussion
1

While the docs do not explicitly say that they should be, this comment Note that if you call this from a background thread while the message-thread is already in the middle of your callback, then this method will cancel any future timer callbacks, but it will return without waiting for the current one to finish. leads me to believe that should should be able to call startTimer and stopTimer from a background thread. Internally, addTimer, removeTimer and resetTimerCounter all use a const LockType::ScopedLockType sl (lock); so I believe the intention is that everything is thread safe.

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

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

However, all of the above happens outside the lock. wasStopped is not protected, so we can try to start started timers and stop timers that aren’t running. This leads to crashes.

Example program to reproduce: juce_bugs/ThreadedTimerTest/Source/MainComponent.cpp at c75b7f3651b88eb7b3071d1a51a1c70eb8d49975 · FigBug/juce_bugs · GitHub

I have 26 recorded occurrences of this crash in a 4 day period, so it is fairly common.

2

Why don’t you use MessageManager::callAsync() to call startTimer and stopTimer from the message-thread directly? If you need lower latency just combine it with an additional atomic in timerCallback() to make it return early instead of stopping the timer.