Home » U++ Library support » U++ Core » Thread::ShutdownThreads not safe
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
|
|
|
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?
(Patch applied).
Mirek
|
|
|
Re: Thread::ShutdownThreads not safe [message #26957 is a reply to message #26953] |
Sun, 13 June 2010 20:03 |
|
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
|
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?
(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.
|
|
|
|
Goto Forum:
Current Time: Mon Apr 29 15:56:04 CEST 2024
Total time taken to generate the page: 0.02775 seconds
|