How to stop the thread if it's running?

[Rajesh R Subramanian]

Cedric Moonen wrote:

Usually, to stop a thread you set a flag to false and this flag is constantly checked within your thread function. Once it is false, the function exits, which stops the thread.

I'll be surprised if you are doing such a thing. What about the usage of Events with WaitFor_Whatever_ functions? Polling for a boolean is not the best approach, IMO, and definitely shouldn't be suggested to a rookie programmer. Not to mention that you will need to then explain the OP what optimization is and why does the optimizer have to be prevented from optimizing this particular boolean variable, and then there comes volatileness, and ...

It is a crappy thing, but it's life -^ Carlo Pallini

[Cedric Moonen]

Yes, sure. But even if you are using events to signal your thread that "data is available for processing", you still need a way to tell the thread that it should stop. Otherwise, having a thread that just wait on an event and stops when the event is signaled is a bit pointless. But as I said in my message, it was a rough advice as everything is clearly explained in the article. My point was that you need a way to tell the thread that it's job is done, and usually you do this using a flag.

Cédric Moonen Software developer
Charting control [v2.0] OpenGL game tutorial in C++

[Rajesh R Subramanian]

At times, it gets ugly with a boolean and I use two events, and two WaitFor... functions one after another. The first one checks if the thread should shut down, and the second one checks if there's data for processing and proceeds if there is. Based on the requirements, the events may be manual or auto reset. Threads are a fascinating thing, aren't they? :) My reply was based on an assumption that you used a boolean for both. I was not kinda terrified.

It is a crappy thing, but it's life -^ Carlo Pallini

[Roger Stoltz]

Rajesh R Subramanian wrote:

And if you're thinking of setting m_bAutoDelete to TRUE, I can almost confidently tell that your approach is wrong (it is so, in 90% of the cases).

Quite the contrary Rajesh. In order to wait for the thread the thread handle must be valid and since the CWinThread destructor closes the handle. This issue is also addressed in the article. Forget it! :-\ Need more caffeine in the morning so I can read and distinguish between the letters in TRUE and FALSE. :java::java::java::java:

"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown

[Rajesh R Subramanian]

Well, I am not contradicting the article, but most code that I've reviewed through, especially written by inexperienced programmers, which were reported 'not working' and had memory leaks, had the default value of m_bAutoDelete was manually changed for absolutely no sane reasons. Therefore my comment. I remember having posted one of such horrible codes in the coding horror board (or at thedailywtf), if I'm right.

It is a crappy thing, but it's life -^ Carlo Pallini

modified on Thursday, April 30, 2009 4:48 AM

[Roger Stoltz]

I know, I know, I know..... I've modified my previous post even before you replied. You were right in first place Rajesh. I jumped to conclusions from your discussion with Cédric. I'll just put a sock in it now... :-O

"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown

[CPallini]

Rajesh R Subramanian wrote:

It cleans up its own mess, unless you set the m_bAutoDelete variable to TRUE before the thread starts.

Is quite the opposite, I suppose. Am I wrong?

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[Rajesh R Subramanian]

The issues with what you've done are: 1. You haven't shown us what you're doing inside the 'run' function. (can you please rename it to something other than run, I'm just terrified looking at the context - CWinThread::Run() ... you get the picture) 2. Your WaitFor... call does not 'wait', it just performs a check on a thread which might be running or might be dead, and then attempts to delete something which (again) may have been already dead (you haven't set the m_bAutoDelete to TRUE). You may first try to run a thread successfully, and then try using Events with WaitFor... functions. Simulate processing with Sleep(10) within a loop (for learning purposes) and then shut it down gracefully by setting an Event. You need not 'delete' the CWinThread object, like I said. Give that excellent article another read, and you will be all set. Post a reply, if you're stuck somewhere.

It is a crappy thing, but it's life -^ Carlo Pallini

[Rajesh R Subramanian]

You are FRAKKIN' RIGHT! It was a typo, but now fixed it to something that cannot go wrong. :)

It is a crappy thing, but it's life -^ Carlo Pallini

[Rajesh R Subramanian]

Nah, it looks like I need some coffee as well. Modified my post such that it makes clear sense. :-D

It is a crappy thing, but it's life -^ Carlo Pallini

[Roger Stoltz]

CPallini wrote:

Rajesh R Subramanian wrote:

It cleans up its own mess, unless you set the m_bAutoDelete variable to TRUE before the thread starts.

Is quite the opposite, I suppose. Am I wrong?

Just to be clear: If CWinThread::m_bAutoDelete is set to TRUE, the destructor closes the thread handle. To be able to wait on the thread handle from another thread m_bAutoDelete must be set to FALSE.

"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown

[Rajesh R Subramanian]

I think Carlo was talking with regards to be able to manually 'delete' the CWinThread object, which can be done with the m_bAutoDelete member set to FALSE. :)

