🔒 Locked

Simple Threading Question

Started by Atrix256 Nov 27, 2011 at 11:02 PM 4 replies 1.9k views

Original Post

Nov 27, 2011 at 11:02 PM

Hi Guys,

I have a point in my program that a certain amount of work needs to be done, and main thread execution needs to block until the work is finished.

I have it working, but something about the way I'm doing it is making it lock up sometimes, so there must be a race condition or something.

Here's what I'm doing (pseudo code), can anyone see what's wrong with it, or have any suggestions for how to do it better? It's C++ / windows

Main Thread:

* Random game / engine logic
* Enter a critical section
* Set NumActiveThreads (a volatile unsigned int) to 8 (the number of worker threads i have)
* Leave the critical section
* While NumActiveThreads > 0, Sleep(0) to yield the thread (i do an enter critical section, read value, leave critical section each time)
* back to top

Worker Thread Function:

* While NumActiveThreads == 0, Sleep(0) to yield the thread (i do an enter critical section, read value, leave critical section each time)
* The thread does it's work
* I used an interlocked decriment on NumActiveThreads after the work that this thread is responsible for is done
* While NumActiveThreads != 0, Sleep(0) to yield the thread (i do an enter critical section, read value, leave critical section each time)
* back to top

The idea is that the worker threads sleep until they see NumActiveThreads != 0, then they wake up, do their work and decriment that number. The main thread sets NumActiveThreads to the number of threads, and then waits for that number to hit zero again.

Thanks for any help!

Hodgman

Nov 28, 2011 at 1:25 AM

The problem seems to me that as soon as all threads have finished, the main thread is allowed to start the next cycle. When that occurs, the main thread is allowed to overwrite the NumActiveThreads variable, however, some of the workers may still be sleeping and when they wake, they will read that variable to tell whether all workers have finished yet -- if the main thread has reset that variable, the worker will wait forever.To simplify the example, let's say that theres 2 worker instead of 8.

Main thread Worker 1 Worker2
 NumActiveThreads == 0, true:Sleep
 NumActiveThreads == 0, true:Sleep
 NumActiveThreads = 2
 NumActiveThreads > 0, true:Sleep 
 NumActiveThreads == 0, false:break
 NumActiveThreads == 0, false:break
 DoWork();
 DoWork(); 
 NumActiveThreads-- (==1)
 NumActiveThreads != 0, true:Sleep
 NumActiveThreads-- (==0)
 NumActiveThreads != 0, false:break
 NumActiveThreads > 0, false,break
 back to top:
 NumActiveThreads = 2
 NumActiveThreads != 0, true:Sleep

Worker 1 will never wake up, because while it was waiting for Worker 2 to finish, the main thread decided to start the next frame and overwrite the NumActiveThreads variable.

Hodgman

Nov 28, 2011 at 1:53 AM

So to suggest a solution, you need to ensure that every worker thread has exited it's "waiting for all workers to finish" state, before the main thread starts another cycle.

Off the top of my head, a fix might be to:
Have each worker wait for all workers to finish, and then increment a counter when it's done sleeping:
* ...
* interlocked decriment on NumActiveThreads
* While NumActiveThreads != 0, Sleep(0)
* interlocked increment on NumFinishedThreads

And have the main thread wait on the counter that is incremented after the workers have slept:
* ...
* While NumFinishedThreads < 8, Sleep(0)
* back to top

Atrix256

Nov 28, 2011 at 2:56 AM

Thanks for the reply Hodgman

I was tired of messing with it so while waiting for a reply I tried switching to using events and seem to have gotten good results with simpler code in the end

I create 2 events per thread, one is a "WakeUp" event and the other is a "GoingToSleep" event, so with 8 threads, there's 16 events total and all are auto resetting events.

It now works like...

Main Thread:

* Random game / engine logic
* Set each thread's "WakeUp" event.
* WaitForMultipleObjects on all of the "GoingToSleep" events, saying to wait infinitely for all of the events to be set
* back to the top

Worker Thread Function:

* WaitForSingleObject to wait infinitely on this current thread's "WakeUp" event.
* The thread does it's work
* The thread signals it's own "GoingToSleep" event
* back to the top

iMalc

Nov 28, 2011 at 7:43 AM

Yeah that's now a very good way to do it IMHO. I've done exactly that several times before.

Atrix256

Nov 28, 2011 at 4:52 PM

Yeah that's now a very good way to do it IMHO. I've done exactly that several times before.

Great to hear thanks iMalc!

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.