Thread safety and queues

General JUCE discussion
1

So imagine i have a Thread processing a queue and a function addJob for adding work to the queue. When the queue runs out of work, so i stop the thread to free up resources.

Imagine addJob(…) is implemented like:

void addJob(void * something)
{
    queue.add(something);
    startThread(5);
}

Imagine the thread run() method is:

void run() override
{
   while (queue.empty())
    processQueue();

  // execution could reach here when addJob(...) is next called
}

What’s the neatest way of ensuring the thread always proceses stuff but can exit when it is no longer needed?

In this simple example addJob could be called while the thread is running but is no longer looping and processing the queue. Then I assume startThread will do nothing and the queue will contain an item that may never get processed.

2

I’d start the thread once and let it wait for a notification. It will take up nearly no resources when it’s waiting for a notification. In contrast to that, starting and stopping the thread is a relatively heavy operation.

void addJob (void * something)
{
    queue.add (something);
    notify();
}

void run() override
{
    while (! threadShouldExit())
    {
        while (! queue.empty())
            processQueue();

        wait (-1);
    }
}
1 Like
3

We have quite a lot of threads already. And this is somethign that might happen once a year and doesn’t need to happen fast. So I was thinkign we’d spin the thread up when we needed it.

4

When I say we have ‘quite a lot of threads’. I’m running about 160 at the moment and keen not to make the problem worse :slight_smile:

5

Wouldn’t be ThreadPoolJob be an option?

That kind of does what you describe.

6

This is one of those threads: How do I do A. Don’t do A, do B :wink:

I don’t think ThreadPool solves it. It doesn’t shut the thread down when it’s not being used. And if we get into having a separate ThreadPool shared between other similar requirements it gets pretty complicated and I have a whole different set of problems!

7

Sorry, was just asking…

2 Likes
8

It’s fine Dan :wink:

9

Dikdn’t mean to sound like i was jumping down your throat about your otherwise potentially helpful suggestion :slight_smile:

10

Have you tried using locks? If you acquire a lock after the while loop in your run method and also tried to acquire it in your addJob it should prevent that situation?

Although I guess there’s still a micro chance of a job being added between the end of the while loop and the lock being gained…

11

Actually, I’ve hit on a fairly simple solution but it feels like a bit of a bodge. At the end of the run() method to start a timer and then:

    void timerCallback() override
    {
     if (queue.isEmpty())
            stopTimer();
       else if (! isThreadRunning())
            startTimer (5);
12

No worries, all good :slight_smile:

14

Is the overall number of elements to be added to the queue known on the writer side when adding the first and thus starting the thread? Then why not store that value into a variable and let the thread decrement it to make sure to not exit the run method before it reached 0?

15

No, it’s random UI elements that might need to add jobs to the queue. The number of requests we will get is unknown, related to user mouse clicks!

16

I think to safely enqueue new jobs to your queue you need to guard the queue with a lock anyway.

If you acquire the lock when you add a job and call startThread (it’s a NOP if it’s already running) and if you acquire the lock when you finish a job and pop it, you can safely check if the queue is empty and stop it.

Now there is no chance that the queue is not empty and not running.

17

At some point during the closing of the thread you reach a moment where the thread is still running, but the lock isn’t held, the user can click addJob and add a Job but have startThread fail to restart the thread, no?

18

addJob has to do:

  • acquire the lock
  • push the job to the queue
  • start the thread if it’s not already running
  • the queue count is now guaranteed to be > 0

when a job finishes it has to do:

  • acquire the lock
  • pop the finished job → queue count goes down
  • if the queue is empty stop the thread

add job and job cleanup are now synchronised and the queue is thread safe

19

addJob has to do:

  • acquire the lock
  • push the job to the queue
  • start the thread if it’s not already running
  • the queue count is now guaranteed to be > 0

when a job finishes it has to do:

  • acquire the lock
  • pop the finished job → queue count goes down
  • if the queue is empty stop the thread

add job and job cleanup are now synchronised and the queue is thread safe

The problem is ‘check the queue is empty and stop the thread’ isn’t an atomic operation.

20

When you add a job, you could check if the queue size is empty and then check if the thread is running… if the queue is empty but the thread is still running then addJob should wait until the thread has stopped to add a new job and start it up again.

void addJob(void * something)
{
    if (queue.empty())
    {
        while (isThreadRunning())
        {
            // Wait for thread to stop...
        }
    }

    queue.add(something);
    startThread(5);
}
21

Oh that’s interesting. Either it’s right, or it’s too hard to spot the problem with it :slight_smile:

Not a massive fan of potentially blocking the message thread, but otherwise that’s alright. Nice one.