|
Thanks for the link given.
So now I used it like following.
CWinThread * myWorkerThread;
myWorkerThread = AfxBeginThread(run, this);
In ~Destructor()
{
DWORD result =WaitForSingleObject(myWorkerThread->m_hThread,0);
if(result == WAIT_OBJECT_0)
delete myWorkerThread;
}
Is this is the correct apporch to do this?
Mike
|
|
|
|
|
No, because you don't stop the thread. So, how can it stop ?
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.
But I guess everything is described in the article Carlo gave to you.
|
|
|
|
|
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 WaitForWhatever 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
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
As a side note to Cédric reply, ponder the usage of the m_bAutoDelete flag. Bottom line: read carefully the article I linked.
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]
|
|
|
|
|
You need not (and in most cases, MUST NOT) be deleteing a CWinThread object. It cleans up its own mess, unless you change the default value of m_bAutoDelete variable before the thread starts. And if you're thinking of changing the value of m_bAutoDelete , I can almost confidently tell that your approach is wrong (it is so, in 90% of the cases). If you knew what m_bAutoDelete is, you won't be asking here on how to shut down a thread gracefully.
Fixed typo!
It is a crappy thing, but it's life -^ Carlo Pallini
modified on Thursday, April 30, 2009 4:50 AM
|
|
|
|
|
|
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
|
|
|
|
|
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... 
"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown
|
|
|
|
|
Nah, it looks like I need some coffee as well. Modified my post such that it makes clear sense. 
It is a crappy thing, but it's life -^ Carlo Pallini
|
|
|
|
|
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]
|
|
|
|
|
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
|
|
|
|
|
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
|
|
|
|
|
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
|
|
|
|
|
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.
"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown
|
|
|
|
|
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.
It is a crappy thing, but it's life -^ Carlo Pallini
|
|
|
|
|
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.
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]
|
|
|
|
|
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 wrote: If I recall well, that isn't the whole story, The CWinThread pointer is deleted too.
You recall correctly! 
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
|
|
|
|
|
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]
|
|
|
|
|
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
|
|
|
|
|
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.
|
|
|
|
|
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;
pThread = AfxBeginThread( run, this, 0, 0, CREATE_SUSPENDED, NULL );
if( pThread )
{
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 )
{
if( ::WaitForSingleObject( pThread->m_hThread, INFINITE ) == WAIT_OBJECT_0 )
{
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.
Believe me, I've suggested Joe's article to fellow programmers since he wrote it almost a decade ago. In my opinion it's the best article that can be found on the internet describing multithreading, especially together with MFC framework.
"It's supposed to be hard, otherwise anybody could do it!" - selfquote
"High speed never compensates for wrong direction!" - unknown
|
|
|
|
|
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? 
"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
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
![Java | [Coffee]](https://codeproject.freetls.fastly.net/script/Forums/Images/coffee.gif)