Timer::callAfterDelay() looping bug

General JUCE discussion
1

Not sure how anyone is using callAfterDelay().
In juce 8 the lambdaInvoker deletes itself upon first timerCallback(), and the destructor is supposed to stopTimer(). But in practice this never happens and the function is repeatedly called forever.

Here’s the fix which worked for me. Note that the stopTimer() MUST come before the NullCheckedInvocation::invoke():


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

    ~LambdaInvoker() final
    {
        stopTimer();
    }

    void timerCallback() final
    {
        stopTimer(); /// NEEDED OR TIMER REPEATEDLY FIRES
        NullCheckedInvocation::invoke (function);
        delete this;
    }

    std::function<void()> function;

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (LambdaInvoker)
};
1 Like
2

The suggested fix is pretty benign, but still…

I am unable to repro this issue, and were trying out a couple of things including twiddling the internal timeout values and making sure the lambda runs for a long time. So just to be sure

  • Is the issue present on develop?
  • Can you perhaps share an example project reproducing the issue?
  • If not, is there anything unusual about the code in question? Does the lambda run for a long time? How many milliseconds are you using for callAfterDelay?
3

This works for me. The big question is… what code is in the lambda you want to call after a delay?
Can your code potentially never finish or take longer than the delay time?

4

I use callAfterDelay() in a number of cases and have not noticed any issues after updating to JUCE 8 (now running 8.04).

5

I, too, use callAfterDelay() in several places in my code without issue (juce 8.0.6).

6

the callable code includes an alert via showOkCancelBox() which means execution time of the lambda is longer than the “after delay” value (1sec in this case).

So I guess it has the opportunity to fire another timer callback.

7

Oh I see. Alerts have changed in j8?
Standalone app with JUCE_MODAL_LOOPS_PERMITTED=1

Put this in your mainComponent constructor and you should see the issue:

  auto testLamb = [&] () {
        DBG("Lambda called");
        auto res = juce::AlertWindow::showOkCancelBox(juce::MessageBoxIconType::NoIcon, "STOP", "and wait");
    };
    j::Timer::callAfterDelay(1000, testLamb);
8

Ouch! I’m guessing that results in the message loop running from within the callback which would then recursively triggers the timer!

9

right… so is my suggested fix a fair solution? Or is it now fundamentally ‘bad’ to use modal alerts in callAfterDelay?

10

Modal dialog boxes, popup menus and similar have been considered bad practice generally in Juce for many years already, not just in delayed callbacks.

2 Likes
11

sure, in a plugin. But this is a standalone app - does the “bad” still apply?

12

Yes, because you’re still blocking the message thread. By default, the #define disabled them, and the comment discourages their use. It’s only kept as an option for backward compatibility with old code. New code should not use modal anything.

1 Like
13

apologies for being a smart ass, but the problem is not the “modality” but the blocking of the message thread via modal loops.

You can use “modal” dialogs, via asynchronic callbacks.

something like this

Component::SafePointer<Component> safePointer (associatedComponent);
AlertWindow::showOkCancelBox (AlertWindow::QuestionIcon,
                                "Update available",
                                "Download?",
                                "Yes",
                                "No",
                                associatedComponent,
                                ModalCallbackFunction::create (
                                    [this, safePointer] (int result)
                                    {
                                        if (safePointer != nullptr)
                                        {
                                            if (result == 1)
                                            {
                                                // download update 
                                            };
                                        };
                                    }));
1 Like
14

Sure, but I believe that around here the word has a more specific meaning, what with the process of convincing everyone to stop using modal dialogs that block the message thread.