Another way for waiting for a AfxBeginThread thread to end

[kalevet]

Hi all,
I'm using AfxBeginThread to create a worker thread, and then wait for
it to end using WaitForSingleObject. I've encountered the known issue
of having to either duplicate the handle or set the CWinThread's
m_bAutoDelete member to false in order to make sure WFSO waits on a
valid handle.
I can't say I really like either way - I'd rather not have to take
care of the allocated CWinThread object (let alone a bunch of objects
at another place), nor would I like to waist time on duplicating the
handle (time matters).
How about that: CWinThread's d'tor will close m_hThread only if it is
not NULL. If I copy m_hThread to my own variable, then set m_hThread
to be NULL, I'll avoid the two problems above. The CWinThread will
indeed be automatically deleted, yet the handle will not be closed
until I close it.
The only problem with that method is that I kind of mess with someone
else's member variable. I've examind CWinThread's source and it looks
safe, but it still feels wrong.
I'd appreciate any comments. In case I wasn't clear, here's the
psaudo:

CWinThread* pt = AfxBeginThread(...);
HANDLE h = pt->m_hThread; // probably better suspend the thread
before this
m_hThread = NULL; // till after this
... work work work ...
WaitForSingleObject(h, INFINITE);
CloseHandle(h); // pt already destructed

Thanks,
Kal

[GuitarBill]

If its just a worker thread, you can use:
hthread = (HANDLE)_beginthread(func...)
...
WaitForSingleObject(hthread);

Bill

"kalevet" <kal...@hotmail.com> wrote in message
news:96343ecd.04072...@posting.google.com...

[Scott McPhillips [MVP]]

kalevet wrote:

> Hi all,
> I'm using AfxBeginThread to create a worker thread, and then wait for
> it to end using WaitForSingleObject. I've encountered the known issue
> of having to either duplicate the handle or set the CWinThread's
> m_bAutoDelete member to false in order to make sure WFSO waits on a
> valid handle.
> I can't say I really like either way - I'd rather not have to take
> care of the allocated CWinThread object (let alone a bunch of objects
> at another place), nor would I like to waist time on duplicating the
> handle (time matters).
> How about that: CWinThread's d'tor will close m_hThread only if it is
> not NULL. If I copy m_hThread to my own variable, then set m_hThread
> to be NULL, I'll avoid the two problems above. The CWinThread will
> indeed be automatically deleted, yet the handle will not be closed
> until I close it.
> The only problem with that method is that I kind of mess with someone
> else's member variable. I've examind CWinThread's source and it looks
> safe, but it still feels wrong.
> I'd appreciate any comments.

I can believe that would work, but it seems rather poor to me, largely
because its operation is unobvious to some future maintainance
programmer. It does not seem simpler or less work than clearing
m_bAutoDelete, which is the well-known, clear and accepted solution.

--
Scott McPhillips [VC++ MVP]

[Michael K. O'Neill]

If you start the thread and then WFSO until its work is done, then you've
lost all benefit of its creation. You might as well do all the work in the
main thread.

Or maybe that psuedo code was too too pseudo.

Mike

"kalevet" <kal...@hotmail.com> wrote in message
news:96343ecd.04072...@posting.google.com...

[Alexander Grigoriev]

You should use _beginthreadex. Braindead _beginthread closes the thread
handle on exit.

"GuitarBill" <GuitarBill_at_cox_dot_net> wrote in message
news:%230VD5iJ...@TK2MSFTNGP09.phx.gbl...

[kalevet]

I would, but I use mfc classes (CString and others) inside the thread.
Back to the first qestion then...

"GuitarBill" <GuitarBill_at_cox_dot_net> wrote in message news:<#0VD5iJc...@TK2MSFTNGP09.phx.gbl>...

[Joseph M. Newcomer]

The code you show is what we call "silly". There is no reason to create a thread and then
block waiting for it; you might as well not bother creating the thread, since it only adds
complexity without adding any value.

The whole point of using threads is so that you DON'T block the main GUI thread.

In addition to being silly, the code is wrong. by the time AfxBeginThread returns, the
thread could have finished, and the pt->m_hThread assignment will be nonsense. You can't
tell.

Use asynchronous notifications. Have the thread PostMessage to your main GUI thread when
it has finished. Waiting is almost always a mistake. In this case, once you've fixed the
bug, the code is pointless because there is no reason to have a thread. But don't block
the GUI thread.
joe

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

[kalevet]

Thanks for all answers. I guess that code was really too psaudo, or my
description wasn't clear enough (English is not my best language)...
My application is a service which has no GUI, so there's no GUI thread
to block. It runs on a double CPU machine, so there is a point in
working concurrently. Moreover, I sometimes use more than one thread
(depending on the CPUs number) to do some batch jobs, which I do have
to eventually wait for.
Now, back to the first question, regardless of how it's used on my
application: Do you think taking the handle from the CWinThread is a
good idea?

kal...@hotmail.com (kalevet) wrote in message news:<96343ecd.04072...@posting.google.com>...

[Tobias Güntner]

kalevet wrote:
> Hi all,
> I'm using AfxBeginThread to create a worker thread, and then wait for
> it to end using WaitForSingleObject. I've encountered the known issue
> of having to either duplicate the handle or set the CWinThread's
> m_bAutoDelete member to false in order to make sure WFSO waits on a
> valid handle.

Why don't you put that code in a function? like

CWinThread* CreateTask(...)
{
CWinThread* pThread = AfxBeginThread(... CREATE_SUSPENDED ...);
pThread->m_bAutoDelete = FALSE;
pThread->Resume();
return pThread;
}

WaitForTask(CWinThread* pThread)
{
WFSO(pThread->m_hThrea);
delete pThread;
}

> nor would I like to waist time on duplicating the
> handle (time matters)

AfxBeginThread internally creates and destroys several events, suspends
and resumes the new thread a few times (context switches included) and
does at least one dynamic memory allocation (the CWinThread).
DuplicateHandle() only needs a fraction of the time needed to create
your thread.

--
Regards,
Tobias

[Doug Harrison [MVP]]

kalevet wrote:

>Hi all,
>I'm using AfxBeginThread to create a worker thread, and then wait for
>it to end using WaitForSingleObject. I've encountered the known issue
>of having to either duplicate the handle or set the CWinThread's
>m_bAutoDelete member to false in order to make sure WFSO waits on a
>valid handle.
>I can't say I really like either way - I'd rather not have to take
>care of the allocated CWinThread object (let alone a bunch of objects
>at another place), nor would I like to waist time on duplicating the
>handle (time matters).

Only significant amounts of time matter. Duplicating the handle requires an
insignificant amount of time. However, duplicating the handle is not the
best approach to this problem.

>How about that: CWinThread's d'tor will close m_hThread only if it is
>not NULL. If I copy m_hThread to my own variable, then set m_hThread
>to be NULL, I'll avoid the two problems above.

Except if you don't start the thread suspended, there is potential for the
thread to exit before you can do all that. Back to square one. Also, you may
well have broken CWinThread, and by assuming responsibility for the handle,
you haven't really avoided "taking care of the allocated CWinThread object";
you've just shuffled things around so that you're "taking care of the open
handle".

>The CWinThread will
>indeed be automatically deleted, yet the handle will not be closed
>until I close it.

You still have to track the handle. What does this buy you over having to
keep up with the CWinThread object?

>The only problem with that method is that I kind of mess with someone
>else's member variable. I've examind CWinThread's source and it looks
>safe, but it still feels wrong.
>I'd appreciate any comments.

Your instincts expressed in "it still feels wrong" are good. Listen to those
ones. :) Analyze what you did. Obviously, you knew from the start that you
were "messing with someone else's member variable", which should have
automatically triggered the klaxons, and so you had to laboriously and
hopefully exhaustively analyze all the potential side effects of your
actions, including those occurring in code you didn't write, except, of
course, for the future side effects, which haven't occurred yet. And you did
all this in hopes of avoiding a very minor pain, i.e. starting a thread
suspended so you can set m_bAutoDelete to false, which BTW you still have to
do to avoid the race condition inherent to the CWinThread design, and in the
end you still have to keep up with the handle so you can close it, which is
no better than keeping up with the CWinThread* so you can delete it.

--
Doug Harrison
Microsoft MVP - Visual C++

[Doug Harrison [MVP]]

Michael K. O'Neill wrote:

>If you start the thread and then WFSO until its work is done, then you've
>lost all benefit of its creation. You might as well do all the work in the
>main thread.
>
>Or maybe that psuedo code was too too pseudo.

In general, you should always join with your threads before allowing your
main thread to exit. Otherwise, they continue to run as the main thread is
shutting down, destroying static duration objects the secondary threads may
be using, taking down the heap and other CRT state, and so forth, until
finally, the CRT calls ExitProcess, which forcibly terminates the secondary
threads, assuming the secondary threads haven't corrupted the program in
some way, and the CRT makes it that far. To sum up, auto-deletion of thread
objects is a misguided idea, and you normally must join with them at some
point. This message explains how to safely disable the auto-deletion
behavior, so that you can wait on the thread handles (i.e. "join" with
them):

http://groups.google.com/groups?selm=m3ep501falcm3ecgdb1tsof6p42f32fvk7%404ax.com

[Joseph M. Newcomer]

Substitute "main thread" for "GUI thread" and my objections remain the same. There is no
reason to create a thread if the creating thread blocks waiting for the created thread to
finish. It makes no sense for any number of processors >= 1.

Use asynchronous notifications. If you don't have a GUI, you can still have a message loop
with an invisible window, or you can use I/O Completion Ports as a queueing mechanism. Get
rid of the forced synchronization. You should not be "waiting" for threads to finish, they
should TELL you when they've finished.

Your English is fine. That wasn't where the problem was. You have a weak program design.
That IS a problem.

You have to create the thread suspended, then extract the handle, and also mark it as
m_bAutoDelete = false before doing a ResumeThread. And you must remember to delete the
CWindThread object when the thread finishes. OR, you can extract the handle, make a
duplicate handle (using DuplicateHandle), and let it delete the CWinThread on its own
because you now have a safe handle (which you must remember to CloseHandle after you've
decided you no longer need it). But you can't extract the handle unless you know the
CWinThread exists, and you can't rely on it existing after the return from the
CreateThread call, because the thread could have already completed!
joe

Joseph M. Newcomer [MVP]