CWinThread and Memory Leaks.

[Keith Sheppard]

I want to have a simple progress bar displayed whilst my MFC application is
doing a lengthy operation. To do this, I am creating a user interface
thread which displays a dialog containing a progress bar control.

This is implemented using two classes:

class CProgressDlg : public CDialog
class CProgressThread : public CWinThread

The CProgressThread class has a member variable: CProgressDlg m_Dlg;

At the start of the lengthy operation I create a new instance of
CProgressThread, and set the m_bAutoDelete member TRUE before calling
CProgressThread::CreateThread();

In CProgressThread::InitInstance I have the following code:
if (!IsWindow(m_Dlg.m_hWnd))
m_Dlg.Create(IDD_PROGRESS_DLG, NULL);
m_Dlg.ShowWindow(SW_SHOW);

During the lengthy operation, I post update messages to the dialog box and
the progress bar is updated as expected.

When the operation is finished, I post a "completed" message to the dialog
box which causes it to call EndDialog(). The dialog then disappears as
expected.

All works perfectly until I close the application. Whereupon visual studio,
in debug mode, reports memory leaks - one for every CProgressThread object
created during the lifetime of the application.

If I try to explicitly delete the CProgressThread object when I've finished
with it, the program asserts.

Any suggestions?

Keith

[Mihajlo Cvetanovic]

Post by Keith Sheppard
All works perfectly until I close the application. Whereupon visual studio,
in debug mode, reports memory leaks - one for every CProgressThread object
created during the lifetime of the application.
If I try to explicitly delete the CProgressThread object when I've finished
with it, the program asserts.

I hope that in CProgressThread::CreateThread() you actually call
AfxBeginThread.

You can't delete a CProgressThread object. You must inform the window
that it should stop immediatley, and then wait until the tread finishes
(with WaitForSingleObject). When the thread finishes the object will be
deleted.

[Keith Sheppard]

Post by Mihajlo Cvetanovic
I hope that in CProgressThread::CreateThread() you actually call
AfxBeginThread.

Ah. No. I thought CreateThread() was an alternative to AfxBeginThread. I
had not even bothered to override CreateThread. I thought the default
implementation would call AfxBeginThread for me.

So are you saying that even with a CWinThread object I must explicitly call
AfxBeginThread() and that overriding CreateThread() is mandatory?

Do you have a code sample?

Keith

[Mihajlo Cvetanovic]

Post by Keith Sheppard

Post by Mihajlo Cvetanovic
I hope that in CProgressThread::CreateThread() you actually call
AfxBeginThread.

Ah. No. I thought CreateThread() was an alternative to AfxBeginThread. I
had not even bothered to override CreateThread. I thought the default
implementation would call AfxBeginThread for me.
So are you saying that even with a CWinThread object I must explicitly call
AfxBeginThread() and that overriding CreateThread() is mandatory?

No, sorry, my bad, I got confused. It doesn't matter how you create a
thread as long as you disable auto delete feature, as Doug mentioned.

[Doug Harrison [MVP]]

Post by Keith Sheppard
I want to have a simple progress bar displayed whilst my MFC application is
doing a lengthy operation. To do this, I am creating a user interface
thread which displays a dialog containing a progress bar control.
class CProgressDlg : public CDialog
class CProgressThread : public CWinThread
The CProgressThread class has a member variable: CProgressDlg m_Dlg;
At the start of the lengthy operation I create a new instance of
CProgressThread, and set the m_bAutoDelete member TRUE before calling
CProgressThread::CreateThread();

That variable starts off true, which is a design flaw. Below are some
excerpts from previous messages I've written about this.

***** About safely using CWinThread:

To safely use CWinThread, you must start the thread suspended and set the
CWinThread object's m_bAutoDelete member to false or DuplicateHandle a copy
of its m_hThread member. Only then should you resume the thread. This avoids
the race condition resulting from the default auto-delete behavior, allowing
you to wait on the thread handle. It's even worse than that. According to
the current WaitForSingleObject documentation, it's undefined for a handle
to be closed while you're waiting on it. Thus, all code which waits on a
CWinThread m_hThread member while the CWinThread auto-deletes itself is
undefined. If you set m_bAutoDelete to false, you assume the responsibility
for deleting the CWinThread, while if you dup m_hThread, you will have to
close the duped handle when you're done with it. I find it easier to change
m_bAutoDelete.

