U++ forum: Welcome to the forum

Search on this site

Search in forums

Home » U++ Library support » U++ Core » Thread::ShutdownThreads not safe

Show: Today's Messages :: Show Polls :: Message Navigator
E-mail to friend

Thread::ShutdownThreads not safe [message #26951]

Sun, 13 June 2010 15:40

hojtsy
Messages: 241
Registered: January 2006
Location: Budapest, Hungary

Experienced Member

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

Report message to a moderator

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

Sun, 13 June 2010 17:54

mirek
Messages: 13975
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

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

Sun, 13 June 2010 20:03

tojocky
Messages: 607
Registered: April 2008
Location: UK

Contributor

luzr wrote on Sun, 13 June 2010 18:54

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

(Patch applied).

Mirek

Very nice observation!

Some time ago I had this issue and did not know what is the problem! My problem was, I try to shutdown thread and it is loop infinitely in some case.

Report message to a moderator

Re: Thread::ShutdownThreads not safe [message #26961 is a reply to message #26957]

Mon, 14 June 2010 09:01

mirek
Messages: 13975
Registered: November 2005

Ultimate Member

tojocky wrote on Sun, 13 June 2010 14:03

Very nice observation!

Some time ago I had this issue and did not know what is the problem! My problem was, I try to shutdown thread and it is loop infinitely in some case.

Very unlikely that proposed patch has in reality changed the generated code in any way... It is just cleaner code in case compiler changes in the future.

Mirek

Report message to a moderator

Previous Topic:	double equals nothing
Next Topic:	DeXml() incomplete

Goto Forum:

-=] Back to Top [=-

[ Syndicate this forum (XML) ] [

] [

]

Current Time: Tue Apr 16 23:58:26 CEST 2024

Total time taken to generate the page: 0.04592 seconds