U++ forum: Welcome to the forum

Re: Thread::ShutdownThreads not safe [message #26953 is a reply to message #26951]

Sun, 13 June 2010 17:54

mirek
Messages: 14267
Registered: November 2005

Ultimate Member

hojtsy wrote on Sun, 13 June 2010 09:40

Hi,

I think the implementation of Thread::ShutdownThreads() and Thread::IsShutdownThreads() is not thread-safe:

static Atomic  sShutdown;

void Thread::ShutdownThreads()
{
	AtomicInc(sShutdown);
	while(sThreadCount)
		Sleep(100);
	AtomicDec(sShutdown);
}

bool Thread::IsShutdownThreads()
{
	return sShutdown;
}

I believe that the correct implementation would be:

static volatile Atomic  sShutdown = 0;

void Thread::ShutdownThreads()
{
	AtomicInc(sShutdown);
	while(AtomicRead(sThreadCount))
		Sleep(100);
	AtomicDec(sShutdown);
}

bool Thread::IsShutdownThreads()
{
	return AtomicRead(sShutdown);
}

Could you please look into this, and fix if I am correct.
Thanks,
- Sandor

Well, yeah, adding volatile is definitely a good idea.. AtomicRead... in fact, all CPUs I have ever studied have that equal to normal read (for Atomic values anyway), but I guess if we ever introduced it, we should be using it, right? Smile

(Patch applied).

Mirek

Report message to a moderator