***** About why you should explicitly join with your threads before exiting
your app:

In general, it's a bad idea to allow secondary threads to continue executing
as the application is shutting down, static duration objects are being
destroyed, the CRT is being taken down, etc, until the CRT finally calls
ExitProcess and kills them all, assuming the secondary threads haven't
deadlocked with the CRT shutting down in the primary thread or crashed in
the meantime, say, due to objects they're using being destroyed. To avoid
this and achieve an orderly shutdown, you have to join with all your threads
before exiting your app, and that means being able to wait on their handles.
But you can't wait on their handles if they auto-delete themselves, because
according to MSDN, it's undefined to close a handle being used in a Wait
function, and it's the secondary thread that closes the CWinThread handle
before auto-deleting and terminating, the latter being the thing which would
cause the handle to become signaled, if only it hadn't been closed already!
This whole auto-deletion concept as applied to threads is pretty much a bad
idea, with limited utility.

Post by Keith Sheppard
if (!IsWindow(m_Dlg.m_hWnd))
m_Dlg.Create(IDD_PROGRESS_DLG, NULL);
m_Dlg.ShowWindow(SW_SHOW);
During the lengthy operation, I post update messages to the dialog box and
the progress bar is updated as expected.
When the operation is finished, I post a "completed" message to the dialog
box which causes it to call EndDialog(). The dialog then disappears as
expected.
All works perfectly until I close the application. Whereupon visual studio,
in debug mode, reports memory leaks - one for every CProgressThread object
created during the lifetime of the application.
If I try to explicitly delete the CProgressThread object when I've finished
with it, the program asserts.
Any suggestions?

The simplest explanation is that your secondary threads are still running as
you shut down, which is a bad situation for the reasons given earlier. In
addition, it's often simpler in the long run to keep all the UI in the main
thread. That is, the main thread would create the window, not the secondary
threads, and the latter would communicate with the main thread mainly
through a (smallish) user-defined PostMessage interface.

[jim_OLP]

I've tried to do a similar progress dialog thing using CWinThread. What
I find is that if the main thread blocks inside a message handler, the
progress UI thread can't dispatch any messages. It looks to me like
multiple CWinThreads are interlocked/synchronized such that only one
can be dispatching a message at any given time. In other words,
CWinThread is useless. Am I missing something? Was this true before VC
7.0?

[Doug Harrison [MVP]]

Post by jim_OLP
I've tried to do a similar progress dialog thing using CWinThread. What
I find is that if the main thread blocks inside a message handler, the
progress UI thread can't dispatch any messages. It looks to me like
multiple CWinThreads are interlocked/synchronized such that only one
can be dispatching a message at any given time. In other words,
CWinThread is useless. Am I missing something? Was this true before VC
7.0?

CWinThread does not do that. It sounds like you have an interthread
SendMessage situation, which is a Windows issue. It can arise not only by
calling SendMessage directly but by calling most any function that
manipulates a window and isn't spelled "PostMessage", as things like
SetWindowText use SendMessage under the hood. Here's an excerpt from one of
my recent messages here which expands on this:

SendMessage is a synchronous call. When the target is a window created by
the calling thread, it amounts to an ordinary function call. When the target
is a window created by a different thread, the calling thread blocks until
the target thread is in a receptive state, i.e. it makes a call to one of a
handful of functions that are considered safe points to dispatch pending
interthread sent messages, e.g. GetMessage, PeekMessage, etc. When the
system detects the target thread is in such a state, it switches to that
thread, which goes on to handle the message. In the meantime, the calling
thread sits there blocked until the target finishes the message, either by
returning from its message handler or by calling ReplyMessage.

