gamedevlists-windows

RE: [GD-Windows] Stopping a thread cleanly

[Brian H. <bri...@py...>]

> Which specific kind of synchronization are you doing here?

This is real primitive stuff.  Basically it's a background thread that's
filling a soundbuffer for streaming audio.  Works great, except for the
potential race condition that occurs when I need to shut down the
thread/streaming audio.  To kill the thread I was using a global
variable, which was a bit messy, but I've now switched to
WaitForSingleObject which is a lot cleaner.

I'm using both a critical section and the event -- the critical section
guards against access to the actual streaming buffer and DSound data,
the event is just to signal to the thread to quit.

Off the top of my head, my new loop is something like:

while ( 1 )
{
   //Guard access to the shared data used in DoWork()
   EnterCriticalSection();

   //Make sure that the event wasn't signaled prior to
   //entering the critical section
   if ( WaitForSingleObject( ... ) == WAIT_TIMEOUT )
   {
      DoWork();
      LeaveCriticalSection();
   }
   else
   {
      LeaveCriticalSection();
      break;
   }

   //Sleep for 250ms or until the event is signaled
   //indicating that the thread should die
   if ( WaitForSingleObject( event, 250 ) == WAIT_OBJECT_0 )
   {
      break;
   }
   //Theoretically a thread switch could occur between here
   //and entering the critical section, which is why
   //we check again inside the critical section for the
   //event
}

Brian

RE: [GD-Windows] Stopping a thread cleanly

[Brian H. <bri...@py...>]

> Somehow, you're giving this worker thread instructions, so I'd 
> suggest sending a shutdown message the same way you send other 
> commands to the thread.

Actually, I'm not giving it instructions -- it's completely autonomous
based on the parameter passed to it which is a pointer to the shared
data it needs to operate on.  It doesn't have any communication
mechanism with the main thread except for the synchronization objects.

> If the worker thread is supposed to asynchronously "pick up" work 
> every so often (sometimes necessary) then you should use the 
> time- out on the wait operation as your throttle for when to look for 
> more work. That way, you can signal this event (or whatever) when 
> it's time to exit, and the thread will immediately wake up and 
> exit, instead of you having to wait for however long the sleep 
> operation would take. You can even use the condition of "waiting 
> for this object did NOT return time-out" as your exit condition, 
> for a race condition free way of doing it :-)
 
> Finally, the most dangerous thing with exiting threads is having 
> other threads wait for those threads; unless you design carefully, 
> it's very easy to get into deadlock that way...

Yeah, that can be nasty, but thankfully it's only a single thread and
only one place kills it, basically via:

SetEvent( event );

while ( !gThreadDead ) ; //pray that gThreadDead actually gets set

DeleteSharedData();

Brian

RE: [GD-Windows] Stopping a thread cleanly

[Jon W. <hp...@mi...>]

> Actually, I'm not giving it instructions -- it's completely autonomous
> based on the parameter passed to it which is a pointer to the shared
> data it needs to operate on.  It doesn't have any communication

I don't see why a critical section would be necessary in that loop. 
There's no shared state going on between the player and the killer, 
except for the event, which is protected enough without the critical 
section.

Anyway, the problem with this is that you delay shut-down (while 
spinning, waiting for that global) for whatever your sleep time is. 
DirectSound supports playback notifier events, so you can set an 
event, say, every 100 milliseconds played, and use this event to 
wake up and fill the buffer. Or you can use your "shutdown" event 
with an appropriate time-out as your sleep thing, if you're OK 
with polling.

playerThread:

for(;;) {
  if( WaitForSingleObject( event, POLL_INTERVAL ) == WAIT_TIMEOUT ) {
    break;
  }
  lock buffer, read disk, unlock buffer
}

stopper:

  SetEvent( event );
  WaitForSingleObject( playerThread, INFINITE );


Regarding declaring members or globals "volatile" when they're shared 
between threads: that's what the keyword is for, so I don't see how 
that is considered ugly (assuming you need that semantic)?

Cheers,

			/ h+

RE: [GD-Windows] Stopping a thread cleanly

[Brian H. <bri...@py...>]

> In this case the compiler could put bExit in a register due 
> to optimisation which obviously isn't good. 

This is actually why this came up.  When I looked at my original code,
it was basically doing this:

while ( 1 )
{
...
if ( pData->bKill )
  break;
...
}

I was more than a little concerned that the compiler wasn't reloading
pData->bKill in some instances, because there were sections of code
where no function calls were occurring but bKill was being checked
multiple times.

I was going to do the knee jerk thing of specing it as volatile, but
decided to go with WFSO instead.

Brian

RE: [GD-Windows] Stopping a thread cleanly

[Brian H. <bri...@py...>]

> I don't see why a critical section would be necessary in that loop. 

It's a guard around the shared data accessed in DoWork().

> There's no shared state going on between the player and the killer, 
> except for the event, which is protected enough without the critical 
> section.

Actually, there is shared state -- in this case, they're both trying to
dork with an LPDIRECTSOUNDBUFFER.  The killer immediately releases it
when the thread is done.  I could have the thread do this, but sometimes
I just want to stop the thread without releasing the buffer.
 
> Anyway, the problem with this is that you delay shut-down (while 
> spinning, waiting for that global) for whatever your sleep time is. 

