Can't Stop Thread

Hi guys, I can't stop my thread:

Code:

---

void MyClass:start()
 DWORD dwThread;
 thread_flag=1;
 hThread=CreateThread(NULL,0,startThread,(LPVOID)this,0,&dwThread);
}

DWORD WINAPI MyClass::startThread(LPVOID params)
{
 MyClass* p=(MyClass*)params;
 return p->myThread();
}

DWORD MyClass::myThread()
{
 while(thread_flag != 0){
  // do stuff
 }
 return 0;
};

void MyClass::stop(){
 // I try to stop it like this but it just hangs
 thread_flag=0;
 WaitForSingleObject(hThread,INFINITE);
 CloseHandle(hThread);
}

---

What am I doing wrong ? Thanks.

When two or more threads access the same memory location where at least one thread is modifying the memory, then all accesses must be synchronized using primitives from your threading library.

In this case, I use a manual reset Win32 Event object. The thread loop is then:

Code:

---

while (WaitForSingleObject(hExitEvent, 0) == WAIT_TIMEOUT)
{
}

---

gg

To carry on with CodePlug's comment, instead of setting the thread_flag to zero in the stop method, you would call SetEvent(hExitEvent);

Even with using an event and WaitForSingleObject, you may still have an issue - if your loop doesn't make it around to the start of the while loop, the thread won't exit.

Put some trace statements in the thread proc and make sure the thread exits the while loop.

Lastly, check out using _beginthreadex over using CreateThread.

It seems like the more I think about thread synchronization the more I think just about everything needs to be protected.

I have a custom window (made with CreateWindow) that I use like a console...

Code:

---

DWORD MyThread()
{
 int value;
 char s[64];
 sprintf(s,"value: %d",value);

 m_console->out(s);
}

---

I'm thinking that can cause problems. Will it interfere with the console's WM_PAINT and other messages? Do I have to only update the console in a critical section (or something similar)? The console is a child of the main window. I really don't understand what needs to be protected and what doesn't.

When dealing multi-threaded apps, you generally need to protect any resources that are shared between threads. There are exceptions to this, such as shared read-only data, but for the most part, it holds true.

To understand why you need to protect your out method running on the UI thread, consider what happens if two threads call the out method. If thread 2 writes "ABCDEFGHI..." and thread 3 writes "123456789...", the out method (running in thread 1) could very well display something unexpected such as "ABC123D45EFG6HI789" due to context switching inside the thread scheduler works.

In terms of synchronization, you can add a critical section as a member of the console class and then lock/unlock it inside the out method. Depending on how often you are calling the out method inside the threads and how much data you are writing, you may delay the processing of windows messages of the main thread. If that is the case, you might want to explore other approaches.

Quote:

---

Originally Posted by endemoniada

I have a custom window (made with CreateWindow) that I use like a console...

Code:

---

DWORD MyThread()
{
 int value;
 char s[64];
 sprintf(s,"value: %d",value);

 m_console->out(s);
}

---

---

What is m_console and what is out(...)? You will get much more useful replies if you post minimally complete example code rather than meaningless code snippets.

When you want to communicate to a window from a worker thread, you should only post messages to the window, otherwise you may end up with a dead-lock. This means that you either need to dynamically allocate memory for the string you want to pass (if the messages are infrequent), or use a synchronized queue to hold the messages. The latter works better for frequent messages, because you shouldn't rely on the order in which messages will be handled by the message loop. You'll still need to post a message to the window to tell it to read more data from the queue.

The other problem you're got is that thread_flag should be defined as volatile.

Code:

---

volatile thread_flag=1;

---

Not withstanding anything other posts have said, when you have a variable like this that can be changed outside of the thread you need to inform the compiler that it cannot optimse use of the variable. In your while loop, the compiler may well have put the value of thread_flag into a register and never updates the register again, So it will always appear to be 1 irrespective of any changes made to it outside of the thread! Using volatile tells the complier to always get the value from memory when it is referenced.

See Richter, Windows via c/c++ chapter 8. (http://www.amazon.co.uk/Windows-via-...1798755&sr=1-1)

Marking a variable as volatile guarantees nothing. 'volatile' is a compiler hint only, and implementation is compiler dependant.

Make sure you know how it works for your compiler before you use it. If you want guarantees and/or portability, you need to use actual synchronisation.

for proper use in multithreading, just guaranteeing an actual read (or write) is not enough, you also need guarantees that the compiler won't optimize/reorder code around the volatile read and/or write. And even that does not necessarily guarantee the exact behaviour you expect on a multi core machine.