Timer::callAfterDelay can cause leaks

1

This seems to have been a problem prior to the latest changes on develop:

It can be illustrated by this (admittedly contrived) example that leaks the CapturedObject prior to the changes (since I’m deliberately capturing by value):

#include <JuceHeader.h>

using namespace juce;

int main (int argc, char* argv[])
{
    ScopedJuceInitialiser_GUI _;

    struct CapturedObject
    {
        CapturedObject()  { Logger::outputDebugString ("CapturedObject ctor"); }
        ~CapturedObject() { Logger::outputDebugString ("CapturedObject dtor"); }

        CapturedObject (const CapturedObject& other)
        : message (other.message)
        {
            Logger::outputDebugString ("CapturedObject copy");
        }

        String message;
        JUCE_LEAK_DETECTOR (CapturedObject);
    };

    CapturedObject data;
    data.message = "Hello, World!";

    Timer::callAfterDelay (1000, [data]()
    {
        Logger::outputDebugString (data.message); // never happens, as expected
    });

    return 0;
}

The failure is much more obvious with the latest changes on develop since a bunch of JUCE objects leak in this case, even if captured by reference. The problem is just made more visible.

This issue can be caused in a real app/plugin if the delay time is long enough to exit the app/plugin before the delay triggers the lambda, so the contrived example isn’t just a silly edge case.

I wonder if the following is a suitable fix in JUCE Timer?

...
struct LambdaInvoker final : private Timer,
                             private DeletedAtShutdown
{
    LambdaInvoker (int milliseconds, std::function<void()> f)  : function (f)
    {
        startTimer (milliseconds);
    }

    void timerCallback() override
    {
        auto f = function;
        delete this;
        f();
    }

    std::function<void()> function;

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (LambdaInvoker)
};
...
2

Good catch! I would prefer if we could move away from DeletedAtShutdown if possible. I haven’t thought about this too hard but I think in this case there is a small possibility that DeletedAtShutdown could delete the object and then the timerCallback could still trigger, meaning undefined behaviour and potentially a double delete. I wonder if we could use use std::enable_shaed_from_this to help manage the memory of the LambdaInvoker instead :thinking:

3

Sounds like a plan!

It does expose some silent callbacks that weren’t happening due to these new explicitly-tracked leaks, rather than the silent leaks that were already there with the LambdaInvoker.

4

Would an explicit stopTimer() in the destructor help instead?

5

Not related to the OP:
Can you explain the need to move away from it and what’s the alternative?

In my own code, I’ve had to use DeletedAtShutdown for singleton-like objects to avoid hitting the leak detector when quitting, but I would very much like to find other solutions.

1 Like