That said, it's quite possible to design a system that works properly with
interthread SendMessage. The major requirement is that the target thread
enter the receptive state on a regular, timely basis, because outside this
state, it cannot dispatch interthread sent messages, and callers will sit
there unable to continue until the target enters the receptive state. In
well-designed programs, GUI threads pretty much meet this requirement by
definition, because to keep the GUI responsive is to process messages with
low latency, and this allows interthread SendMessage to work.

What about the deadlock potential? It's always an issue when using
synchronization, and interthread SendMessage is a form of synchronization.
There is a great deal of confusion concerning the likelihood of deadlock
when the target window merely wants to send a message to a window in the
calling thread, as happens in many parent/child window scenarios. Raymond
Chen addressed this in his blog:

When can a thread receive window messages?
http://weblogs.asp.net/oldnewthing/archive/2004/06/08/150929.aspx
<q>
If one thread T1 send a message to a window that belongs to another thread
T2, the sending thread T1 is put to sleep until the receiving thread replies
to the message. But if somebody else sends a message to thread T1, thread T1
is woken to process the message, then is returned to sleep.
...
Another case is that thread T1 sends a message to thread T2, and thread T2
needs to ask thread T1 for help before it can finish the operation. This
isn't as strange as it sounds. You do something similar all the time without
realizing it when you respond to a WM_NOTIFY message by sending messages
back to the control that sent the notification. (For example, you may
respond to a LVN_ITEMACTIVATE by sending back a LVM_GETITEM to get
information about the item that was activated.)
</q>

So, it appears this common concern is not actually a problem at all, because
the receptive state includes being blocked in interthread SendMessage.

The main question you need to ask yourself when thinking about using
interthread SendMessage is, "Do I really need this call to be synchronous?"
If the answer is "yes," then you simply have to make it safe, and the
preceding discussion should guide you in this. If the answer is "no," then
just use PostMessage. However, if the answer is, "I don't know, but to be
safe, I think I can replace it with some send/ack scheme using PostMessage,"
you need to ensure that the replacement method really is safe, and what your
app can do between posting the message and receiving the acknowledgement
doesn't mess you up. In particular, if you return to your main message loop,
which is typical, you put your whole program in play, which can invalidate
your assumptions about what can happen between the post and the ack. IOW,
the program may be in an unanticipated state when the ack is received, and
more than anything, this points out why, "I don't know, but to be safe..."
is an inadequate basis on which to proceed. Anyway, these are the sorts of
things you need to think about.

[jim_OLP]

Thanks for the reply, I appreciate your expertise and hope you can get
me out of this deadlock.

No inter-thread SendMessage whatsoever. This is all stuff I've done
successfully in the past, at 6.0, and I can't seem to make it work
now.

