How to stop the thread if it's running?

CPallini · modified on Thursday, April 30, 2009 4:50 AM

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

David Crow

mikert_2008 wrote:

This I will put in the destructor of class (which starts the thread)...

You're starting a thread from within the destructor? :confused:

"Old age is like a bank account. You withdraw later in life what you have deposited along the way." - Unknown

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons

David Crow

You need two CEvent objects, one representing the request from the primary thread to stop, and the other representing the acknowledgment from the secondary thread that it has stopped. When the primary thread responds to a stop request (e.g., Alt+F4, X, Cancel), it signals the first event and then waits for the second event. In the secondary thread, call WaitForSingleObject() on that event with 0 as the wait time. If it comes back as signaled, signal the other event.

"Old age is like a bank account. You withdraw later in life what you have deposited along the way." - Unknown

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons