WaitForSingleObject() failing?

[Davinder]

Hi
I am writing a Windows Explorer type SDI MFC Application.
This is a multi-threaded application.
All the threads created by me are worker threads.

Now there is a situation where one worker thread is still running & the main
app thread creates another worker thread.

I want to stop this.
The first thread will finish then only the second thread can start.

To achieve this i have used the function WaitForSingleObject() but this
function is failing judged by its return value.

The WaitForSingleObject() is used in the following fashion:

void CMainFrame::OnToolsSaveInformation()
{
if(pApp->FileSys)
{
pApp->pWorkerThread = AfxBeginThread(SaveScanInfo_WorkerThread,
(void*)pApp->MFC_Obj); // LINE 1

DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread,INFINITE); //
LINE 5

if(ret == WAIT_FAILED)
MessageBox(_T("Wait Failed"));
else
MessageBox(_T("Wait Success"));
}

pApp->pWorkerThread = AfxBeginThread(DoRestOfTheWork_WorkerThread,
(void*)pApp->MFC_Obj); // LINE 2
}

As evident from the above code 2 worker threads are created in Lines 1& 2.
In Line 5 WaitForSingleThread() is being called but it is failing(Wait
Failed msg is prompted.)

I donot understand y the function is failing.

Plz suggest the way out.

Best Regards

[Henrik Schmid]

By default, the thread object is automatically deleted and the handle closed
when the thread terminates.
Maybe WFSO failes, when the handle it waits on is closed.

See inline.

"Davinder" wrote:

> Hi
> I am writing a Windows Explorer type SDI MFC Application.
> This is a multi-threaded application.
> All the threads created by me are worker threads.
>
> Now there is a situation where one worker thread is still running & the main
> app thread creates another worker thread.
>
> I want to stop this.
> The first thread will finish then only the second thread can start.
>
> To achieve this i have used the function WaitForSingleObject() but this
> function is failing judged by its return value.
>
> The WaitForSingleObject() is used in the following fashion:
>
> void CMainFrame::OnToolsSaveInformation()
> {
> if(pApp->FileSys)
> {

Change this

> pApp->pWorkerThread = AfxBeginThread(SaveScanInfo_WorkerThread,
> (void*)pApp->MFC_Obj); // LINE 1

to
pApp->pWorkerThread = AfxBeginThread(SaveScanInfo_WorkerThread,
(void*)pApp->MFC_Obj,
THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
pApp->pWorkerThread->m_bAutoDelete = false;
pApp->pWorkerThread->ResumeThread();

>
>
> DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread,INFINITE); //
> LINE 5

Then you have to delete the CWinThread object:
delete pApp->pWorkerThread;

[Henrik Schmid]

I think the real problem is, that you are passing a pointer to a CWinThread
and not the thread handle to WaitForSingel Object.
Try:
DWORD ret = ::WaitForSingleObject(
pApp->pWorkerThread->m_hThread,INFINITE);

[Scott McPhillips [MVP]]

The first parameter to WaitForSingleObject is supposed to be the thread
handle (HANDLE), not a CWinThread pointer. You can use
pApp->pWorkerThread->m_hThread.

If the thread ends quickly you may also have the problem Henrik
describes. It is safer to create the thread with CREATE_SUSPENDED, then
do pWorkerThread->m_bAutoDelete = FALSE, then
pWorkerThread->ResumeThread(). This assures that the HANDLE remains
valid until after your WaitForSingleObject. Then you should delete
pWorkerThread.

--
Scott McPhillips [VC++ MVP]

[Walter Eicher]

Hi Scott,

On Wed, 04 Jan 2006 08:22:45 -0500, "Scott McPhillips [MVP]"
<org-dot-mvps-at-scottmcp> wrote:

> It is safer to create the thread with CREATE_SUSPENDED, then
>do pWorkerThread->m_bAutoDelete = FALSE, then
>pWorkerThread->ResumeThread().

Always when a problem with the lifetime of a thread comes up, these
three lines of code are posted as THE solution.

Why not simple and easy, and only one line of code?

UINT xy::ThreadProc (LPVOID foo)
{
AfxGetThread()->m_bAutoDelete = FALSE;

while (true)
{
// thread code
}
return 0;
}

I always use this solution and never had any problems.

I feel like walking on thin ice, because I never see my code as a
possible solution ;-)

Best Regards
Walter

[Michael K. O'Neill]

"Walter Eicher" <walter...@nospam.microcrystal.ch> wrote in message
news:mqlnr1h26guh557os...@4ax.com...

A lot can happen between the time that the main thread starts a worker
thread, and the time that the worker actually starts running (and executes
your proposed first line of code).

It's safer for the main thread to start the worker suspended, and then set
the m_bAutoDelete flag before anything more happens.

Just my understnading, anyway.

Mike

[Michael K. O'Neill]

"Davinder" <davi...@stellarinfo.com> wrote in message
news:er7iNmR...@TK2MSFTNGP14.phx.gbl...

You're calling WFSO right inside a (button?) handler for the main GUI
thread, and you're calling it with an INFINITE wait period. That's not
recommended, since it will freeze the app's main GUI.

Figure out some other mechanism for triggering the second thread. Perhaps
the first thread could start the second thread when it's finished?

Mike

[Doug Harrison [MVP]]

I think it should work, but it does distribute the responsibility in that
the thread function makes a crucial decision on behalf of the code which
created the CWinThread. I prefer to fix it near the AfxBeginThread line, so
I'll know it's done, rather than some distance away in the thread function,
where I'll have to look to know that it's done. If your convention is for
every thread function to do this, that's fine, but the problem is getting
everyone else to go along. :) I'd bet most every snippet involving
AfxBeginThread posted here does not refer to a thread function that knocks
out m_bAutoDelete, so it's also easier in a sense to fix it locally rather
than bring the thread function into the discussion.

--
Doug Harrison
Visual C++ MVP

[Scott McPhillips [MVP]]

Walter Eicher wrote:
> I feel like walking on thin ice, because I never see my code as a
> possible solution ;-)
>
> Best Regards
> Walter
>

Hi Walter,
Your solution looks reliable and equally valid to me. It's not as easy
to explain (especially in a newsgroup code snippet) as THE common
three-line solution. The three-line solution is local and atomic in a
sense, so it is less complex mentally and communicates better to a
maintainer.

[Joseph M. Newcomer]

See below...

On Wed, 4 Jan 2006 15:53:57 +0530, "Davinder" <davi...@stellarinfo.com> wrote:

>Hi
>I am writing a Windows Explorer type SDI MFC Application.
>This is a multi-threaded application.
>All the threads created by me are worker threads.
>
>Now there is a situation where one worker thread is still running & the main
>app thread creates another worker thread.
>
>I want to stop this.
>The first thread will finish then only the second thread can start.

****
So if the first thread is running, why not disable the operation that creates the thread?
This would be best.

If you don't want to do this, why not have a "thread pool" of one thread and queue up
requests to it, instead of creating threads on demand? Then as each computation finishes,
the next will be dequeued.

The WaitFor operation is essentially a really poor approach to the problem, and should be
avoided. It blocks the GUI thread, which is very bad style.
****

>
>To achieve this i have used the function WaitForSingleObject() but this
>function is failing judged by its return value.
>
>The WaitForSingleObject() is used in the following fashion:
>
>void CMainFrame::OnToolsSaveInformation()
>{
> if(pApp->FileSys)
> {
> pApp->pWorkerThread = AfxBeginThread(SaveScanInfo_WorkerThread,
>(void*)pApp->MFC_Obj); // LINE 1
>
>
> DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread,INFINITE); //
>LINE 5

****
This makes absolutely no sense whatsoever. Why would you block the main thread until a
secondary thread finishes? It completely defeats the purpose of having a secondary
thread!

In addition, you have many other errors here. Even your test below is poor style, because
there are many possible return values, and you assume that if it is not WAIT_FAILED it
must necessarily be a success, which should not be assumed.

There are some very serious failures here. For example, when a worker thread finishes,
the CWinThread object is implicitly deleted. But you are attempting to use it in the
WaitForSingleObject, which means that it could be completed, the object rendered invalid,
and the ::WaitFor pointing to meaningless garbage on the heap.

To deal with this, you would have to create the thread suspended, set the
pWorkerThread->m_bAutoDelete flag to FALSE, then do a ResumeThread to allow the thread to
run.

Also, why are you putting this in the app when it is managed by the mainframe? This style
of dumping global variables into the app is one of the worst practices I've seen. If the
mainframe manages it, it should be a member of the mainframe. If the app is managing it,
why is there code in the mainframe to deal with it?

A good policy is that if you ever have to write (CMyApp*)AfxGetApp, you've made a serious
coding blunder. This is one of the touchstones of a poorly-organized program. Somewhere,
somebody said "global variables are bad", so programmers apply syntactic sacharrine (not
as good as syntactic sugar) to global variables my simply moving them into the
CWinApp-derived class. This is identical to using global variables, but has the
disadvantages that it is clumsier to use, and requires the CWinApp-derived header file be
included (if you are including the project.h file for project the name of your app in any
but the CWinApp and CMainFrame classes, you have a structural error in your code. Alas,
Microsoft keeps dumping this declaration into various derived classes and I have to keep
removing it). If you need a global variable, make it a global variable. In the best of
organizations, each global variable has a pair of .h and .cpp files that define it, and
they are included ONLY in the modules that use them (no "globals.h" file, which is another
example of exceptionally poor programming style)
*****

>
> if(ret == WAIT_FAILED)
> MessageBox(_T("Wait Failed"));
> else
> MessageBox(_T("Wait Success"));
> }
>
>
> pApp->pWorkerThread = AfxBeginThread(DoRestOfTheWork_WorkerThread,
>(void*)pApp->MFC_Obj); // LINE 2

****
You have made one of the most common structural errors of asynchronous programming. You
are trying to pretend you live in a synchronous world. Since you think that if you want
to do A,B you would write
A;
B;
then if you have to do two threads, you have erroneously generalized this to
A;
wait for A to complete
B;
which makes no sense. The correct approach is:

A;
and that's all. THen when thread A completes, it does a PostMessage of a user-defined
message to the main GUI thread, e.g.,
wnd->PostMessage(UWM_A_COMPLETE);

then have a handler
LRESULT CWhatever::OnAComplete(WPARAM, LPARAM)
{
B;
return 0;
}

Windows is a fundamentally asynchronous, event-driven universe. Trying to apply retro
concepts of synchronous sequentiality are dooumed. You either get unusable code, or
undebuggable code, or unmaintainable code. What you need to do is always think of
asynchronous event-driven models. A, when it completes, sends an asynchronous
notification of its completion. Upon receipt of the notification, a thread B is created.
You can generalize this if you need to know when B finishes.
*****

>}
>
>As evident from the above code 2 worker threads are created in Lines 1& 2.
>In Line 5 WaitForSingleThread() is being called but it is failing(Wait
>Failed msg is prompted.)
>
>I donot understand y the function is failing.

****
You have fundamental errors in parallelism. You also have an unusable design. Rewrite it
as suggested.
****

>
>Plz suggest the way out.
>
>Best Regards
>
>
>
>
>

Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm

[Davinder]

Hi
Thanx everybody for the suggestions

Now i have given this statement:
DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread->m_hThread,INFINITE);

As a result of this the ::WaitForSingleObject() has started to work but
another problem has come up.

The ::WaitForSingleObject() has stopped both the threads - the worker thread
& the main application thread thus hanging
my application.

where as only the main app thread(calling thread) shud have stopped & not
the worker thread.

Why is the worker thread being stopped.

Waiting for ur suggestions

Regards

"Davinder" <davi...@stellarinfo.com> wrote in message
news:er7iNmR...@TK2MSFTNGP14.phx.gbl...

[Davinder]

I have changed the code.
plz have a look:

pApp->pWorkerThread =
AfxBeginThread(SaveScanInfo_WorkerThread,(void*)pApp->MFC_Obj,
THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);

pApp->pWorkerThread->m_bAutoDelete = false;
pApp->pWorkerThread->ResumeThread();

DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread->m_hThread,5000);

delete pApp->pWorkerThread;

As a result of the above code following has happened:

The ::WaitForSingleObject() has stopped both the threads - the worker thread
& the main application thread thus hanging
my application.
where as only the main app thread(calling thread) shud have stopped & not
the worker thread.
Why is the worker thread being stopped.

Waiting for ur suggestions

Regards

"Davinder" <davi...@stellarinfo.com> wrote in message
news:er7iNmR...@TK2MSFTNGP14.phx.gbl...

[Davinder]

So if the first thread is running, why not disable the operation that
creates the thread?
This would be best.

How do i disable the operation that creates yhe thread.

Waiting for suggestions

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message
news:p8kor1t6u0f8dngf5...@4ax.com...

[Walter Eicher]

Hi Michael,

On Wed, 4 Jan 2006 08:22:50 -0800, "Michael K. O'Neill"
<mikeat...@nospam.hotmail.com> wrote:

>A lot can happen between the time that the main thread starts a worker
>thread, and the time that the worker actually starts running (and executes
>your proposed first line of code).

I dont see any risk here, because m_bAutoDelete is only checked after
the thread terminates. It would be also possible to set m_bAutoDelete
just before the return statement.

Best Regards
Walter

[Walter Eicher]

Hi Doug,

On Wed, 04 Jan 2006 10:34:08 -0600, "Doug Harrison [MVP]"
<d...@mvps.org> wrote:

>I think it should work, but it does distribute the responsibility in that
>the thread function makes a crucial decision on behalf of the code which
>created the CWinThread. I prefer to fix it near the AfxBeginThread line, so
>I'll know it's done, rather than some distance away in the thread function,
>where I'll have to look to know that it's done. If your convention is for
>every thread function to do this, that's fine,

I never had a thread where m_bAutoDelete = TRUE made any sense. IMHO
the decision to set m_bAutoDelete to TRUE as default was wrong.

>but the problem is getting everyone else to go along. :)

Ok, 2 programmers 4 opinions ;-)

Best Regards
Walter

[Scott McPhillips [MVP]]

Davinder wrote:
> The ::WaitForSingleObject() has stopped both the threads - the worker thread
> & the main application thread thus hanging
> my application.
> where as only the main app thread(calling thread) shud have stopped & not
> the worker thread.
> Why is the worker thread being stopped.

Use the debugger 'Break' command and look at the worker thread stack to
find out where it has stopped.

One possibility is that your worker thread is accessing a window created
in the main thread. That would stop the worker thread until the main
thread processes messages. But it cannot process messages while in the
WaitForSingleObject call.

The best solution would to to get rid of the WaitForSingleObject call.
It defeats the purpose of doing work in a worker thread.

[Davinder]

Thanx for the suggestion
The worker thread updates the status bar pane 0.
So probably this is the reason y both threads are blocked.

I shud better avoid using ::WaitForSingleObject().

Now how to do i ensure that the second worker thread is not started untill
the first worker thread finishes his job.

waiting for suggestions

Best Regards

"Scott McPhillips [MVP]" <org-dot-mvps-at-scottmcp> wrote in message
news:uVi5ZdfE...@TK2MSFTNGP09.phx.gbl...

[Scott McPhillips [MVP]]

Davinder wrote:
> Thanx for the suggestion
> The worker thread updates the status bar pane 0.
> So probably this is the reason y both threads are blocked.
>
> I shud better avoid using ::WaitForSingleObject().
>
> Now how to do i ensure that the second worker thread is not started untill
> the first worker thread finishes his job.
>
> waiting for suggestions
>
> Best Regards

The easy and obvious solution would be to put both jobs in the one
worker thread. It does the first job, then does the second.

Otherwise, consider notifying the main thread when the first thread is
done. (PostMessage). The main thread message handler could start the
second thread. See http://www.mvps.org/vcfaq/mfc/12.htm

[Davinder]

Thanx for the suggestions

Can the main thread be notified from with in the worker thread function like
this:

UINT SaveScanInfo_WorkerThread(LPVOID pParam)
{
CStellarPhoenixApp *pApp = ( CStellarPhoenixApp* )AfxGetApp();

if(pApp->FileSys)
pApp->FileSys->SaveScanInformation(SavePath,(void*)pApp->MFC_Obj);
else

pApp->ntFileSys->SaveScanInformation(SavePath,(void*)pApp->MFC_Obj);

//Here the work of first worker thread is done. Now posting message to the
main thread to start the second thread.

::PostMessage(hwnd, MY_WM_MESSAGE1, (WPARAM)0, (LPARAM)0);

return 0;
}

Is this the right place to post message to the main thread.

Waiting for ur suggestions

Best Regards

"Scott McPhillips [MVP]" <org-dot-mvps-at-scottmcp> wrote in message

news:etw1QwfE...@TK2MSFTNGP10.phx.gbl...

[Scott McPhillips [MVP]]

Davinder wrote:
> //Here the work of first worker thread is done. Now posting message to the
> main thread to start the second thread.
>
> ::PostMessage(hwnd, MY_WM_MESSAGE1, (WPARAM)0, (LPARAM)0);
>
> return 0;
> }
>
> Is this the right place to post message to the main thread.

Yes.

[Joseph M. Newcomer]

I already gave you the code sample. What is not obvious about it?
joe

[Joseph M. Newcomer]

See below...

On Thu, 5 Jan 2006 13:03:49 +0530, "Davinder" <davi...@stellarinfo.com> wrote:

>Hi
>Thanx everybody for the suggestions
>
>Now i have given this statement:
>DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread->m_hThread,INFINITE);
>

****
And how does this change the fundamental problem? The fundamental problem is that to do
what you are trying to do, you MUST create the thread suspended, set its m_bAutoDelete
flag to FALSE, and then resume the thread. However, a simpler solution is to ignore this
entirely. You don't need to save the CWinThread object for the first thread; just do as I
suggested, and have the termination of the thread PostMessage a request to start the
second thread. Lose the WaitForSingleObject *entirely*, as it represents an untenable
programming style.
*****

>
>As a result of this the ::WaitForSingleObject() has started to work but
>another problem has come up.
>
>The ::WaitForSingleObject() has stopped both the threads - the worker thread
>& the main application thread thus hanging
>my application.

****
Yep. I'll bet it is because you are trying to manipulate controls in the GUI thread from
the worker thread, which is a fatal error if the main GUI thread is blocked.

Rules are simple:
Do not ever block the main GUI thread
Do not ever touch a window in the main GUI thread from a worker thread

You are obviously violating the first rule, and I suspect you are violating the second
rule. Rewrite the code
****

>
>where as only the main app thread(calling thread) shud have stopped & not
>the worker thread.

****
Stopping the main app is already a fundamental design error. Fix it.
Since it sounds like you are manipulating controls from the worker thread, eliminate all
such manipulations. PostMessage user-defined messages from the worker thread to the main
GUI thread and let the main GUI thread manipulate the controls.
****

>
>Why is the worker thread being stopped.

****
Because it tried to SendMessage to a blocked thread. (a) the thread it sent to should not
have been blocked (b) it should never have tried to do a SendMessage to another thread.
****

[Joseph M. Newcomer]

I already posted the code on how to do this. Why didn't you read my example?
joe

[Joseph M. Newcomer]

On Thu, 5 Jan 2006 13:12:12 +0530, "Davinder" <davi...@stellarinfo.com> wrote:

>I have changed the code.
>plz have a look:
>
>
>pApp->pWorkerThread =

****
Get rid of this assignment. You don't need to save the reference to the worker thread
object. At best, store it in a local variable to make sure the thread started. But you
don't need to save it. Forget the CREATE_SUSPENDED. Forget m_bAutoDelete. Both of these
are required because you are erroneously waiting on the thread to terminate, and that is a
design error which must be eliminated.
****