It is a crappy thing, but it's life -^ Carlo Pallini

[Roger Stoltz]

Rajesh R Subramanian wrote:

I think Carlo was talking with regards to be able to manually 'delete' the CWinThread object

I understood it differently. I think he's talking about that your sentence can be interpreted as "if m_bAutoDelete is set to TRUE, CWinThread will not clean up its own mess". You've got a way with words Rajesh. :-D

"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown

[Rajesh R Subramanian]

Roger Stoltz wrote:

I think he's talking about that your sentence can be interpreted as "if m_bAutoDelete is set to TRUE, CWinThread will not clean up its own mess".

It occurred to me as well, but I went through the whole conversation again and finally found out something that made sense. Now, let's hope Carlo doesn't come up to tell us he was actually talking about Pasta and Rosemary. :-D

It is a crappy thing, but it's life -^ Carlo Pallini

[CPallini]

If I recall well, that isn't the whole story, The CWinThread pointer is deleted too. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[CPallini]

Rajesh R Subramanian wrote:

Carlo doesn't come up to tell us he was actually talking about Pasta and Rosemary.

Nope, you should know, I was actually talking about pizza 'napoli' and beer. :-D

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[Roger Stoltz]

CPallini wrote:

If I recall well, that isn't the whole story, The CWinThread pointer is deleted too.

You recall correctly! :thumbsup: There's a delete this; in the destructor as well that is executed if m_bAutoDelete is set to TRUE. I omitted it for brevity since I considered the name of the variable m_bAutoDelete rather self-explanatory. ;) But the only thing the OP need is to read Joe's article over and over again until it's understood. Really.

"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown

[CPallini]

Roger Stoltz wrote:

But the only thing the OP need is to read Joe's article over and over again until it's understood. Really.

"Softly spoken magic spells". :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[mikert_2008]

1. Run is simple displaying function. which has nothing to do with CWinThread::Run(). Now it looks like below... CWinThread * myWorkerThread; myWorkerThread = AfxBeginThread(run, this); myWorkerThread->m_bAutoDelete = TRUE; UINT run(LPVOID pParam) { .......... ............ // display the data } In ~Destructor() { DWORD result =WaitForSingleObject(myWorkerThread->m_hThread,0); if(result == WAIT_OBJECT_0) delete myWorkerThread; } 2. In WaitFor... I have given time 0 as I require only to check the status of the thread.

[Roger Stoltz]

mikert_2008 wrote:

CWinThread * myWorkerThread;
myWorkerThread = AfxBeginThread(run, this);

myWorkerThread->m_bAutoDelete = TRUE;

In all the confusion from the discussion earlier today you're partly forgiven, but consider the following for spawning the worker thread:

CWinThread* pThread;
// Create thread suspended to avoid race conditions
pThread = AfxBeginThread( run, this, 0, 0, CREATE_SUSPENDED, NULL );

// Make sure the thread could be created and
// the pointer is valid before operating on it
if( pThread )
{
// Don't let the object delete itself since we want
// to be able to wait on the thread handle
// This is important and addressed in the article linked to by Carlo!
pThread->m_bAutodelete = FALSE;
pThread->ResumeThread();
}

What kind of event stops the worker thread from running? The way I interpret your destructor is that the thread does its job and then terminates by itself. What does the thread do? Your comment "display the data" worries me since you should not touch the GUI from a worker thread in order to avoid deadlocks. Probably you shouldn't be using multithreading at all for the problem you're trying to solve, since you seem to be doing thing sequentially and not in parallel. Your destructor doesn't really make sense. There's also a race condition that in most cases would generate an exception when you're trying to free memory that has already been freed since you haven't set m_bAutoDelete to FALSE. You're mixing manual and automatic deletion of the CWinThread object. You should also make sure all thread spawned by your program are terminated before allowing the program to exit. It's considered Best Practice because when you exit your program you don't have the power to do anything about it any longer. The most harsh way the code that waits for the thread to exit could be like this:

if( pThread )
{
// Here should probably be something that tells the thread
// to stop executing such as an event that is signalled

if( ::WaitForSingleObject( pThread->m\_hThread, INFINITE ) == WAIT\_OBJECT\_0 )
{
    // Now it's safe to delete the CWinThread object!
    delete pThread;
    pThread = NULL;
}

}

You need to get back to the article and read it again as the above suggests that you haven't read it carefully enough and apparently have not understood it properly. Bel