Stopping a thread pool?

We’re seeing a problem that maybe someone here can give some guidance on?

We were having a major slowdown in our plugin, and in the DAWs, when our graphs were busy drawing, especially while scrolling (either by hand or when the transport was running and we were scrolling to keep up with the playhead). So, we added a thread pool with a lambda in the graph regeneration function to queue up its content updates and free up the DAW from waiting on us. This really freed up a lot of CPU, but introduced a problem.

If you do a bunch of scrolling or zooming, it looks like the queue can get backed up from the mouse movements generating these calls, and sometimes ends up with a delay in the last mouse movement’s graph regeneration of a second or more. If the user closes the plugin, that thread pool’s lambda sometimes is still executing when the plugin destroys the view component that contains that graph regeneration code. That leads to a crash on closing the plugin, because it access items that have been deleted since the job started.

We enter the lambda, the plugin closes, and we call removeAllJobs(true,1000), but the job doesn’t finish and exit until too late.

I’m wondering if there is a better way to end the job in this case, that will ensure that the lambda finishes before the rest of the plugin is destroyed (especially the Processor, which owns the data that is needed for generating the graph).

One idea I have would be to set that timeout on removeAllJobs() to 0 or something smaller, but I’m not sure if that will work in all cases or simply make the crash more rare.

Or maybe, since the graph only needs the latest data, perhaps we should clear the queue before adding a new job, so that the queue only ever has at most the current job and one about to start? To do that, I assume we’d remove all jobs except the running job (if there is one), then call addJob() for the call we’re making now? That seems like it would help, but I don’t know for sure if that will resolve the problem or not. I haven’t logged the number of jobs waiting to see if it’s growing unreasonably or not and that is causing the delay, but if so then this solution seems reasonable.

I am trying using removeAllJobs(true,50) just before the lambda starts, as a test.

I’m not sure what value to set for the timeout if I do removeAllJobs() before the lambda (or in our destructor, for that matter), but I’ve set it to 50 before the lambda. Not sure if we should shorten the time in that call in the destructor.

We also added a mutex and std::lock_guard on it just before calling removeAllJobs() in the destructor, and use this code at the top of the lambda to try to get a lock, in case the destructor gets called before we can stop the job:

std::unique_lock lock(mRemoveAllThreadsMutex, std::try_to_lock);
if (! lock.owns_lock())
  return;

Do these steps look like a good plan for guaranteeing it won’t crash due to the queue still running when the plugin is destroyed?

in the graph regeneration function ← is this a JUCE thing or something related to your app?

I think if it’s possible that your function will take that long you’ll not want to add to the threadpool through lambda functions but have an a dedicated ThreadPoolJob subclass – then you’ll gain access to the method: shouldExit()

Sprinkle shouldExit() around your threadpool job and return without finishing if It’s true. Hopefully it makes sense for your use case I’m not 100% sure what you’re describing but didn’t see this mentioned.

1 Like

^ this also sounds like an excellent option

We have a function the generates the contents of the graph in an offscreen graphics context, which simply gets copied to the drawing context inside paint (after which we add a few “overlay” items that draw over top of the graph contents, like a vertical bar showing the transport position at the last proceesBlock call).

I think the issue is that sometimes it takes longer than the speed at which we’re receiving mouse updates if the user scrolls the graph around really quickly, and that causes more than one job to queue up. From my testing so far, I no longer see jobs piling up, and there is no visible delay from the last mouse movement to the last log statement when exiting the llambda.

I’ll definitely keep the idea of using that shouldExit() call in my pocket, though. Might need to do that.

1 Like

+1 You can always wrap and send threadShouldExit() inside a lambda like, [&]()->bool { return threadShouldExit(); } to any other-object function calls.

Does that work with ThreadPoolJobs? Would love to be able to just construct them inline when needed, but the lack of exit checking made it impossible unless it’s really small stuff.

If you’d be willing to post a full example that’d be rad!

1 Like

It is just called slightly differently, but the concept is the same:

if (shouldExit()) 
    return juce::ThreadPoolJob::jobNeedsRunningAgain; // or jobHasFinished if you want to shut down anyway

// and somewhere else
job->signalJobShouldExit();

Ahh yes of course – but shouldExit() doesn’t exist when you add to the threadpool via:

    /** Adds a lambda function to be called as a job.
        This will create an internal ThreadPoolJob object to encapsulate and call the lambda.
    */
    void addJob (std::function<ThreadPoolJob::JobStatus()> job);

    /** Adds a lambda function to be called as a job.
        This will create an internal ThreadPoolJob object to encapsulate and call the lambda.
    */
    void addJob (std::function<void()> job);

Well, here is what I was trying to suggest in the context of ThreadPool,


JobStatus PresetRefreshJob::runJob()
{
    if(! shouldExit())
    {        
        // some code check and set refresh state here

        tree.refreshPresetDirs ([&]()->bool { return shouldExit(); });

        if(! shouldExit())
        {
            // some code to tell the main thread it should swap the trees
        }
    }

    // return accordingly
}
void Tree::refreshPresetDirs (std::function<bool()> scanShouldStop)
{
    // some code here

    for (auto& dir : dirs)
    {
        if(scanShouldStop())
            break;

        // some code to scan presets here
    }
}
1 Like

Love that! Thanks for the tip