Which is why I'm WFSO now instead of just a pure sleep.

> DirectSound supports playback notifier events, so you can set an 
> event, say, every 100 milliseconds played, and use this event to 
> wake up and fill the buffer.

Except those don't work all the time.  Way back when, Candy Cruncher
used notifications, until I started getting E_NOINTERFACE failures on
some older drivers and sound cards.  Getting rid of those made my audio
a lot more robust.

> Regarding declaring members or globals "volatile" when they're shared 
> between threads: that's what the keyword is for, so I don't see how 
> that is considered ugly (assuming you need that semantic)?

I was originally going to declare the _class_ as volatile, which
suddenly breaks a lot of things (respecing volatile is much like
respecing const -- it ain't plug and play).  But just specing that one
member would have worked (and is all that's necessary).

Brian

RE: [GD-Windows] Stopping a thread cleanly

[Brian S. <bs...@mi...>]

> > In this case the compiler could put bExit in a register due
> > to optimisation which obviously isn't good.
>=20
> This is actually why this came up.  When I looked at my original code,
> it was basically doing this:
>=20
> while ( 1 )
> {
> ...
> if ( pData->bKill )
>   break;
> ...
> }
>=20
> I was more than a little concerned that the compiler wasn't reloading
> pData->bKill in some instances, because there were sections of code
> where no function calls were occurring but bKill was being checked
> multiple times.
>=20

The compiler should always reload it unless you've specified "assume no
aliasing" (which is not the default, even in optimized builds).  Since
it's not a local variable, it has to assume that pData->bKill can be
changed every time through the loop.

--brian

RE: [GD-Windows] Stopping a thread cleanly

[Jon W. <hp...@mi...>]

It only needs to assume this if you're calling any other function 
from within that loop. Of course, Sleep(), or WaitForSingleObject(), 
or pretty much anything else you might want to do in the loop does 
count as a function call (as long as it doesn't get inlined) but 
I thought I'd mention it.

volatile is your friend when you're using a shared-memory thread 
communication model.

Cheers,

				/ h+

> The compiler should always reload it unless you've specified "assume no
> aliasing" (which is not the default, even in optimized builds).  Since
> it's not a local variable, it has to assume that pData->bKill can be
> changed every time through the loop.

RE: [GD-Windows] Stopping a thread cleanly

[Brian S. <bs...@mi...>]

Function calls have nothing do with it.  "Assume aliasing across
function calls" is a separate setting.

This program has an aliasing problem, yet thread 2 has no function calls
in it.  See for yourself:

#include <windows.h>
#include <stdio.h>

int gTrueCount, gFalseCount;

DWORD WINAPI ThreadProc(LPVOID param)
{
	bool * fooPtr =3D (bool *)param;
	while (1)
	{
		if (*fooPtr)
		{
            gTrueCount++;
		}
		else
		{
            gFalseCount++;
		}
	}
}

int main(int argc, char **argv)
{
    gTrueCount =3D 0;
    gFalseCount =3D 0;
    bool foo =3D true;
    CreateThread(NULL, 0, ThreadProc, &foo, 0, NULL);
    while (1)
    {
        foo =3D !foo;
        printf("True count: %d False count: %d\n", gTrueCount,
gFalseCount);
        ::Sleep(1000);
    }
    return 0;
}

> -----Original Message-----
> From: Jon Watte [mailto:hp...@mi...]
> Sent: Monday, February 18, 2002 10:56 AM
> To: Brian Sharon; bri...@py...; gamedevlists-
> wi...@li...
> Subject: RE: [GD-Windows] Stopping a thread cleanly
>=20
>=20
> It only needs to assume this if you're calling any other function
> from within that loop. Of course, Sleep(), or WaitForSingleObject(),
> or pretty much anything else you might want to do in the loop does
> count as a function call (as long as it doesn't get inlined) but
> I thought I'd mention it.
>=20
> volatile is your friend when you're using a shared-memory thread
> communication model.
>=20
> Cheers,
>=20
> 				/ h+
>=20
> > The compiler should always reload it unless you've specified "assume
no
> > aliasing" (which is not the default, even in optimized builds).
Since
> > it's not a local variable, it has to assume that pData->bKill can be
> > changed every time through the loop.
>=20

RE: [GD-Windows] Stopping a thread cleanly

[Jon W. <hp...@mi...>]

> Function calls have nothing do with it.  "Assume aliasing across
> function calls" is a separate setting.
> 
> This program has an aliasing problem, yet thread 2 has no function calls
> in it.  See for yourself:

The difference in this program would be that I would no think the 
compiler incorrect to put the values in registers, thus you'd have 
to use the "volatile" qualifier. However, the compiler cannot 
correctly make any guesses about whether something on the other end 
of a pointer changes when you call a function with separate linkage 
(i e, in a separate translation unit).

Basically, my view is that the standard does not require the compiler 
to be thread, or signal, aware in its code generation, but it DOES 
require it to be conservative on aliasing. It's already the case 
that compilers generate thread-unsafe code for some constructs, such 
as initialization of statics with function scope.

Then, what specific compilers do with specific options, is another 
question (and ultimately, quite relevant :-)

Cheers,

			/ h+