>AfxBeginThread(SaveScanInfo_WorkerThread,(void*)pApp->MFC_Obj,
> THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
>
>pApp->pWorkerThread->m_bAutoDelete = false;
>pApp->pWorkerThread->ResumeThread();
>
>DWORD ret = ::WaitForSingleObject(pApp->pWorkerThread->m_hThread,5000);

****
This is even worse than the previous code. Not only did you leave this code in (it should
have been eliminated) but you put a timeout on it. Then you don't check to see if there
was a timeout, and you do a delete of the worker thread without knowing if it is still
running. This code is all erroneous. Get rid of all of it
****

>
>delete pApp->pWorkerThread;
>
>
>
>As a result of the above code following has happened:
>
>The ::WaitForSingleObject() has stopped both the threads - the worker thread
>& the main application thread thus hanging
>my application.

****
Same problem I already described. (a) DO NOT WAIT (b) YOU ARE MANIPULATING A CONTROL IN
THE MAIN GUI THREAD FROM THE WORKER THREAD. Both of these are fundamental design errors,
and the code must be rewritten.
****n

[Joseph M. Newcomer]

See below...

On Thu, 5 Jan 2006 19:58:47 +0530, "Davinder" <davi...@stellarinfo.com> wrote:

>Thanx for the suggestions
>
>Can the main thread be notified from with in the worker thread function like
>this:
>
>UINT SaveScanInfo_WorkerThread(LPVOID pParam)

****
Why is this not a static class member function of a class? Why is it just a global
function?
****

>{
> CStellarPhoenixApp *pApp = ( CStellarPhoenixApp* )AfxGetApp();
>
> if(pApp->FileSys)
> pApp->FileSys->SaveScanInformation(SavePath,(void*)pApp->MFC_Obj);
> else
>
>pApp->ntFileSys->SaveScanInformation(SavePath,(void*)pApp->MFC_Obj);
>
>
> //Here the work of first worker thread is done. Now posting message to the
>main thread to start the second thread.
>
> ::PostMessage(hwnd, MY_WM_MESSAGE1, (WPARAM)0, (LPARAM)0);

****
Where does hwnd come from? Why are you manipulating the app when you should probably be
doing this from the mainframe?
****

>
> return 0;
>}
>
>Is this the right place to post message to the main thread.

****
I get scared every time I see variables in the CWinApp class. Most of them have nothing
to do with the CWinApp, and this looks like one of them. But if you still keep using the
app, what wnd is being used.
****

>
>Waiting for ur suggestions
>
>Best Regards
>
>
>
>
>
>
>"Scott McPhillips [MVP]" <org-dot-mvps-at-scottmcp> wrote in message
>news:etw1QwfE...@TK2MSFTNGP10.phx.gbl...
>> Davinder wrote:
>> > Thanx for the suggestion
>> > The worker thread updates the status bar pane 0.
>> > So probably this is the reason y both threads are blocked.
>> >
>> > I shud better avoid using ::WaitForSingleObject().
>> >
>> > Now how to do i ensure that the second worker thread is not started
>untill
>> > the first worker thread finishes his job.
>> >
>> > waiting for suggestions
>> >
>> > Best Regards
>>
>> The easy and obvious solution would be to put both jobs in the one
>> worker thread. It does the first job, then does the second.
>>
>> Otherwise, consider notifying the main thread when the first thread is
>> done. (PostMessage). The main thread message handler could start the
>> second thread. See http://www.mvps.org/vcfaq/mfc/12.htm
>>
>> --
>> Scott McPhillips [VC++ MVP]
>>
>

[Alexander Grigoriev]

You may have to wait for the thread if you want to exit the app (or delete
the objects it uses) before the thread finishes by itself.
It's always a good idea to keep track of the thread(s), to allow graceful
shutdown of those.

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message

news:j39rr152gtuhr11jv...@4ax.com...

[Joseph M. Newcomer]

Not really. I have eliminated any need to wait for threads by handling app exit
asynchronously. I rarely keep track of CWinThread objects, using other means of doing
shutdowns.
joe

[Doug Harrison [MVP]]

On Thu, 05 Jan 2006 09:07:46 +0100, Walter Eicher
<walter...@nospam.microcrystal.ch> wrote:

>I never had a thread where m_bAutoDelete = TRUE made any sense. IMHO
>the decision to set m_bAutoDelete to TRUE as default was wrong.

The whole idea was terribly misguided. It's a built-in race condition for
the reasons given here in Q1 and Q2:

http://members.cox.net/doug_web/threads.htm

[Doug Harrison [MVP]]

On Fri, 06 Jan 2006 10:39:11 -0500, Joseph M. Newcomer
<newc...@flounder.com> wrote:

>Not really. I have eliminated any need to wait for threads by handling app exit
>asynchronously. I rarely keep track of CWinThread objects, using other means of doing
>shutdowns.

If you don't join with all your threads before exiting, you have a race
condition in your application termination.

[Joseph M. Newcomer]

Yes. The issue is whether you use a synchronous join or an asynchronous join. I have
tended more and more in recent years to using an asynchronous join because I don't get
caught even inadvertently in a situation where there can be a thread deadlock.

In one case, I had some variable number of threads, n, in the program. All I did was keep
a reference count of outstanding threads, and when OnClose happened, I deferred executing
the parent class OnClose until the reference count of outstanding threads had gone to
zero. So all OnClose did was simply initiate thread shutdown and return; the "join" was
when the outstanding thread count went to zero.
joe

[Doug Harrison [MVP]]

On Fri, 06 Jan 2006 16:08:17 -0500, Joseph M. Newcomer
<newc...@flounder.com> wrote:

>Yes. The issue is whether you use a synchronous join or an asynchronous join. I have
>tended more and more in recent years to using an asynchronous join because I don't get
>caught even inadvertently in a situation where there can be a thread deadlock.
>
>In one case, I had some variable number of threads, n, in the program. All I did was keep
>a reference count of outstanding threads, and when OnClose happened, I deferred executing
>the parent class OnClose until the reference count of outstanding threads had gone to
>zero. So all OnClose did was simply initiate thread shutdown and return; the "join" was
>when the outstanding thread count went to zero.

Unless you WaitForSingleObject (or use one of its cousins), you still have
a race condition. Finding a reference count equal to zero is not a
substitute for finding a thread handle signaled.

[Joseph M. Newcomer]

How so? The way it works is that the last thing the thread does before returning from the
thread is a PostMessage. Since nothing accesses the thread after that point, and it
accesses nothing else, the only issue is whether or not the process completes and does
ExitProcess before the thread actually finishes. As far as I can tell, all required
conditions are met for clean thread termination.
joe

[Alexander Grigoriev]

You actually would want your threads to really really exit (at least execute
'delete this') _before_ the MFC and CRTL starts shutting down. Your scheme
doesn't guarantee that.

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message
news:ttqtr1ta27q29gctd...@4ax.com...

[Doug Harrison [MVP]]

On Fri, 06 Jan 2006 17:17:58 -0500, Joseph M. Newcomer
<newc...@flounder.com> wrote:

>How so? The way it works is that the last thing the thread does before returning from the
>thread is a PostMessage.

That may be the last thing you do, but it isn't the last thing
_AfxThreadEntry does, and that's the function MFC passed to _beginthreadex.
It is _AfxThreadEntry that calls your function, and when you return to it,
among other things, it goes on to call AfxEndThread, which normally causes
the CWinThread to do its evil "delete this." And those are just the
highlights.

There is yet another level of indirection in the CRT, where _beginthreadex
passes the address of a function _threadstartex to ::CreateThread, and it
is _threadstartex that ultimately calls _AfxThreadEntry:

_endthreadex (
( (unsigned (__CLR_OR_STD_CALL *)(void
*))(((_ptiddata)ptd)->_initaddr) )
( ((_ptiddata)ptd)->_initarg ) ) ;

In this case, _initaddr would be equal to _AfxThreadEntry. Note that
_endthreadex performs its own actions when control enters it, and even if
AfxEndThread didn't call _endthreadex for you, the CRT would call it for
you. So there is a lot that goes on in the thread after you perform your
PostMessage.

>Since nothing accesses the thread after that point, and it
>accesses nothing else, the only issue is whether or not the process completes and does
>ExitProcess before the thread actually finishes. As far as I can tell, all required
>conditions are met for clean thread termination.

Nope, the only way to achieve an orderly shutdown is to wait on the thread,
and this means keeping tabs on all your CWinThread objects, which means
disabling auto-delete and assuming ownership of them.

[Davinder]

Hi guys.
Thanx a lot for the suggestions.
They are really helpful.

Following code got me thru:
pApp->pWorkerThread =

AfxBeginThread(SaveScanInfo_WorkerThread,(void*)pApp->MFC_Obj,
THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);

pApp->pWorkerThread->m_bAutoDelete = false;
pApp->pWorkerThread->ResumeThread();

// Enter a message loop until we get a WM_QUIT message,
// or the thread we are waiting for finishes.
while (TRUE)
{
DWORD result;
MSG msg;

// Read all of the messages in this next loop,
// removing each message as we read it.
while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
{
// If it is a quit message, exit.
if (msg.message == WM_QUIT)
return;
// Otherwise, dispatch the message.
DispatchMessage(&msg);
}

DWORD cCount = 1;
HANDLE handles[1];

handles[0] = pApp->pWorkerThread->m_hThread;

// Wait for any message sent or posted to this queue
// or for the thread to be signaled.
result = MsgWaitForMultipleObjects(cCount,
handles,
FALSE,
INFINITE,
QS_ALLINPUT);

// The result tells us the type of event we have.
if (result == (WAIT_OBJECT_0 + cCount))
{
// New messages have arrived.
// Continue to the top of the always while loop to
// dispatch them and resume waiting.
continue;
}
else if (result == WAIT_FAILED)
{
// handle error
break;
}
else
{
//our thread has finished...
break;
}
}//end of while..

delete pApp->pWorkerThread;

"Joseph M. Newcomer" <newc...@flounder.com> wrote in message
news:dd9rr19tdm8kcv6n9...@4ax.com...

[Joseph M. Newcomer]

First thing to understand about PeekMessage is that using it is almost always a mistake.
In particular, your code is erroneous. It does not call TranslateMessage. It imposes a
substantial overhead in the computation. Code like this is hard to debug and hard to keep
working right.
joe