Bug: juce::Timer is broken on the develop branch

General JUCE discussion
1

I was trying to find the reason why the tooltips don’t work in Pro Tools. Turns out the juce::Timer class is broken on the develop branch. Specifically, this commit is the one breaking the timer.

The problem is specific to running inside Pro Tools, I found that when running inside other hosts (Logic Pro, Reaper, Ableton) the problem doesn’t exist.

Diving deeper the problem seems to be that Timer::TimerThread::handleAsyncUpdate never gets called so the timer thread never starts.

Environment:
macOS 14.2.1 (23C71) (Sonoma), Apple M1 Max
Pro Tools 2023.9.0
Xcode 15.2

2

I can confirm that switching to the develop branch and running the unit tests, the Timer::callPendingTimersSynchronously now fails 100% of the times:

_______________________________________________________________________ 
test_call_pending_timers_synchronously
________________________________________________________________________

juce_app = <popsicle.TestApplication object at 0x106c68db0>

    def test_call_pending_timers_synchronously(juce_app):
        t = CustomTimer()
    
        t.startTimer(50)
        assert t.timesCalled == 0
    
        time.sleep(0.1)

        juce.Timer.callPendingTimersSynchronously()
>       assert t.timesCalled >= 1
E       assert 0 >= 1
E        +  where 0 = <tests.test_juce_events.test_Timer.CustomTimer object at 0x106b57d10>.timesCalled```
3

@ruurdadema I think I’ve managed to reproduce your issue but only under certain circumstances.

Is it possible you have a timer that is being created before the MessageManager has started running? One way this might happen (and the way I’ve reproduced it) is if a timer (note it could be any Timer!), or an object that creates a Timer, is being created as part of a static instance.

If you are able to build a debug version of your plugin and launch ProTools from your IDE you should see an assertion.

I’m not sure why this would have worked before I’ll look into it and see if there is something sensible we can do on our end.

@kunitoki thanks for sharing. I did have a go at reproducing roughly what it looks like you have but I’m not sure it’s a fair test in this instance.

  1. A message manager needs to be created and running before the Timer is created (it seems like this was always the case)
  2. There needs to be at least 50 ms between startTimer(50) and callPendingTimersSynchronously(), ideally much more to get the number of false negative results down
  3. If the MessageManager is already running I don’t think you should need to call callPendingTimersSynchronously(). I think the function is only there to support some kind of VST edgecase? I haven’t looked into in any detail.
4
  1. MessageManager is running in that test, that code actually runs on it
  2. Time has elapsed already
  3. I expect callPendingTimersSynchronously to fire them synchronously
5

EDIT: I think I found the cause of the problem, skip to the next reply.

By attaching the debugger like you suggested I was able to get a bit more insight into what happens.

I placed breakpoints inside:

  • MessageManager::MessageManager()
  • void TimerThread::handleAsyncUpdate()
  • void TimerThread::run()
  • TimerThread::TimerThread()
  • TimerThread::~TimerThread()

After Pro Tools starts scanning plugins the following sequence is called:

  • MessageManager::MessageManager()
  • TimerThread::TimerThread()

Then Pro Tools offers the dashboard window and I’m able to open an empty session. After that I instantiate our plugin after which the following calls are made:

  • MessageManager::MessageManager()

After that, when I close Pro Tools I get several leaked object asserts:

  • *** Leaked objects detected: 2 instance(s) of class SharedResourcePointer
  • *** Leaked objects detected: 1 instance(s) of class TimerThread
  • *** Leaked objects detected: 1 instance(s) of class Thread
  • *** Leaked objects detected: 3 instance(s) of class WaitableEvent
  • *** Leaked objects detected: 1 instance(s) of class AsyncUpdater

The interesting thing here is that I expect more calls to be made when the plugin scanning process happens. I expect something like this:

  • MessageManager::MessageManager()
  • TimerThread::TimerThread()
  • void TimerThread::handleAsyncUpdate()
  • void TimerThread::run()

But this is not what happens.

Without diving in too deep, what seems to be happening is something like this:

  • Plugin scanning starts
  • MessageManager gets created
  • A Timer gets created (my MainPluginProcessor which is a subclass of juce::AudioProcessor also inherits from Timer which seems to trigger the creation of the TimerThread)
  • A TimerThread gets created which triggers an async update to start the TimerThread thread running
  • Pro Tools destroys the plugin instance and takes MessageManager with it
  • For some reason a Timer sticks around keeping the SharedResourcePointer with TimerThread alive. I think this is where the core problem is.
  • After loading Pro Tools and instantiating the plugin a MessageManager gets created but not a TimerThread, since that one already exists.

So we seem to basically end up with a case where the TimerThread is instantiated, but not started because the MessageManager is destroyed before it gets a chance to callback the async updater.

When I switch to one commit before the problematic commit, TimerThread gets properly DeletedAtShutdown and the JUCE leak detector doesn’t report any leaks when shutting down Pro Tools:

juce::Timer::TimerThread::~TimerThread() juce_Timer.cpp:41
juce::Timer::TimerThread::~TimerThread() juce_Timer.cpp:40
juce::Timer::TimerThread::~TimerThread() juce_Timer.cpp:40
juce::DeletedAtShutdown::deleteAll() juce_DeletedAtShutdown.cpp:75
juce::shutdownJuce_GUI() juce_MessageManager.cpp:507
juce::ScopedJuceInitialiser_GUI::~ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:515
juce::ScopedJuceInitialiser_GUI::~ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:515
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2694
-- redacted multiple lines -- 
main 0x0000000104bd6a58
start 0x00000001801090e0

Back to the problematic commit, I added some extra breakpoints to:

MainPluginProcessor::MainPluginProcessor()
MainPluginProcessor::~MainPluginProcessor()
MessageManager::~MessageManager()

Which results in the following order of execution:

MessageManager::MessageManager:

juce::MessageManager::MessageManager() juce_MessageManager.cpp:29
juce::MessageManager::MessageManager() juce_MessageManager.cpp:28
juce::MessageManager::getInstance() juce_MessageManager.cpp:51
juce::initialiseJuce_GUI() juce_MessageManager.cpp:499
juce::ScopedJuceInitialiser_GUI::ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:514
juce::ScopedJuceInitialiser_GUI::ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:514
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2673
-- redacted multiple lines -- 
main 0x0000000100f7aa58
start 0x00000001801090e0

TimerThread::TimerThread:
(as a result of instantiating MainPLuginProcessor)

juce::Timer::TimerThread::TimerThread() juce_Timer.cpp:34
juce::Timer::TimerThread::TimerThread() juce_Timer.cpp:33
std::construct_at[abi:v160006]<…>(juce::Timer::TimerThread *) construct_at.h:38
std::allocator_traits::construct[abi:v160006]<…>(std::allocator<…> &, juce::Timer::TimerThread *) allocator_traits.h:304
std::__shared_ptr_emplace::__shared_ptr_emplace[abi:v160006]<…>(std::allocator<…>) shared_ptr.h:284
std::__shared_ptr_emplace::__shared_ptr_emplace[abi:v160006]<…>(std::allocator<…>) shared_ptr.h:276
std::allocate_shared[abi:v160006]<…>(const std::allocator<…> &) shared_ptr.h:995
std::make_shared[abi:v160006]<…>() shared_ptr.h:1004
juce::SharedResourcePointer::Weak::lockOrCreate() juce_SharedResourcePointer.h:156
juce::SharedResourcePointer::SharedResourcePointer() juce_SharedResourcePointer.h:172
juce::SharedResourcePointer::SharedResourcePointer() juce_SharedResourcePointer.h:90
juce::Timer::Timer() juce_Timer.cpp:293
MainPluginProcessor::MainPluginProcessor() MainPluginProcessor.cpp:41
MainPluginProcessor::MainPluginProcessor() MainPluginProcessor.cpp:46
createPluginFilter() MainPluginProcessor.cpp:552
juce::createPluginFilterOfType(juce::AudioProcessor::WrapperType) juce_CreatePluginFilter.h:35
AAXClasses::getPlugInDescription(AAX_IEffectDescriptor &, const AAX_IFeatureInfo *) juce_audio_plugin_client_AAX.cpp:2589
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2682
-- redacted multiple lines -- 
main 0x0000000100f7aa58
start 0x00000001801090e0

MainPluginProcessor::MainPluginProcessor:

MainPluginProcessor::MainPluginProcessor() MainPluginProcessor.cpp:47
MainPluginProcessor::MainPluginProcessor() MainPluginProcessor.cpp:46
createPluginFilter() MainPluginProcessor.cpp:552
juce::createPluginFilterOfType(juce::AudioProcessor::WrapperType) juce_CreatePluginFilter.h:35
AAXClasses::getPlugInDescription(AAX_IEffectDescriptor &, const AAX_IFeatureInfo *) juce_audio_plugin_client_AAX.cpp:2589
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2682
-- redacted multiple lines -- 
main 0x0000000100f7aa58
start 0x00000001801090e0

MainPluginProcessor::~MainPluginProcessor:

MainPluginProcessor::~MainPluginProcessor() MainPluginProcessor.cpp:120
MainPluginProcessor::~MainPluginProcessor() MainPluginProcessor.cpp:119
MainPluginProcessor::~MainPluginProcessor() MainPluginProcessor.cpp:119
std::default_delete::operator()[abi:v160006](juce::AudioProcessor *) const unique_ptr.h:65
std::unique_ptr::reset[abi:v160006](juce::AudioProcessor *) unique_ptr.h:297
std::unique_ptr::~unique_ptr[abi:v160006]() unique_ptr.h:263
std::unique_ptr::~unique_ptr[abi:v160006]() unique_ptr.h:263
AAXClasses::getPlugInDescription(AAX_IEffectDescriptor &, const AAX_IFeatureInfo *) juce_audio_plugin_client_AAX.cpp:2661
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2682
-- redacted multiple lines -- 
main 0x0000000100f7aa58
start 0x00000001801090e0

MessageManager::~MessageManager:

juce::MessageManager::~MessageManager() juce_MessageManager.cpp:42
juce::MessageManager::~MessageManager() juce_MessageManager.cpp:36
juce::deleteAndZero<…>(juce::MessageManager *&) juce_Memory.h:40
juce::MessageManager::deleteInstance() juce_MessageManager.cpp:65
juce::shutdownJuce_GUI() juce_MessageManager.cpp:508
juce::ScopedJuceInitialiser_GUI::~ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:515
juce::ScopedJuceInitialiser_GUI::~ScopedJuceInitialiser_GUI() juce_MessageManager.cpp:515
GetEffectDescriptions(AAX_ICollection *) juce_audio_plugin_client_AAX.cpp:2694
-- redacted multiple lines -- 
main 0x0000000104f9ea58
start 0x00000001801090e0

btw: I never hit the assert inside Timer::startTimer:

// 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
6

I think I know what is happening and how to reproduce. The clue lies in the fact that I’m calling juce::Timer::callAfterDelay from inside my MainPluginProcessor::MainPluginProcessor constructor like so:

juce::WeakReference weakThis (this); 
juce::Timer::callAfterDelay (100, [weakThis] {
    if (weakThis == nullptr)
        return;

    // Do some work
});

What happens is this:

  • As a result of calling juce::Timer::callAfterDelay a new LambdaInvoker instance gets created (using new, ahem).
  • This LambdaInvoker inherits from juce::Timer and deletes itself when the Timer calls timerCallback().
  • However, before the MessageManager gets a chance to call timerCallback() on the LambdaInvoker instance, it gets destroyed.
  • Now the LambdaInvoker is leaked and since LambdaInvoker is a subclass of Timer it keeps the instance of SharedResourcePointer<TimerThread> alive.
  • As a result of this, Timer, SharedResourcePointer<TimerThread> and TimerThread never get deleted.

The commit in itself is not wrong, but where previously the TimerThread would be forcibly destroyed at shutdown, now it stays alive due to the memory leak. The memory leak probably has been there for a while, at least in this specific case with Pro Tools.

2 Likes
7

Amazing thanks I’ll try and get a patch together for this. I’ve got a couple of ideas.

8

Oh, wow - I do a very similar thing. Not using develop at the moment - thanks for finding this and saving me a potential major headache!!!

9

The leak caused issue couple of month ago already where I had some crashes in Live

  class CallAfterDelay : public juce::Timer
  {
  public:
    CallAfterDelay(int ms, std::function<void()> callback_)
    : callback(callback_)
    {
      startTimer(ms);
    }
    
    void timerCallback() override
    {
      stopTimer();
      callback();
    }
  protected:
    std::function<void()> callback;
  };

in my app, I instantiate a unique_ptr of CallAfterDelay in my plugin ctor to avoid the issue.

10

I think this problem also exists in currently released versions. The reason that this was exposed on develop is that the Timer logic has been changed to use juce::SharedResourcePointer<TimerThread> instead of DeletedAtShutdown.

11

Hmmm. Well, I hadn’t noticed any issues so far, so I still suspect it was an issue waiting to happen for me. Anyway, thanks for your detailed bug-hunting!

1 Like
12

I’ve pushed a change to prevent the memory leak

I have another change in an internal MR that should help further but potentially the above is enough if the leak was the cause of the issue for you?

4 Likes
13

Thanks!

I can confirm that the tooltips are working again inside Pro Tools with this commit. I think it’s safe to assume that the leak will have been solved.

Thank you for your quick and adequate response!

2 Likes