My main thread creates a CWinThread-derrived class with AfxBeginThread.
That thread creates a modeless dialog in InitInistance. Things are fine
until the main thread enters a loop iinside a message handler (the
reason for the progress thread. The loop has a generous Sleep() in it
so it's not hogging the CPU. After that, the modeless dialog in the
'progress' thread is dead. Spy++ shows it never gets any messages. If I
break into the debugger I see my progress thread blocked at a
DefWndProc call to m_pfnSuper. And it's total fog to me after that
point, as I don't know what m_pfnSuper represents. I suspect MFC is
trying to pass unhandled messages to the main window, thinking it's the
parent of my dialog, but that makes no real sense. When I create the
modeless dialog I don't give it a parent.

[Doug Harrison [MVP]]

Post by jim_OLP
Thanks for the reply, I appreciate your expertise and hope you can get
me out of this deadlock.
No inter-thread SendMessage whatsoever.

None that you've identified so far, anyway. :)

Post by jim_OLP
This is all stuff I've done
successfully in the past, at 6.0, and I can't seem to make it work
now.
My main thread creates a CWinThread-derrived class with AfxBeginThread.
That thread creates a modeless dialog in InitInistance. Things are fine
until the main thread enters a loop iinside a message handler (the
reason for the progress thread. The loop has a generous Sleep() in it
so it's not hogging the CPU.

Inside the Sleep, it's also not processing messages or doing much of
anything else. This is anathema to UI threads, which as much as possible,
should sit in a GetMessage loop at all times.

Post by jim_OLP
After that, the modeless dialog in the
'progress' thread is dead. Spy++ shows it never gets any messages. If I
break into the debugger I see my progress thread blocked at a
DefWndProc call to m_pfnSuper. And it's total fog to me after that
point, as I don't know what m_pfnSuper represents. I suspect MFC is
trying to pass unhandled messages to the main window, thinking it's the
parent of my dialog, but that makes no real sense. When I create the
modeless dialog I don't give it a parent.

I'm sure you could get to the bottom of this if you kept spelunking around,
but I'd move the time-consuming operation out of the UI thread and into a
worker thread, and I'd have the UI thread host the progress dialog for the
worker. The worker would then communicate with this window (or another
designated target window) mainly through a (smallish) user-defined
PostMessage interface. This approach is probably the closest you can get to
something that "just works".

This is a wild guess, and hopefully someone will correct me or expand on
this as appropriate, but ISTR that if you don't specify the main window for
a secondary UI thread, MFC sets it to the primary thread's main window. I
don't know if this could pose a problem, but if I were ever to create
multiple UI threads, I'd look into it.

[jim_OLP]

[sigh...] As is so often the case I'm dealing with a large legacy
project and can't move the time-consuming operation into its own
thread. It has to access data in the main thread, so the progress
window has to be in the 'other' thread.


As you point out "ISTR that if you don't specify the main window for
a secondary UI thread, MFC sets it to the primary thread's main
window." I think that's what's happening and I think for some reason,
unprocessed messages are going to the main window in the main thread.
Maybe if I set m_pMainWnd in the 'progress' thread...

When I've done this in the past, the second thread has created a
full-blown MFC doc/frame/view lashup which probably satisfied all the
hidden requirements. This time I'm just creating a single modeless
dialog.

[Doug Harrison [MVP]]

Post by jim_OLP
[sigh...] As is so often the case I'm dealing with a large legacy
project and can't move the time-consuming operation into its own
thread. It has to access data in the main thread, so the progress
window has to be in the 'other' thread.
As you point out "ISTR that if you don't specify the main window for
a secondary UI thread, MFC sets it to the primary thread's main
window." I think that's what's happening and I think for some reason,
unprocessed messages are going to the main window in the main thread.
Maybe if I set m_pMainWnd in the 'progress' thread...
When I've done this in the past, the second thread has created a
full-blown MFC doc/frame/view lashup which probably satisfied all the
hidden requirements. This time I'm just creating a single modeless
dialog.

Unprocessed messages aren't sent to the main window, or any other window
besides the target window, for that matter. However, command messages are
routed amongst various windows in doc/view. In addition, PreTranslateMessage
goes up the chain of parents, so if your main window in thread A is parent
to a window in thread B, you could conceivably run into trouble that way.
Oh, and if we're right that your main window is the same in both your
threads, then I'd expect AfxGetMainWnd to be a potential trouble spot as
well. Lots of code does AfxGetMainWnd()->whatever(), and that can easily end
up in SendMessage.

[jim_OLP]

I accept what you're telling me, that CWinThread is useable in multiple
instances and won't try to synchronize at the message level. I now
think the problem is trying to use a CDialog as the top-level window in
my progress thread. MFC insists it have a parent even if I don't give
it one; so it becomes a child window instead of an 'owned' window. If
I call SetFocus on it from the main thread before entering the loop, it
wakes up, but loses the focs when I switch apps and goes dead again.

I think I need it to be a child of the desktop but owned by my
application. Maybe MFC won't support a CDialog in this role.

[jim_OLP]

"nside the Sleep, it's also not processing messages or doing much of
anything else. This is anathema to UI threads, which as much as
possible,
should sit in a GetMessage loop at all times."


Well, yes. I guess that's the whole reason I have to create another
thread. If my main thread could keep pumping messages, I'd have no
problem, but it's tied up in a lengthy computation so to manage a
progress control I need a dialog in another thread.

BTW I tried setting m_pMainWnd to the dialog itself - no joy. Tried
overriding DefWindowProc in the progress dialog, and just calling
::DefWindowProc. It hangs right there, unless the main thread pumps
messages.

I don't get it. I guess I give up at this point and I just do not see
any utility to CWinThread if it can't handle messages without some
unspecified help from the main thread.

[Keith Sheppard]

Doug

I didn't quite understand your posting. Currently I don't bother to store
the m_hThread member. Once I have created the thread, I have no further
dealings with it other than to post messages to the created dialog box's
window. I don't ever do any WaitForSingleObject calls on any thread related
handles.

Do the concerns you raised apply in this instance?

I may have some more information about my root cause problem. When my
process has finished, I post a message to the dialog which causes it to
close, but I think the thread continues to run. How do I tell the thread
that it's time to pack its bags and go?

Keith

[Scott McPhillips [MVP]]

Post by Keith Sheppard
Doug
I didn't quite understand your posting. Currently I don't bother to store
the m_hThread member. Once I have created the thread, I have no further
dealings with it other than to post messages to the created dialog box's
window. I don't ever do any WaitForSingleObject calls on any thread related
handles.
Do the concerns you raised apply in this instance?
I may have some more information about my root cause problem. When my
process has finished, I post a message to the dialog which causes it to
close, but I think the thread continues to run. How do I tell the thread
that it's time to pack its bags and go?
Keith

To assure a clean shutdown it is necessary to tell the thread to pack
its bags and go, and then pause the main thread until the secondary
thread has terminated.

Signal the thread to quit. You might use a posted message, or SetEvent,
or a shared bool. When the thread senses this signal it must call
PostQuitMessage to itself (if it is a UI thread) or it must return from
the thread function (if it is a worker thread).

Meanwhile, the main thread must wait. It can use WaitForSingleObject on
the thread handle. Doug's posting covers the intricacies of doing this
reliably.

[Keith Sheppard]

Post by Scott McPhillips [MVP]
Signal the thread to quit. You might use a posted message, or SetEvent,
or a shared bool. When the thread senses this signal it must call
PostQuitMessage to itself (if it is a UI thread) or it must return from
the thread function (if it is a worker thread).

Thanks Scott. It was a UI thread but PostQuitMessage was the missing piece
of the jigsaw. It's working now, thanks.

Keith

[Keith Sheppard]

Post by Scott McPhillips [MVP]
To assure a clean shutdown it is necessary to tell the thread to pack
its bags and go, and then pause the main thread until the secondary
thread has terminated.

Currently I am not pausing the main thread and not getting any problems.
Note that there is no shared data between the UI thread and the main thread
and that the main thread doesn't really care whether the UI thread has
finished or not. The sole purpose of the UI thread is to display a progress
bar and the only communication from main->UI thread is by sending Windows
messages. There is no communication in the other direction.

Given this level of independence, is it still necessary to pause the main
thread whilst shutting the UI thread?

Keith

[Scott McPhillips [MVP]]

Post by Keith Sheppard

Post by Scott McPhillips [MVP]
To assure a clean shutdown it is necessary to tell the thread to pack
its bags and go, and then pause the main thread until the secondary
thread has terminated.

Currently I am not pausing the main thread and not getting any problems.
Note that there is no shared data between the UI thread and the main thread
and that the main thread doesn't really care whether the UI thread has
finished or not. The sole purpose of the UI thread is to display a progress
bar and the only communication from main->UI thread is by sending Windows
messages. There is no communication in the other direction.
Given this level of independence, is it still necessary to pause the main
thread whilst shutting the UI thread?
Keith

No, not necessary here as you seem to have the issues covered. The
purpose of waiting for the secondary thread to quit is to assure it will
not access shared data or passed handles after such data has disappeared.

[Keith Sheppard]

Scott

Thanks for your help and advice.

Keith