Re: Threads, Message Pump and Sync
Hi Joe,
Thank you for replying to my message.
***
Insufficient description. We need to see the code. For example, did=
you set
m_bAutoDelete to FALSE? To do this, you have to create the thread susp=
ended and use
CWinThread::ResumeThread to start it after clearing the autodelete. S=
o this vague
handwave is an inadequate description.
****
Yes, I did set m_bAutoDelete to FALSE and deleted the CWindThread*
object myself.
****
Then your code is erroneous. You are responsible for making sure the w=
orker thread has
shut down BEFORE the window you are posting messages to has been deleted.=
> ****
***
If the dialog is doing a WaitFor, that is part of the problem. See my =
essay on thread
shutdown. What you have to do is NOT close the dialog, and NOT do a Wa=
itFor. Instead,
you indicate a close is pending, and return from the OnOK or OnCancel han=
dlers (or
whatever handler is shutting your dialog down) WITHOUT closing the dialog=
.. Then, your
worker thread, as its last gasp, does a PostMessage of a UWM_THREAD_DEAD =
or similar
user-based message you invent.
I have tried to implement this when I got the idea to send a WM_CLOSE
as my last message. I have now tried a variation to comply w/ your
directions of using a "dead" message instead of a WM_CLOSE but I still
have problems. Therefore, I wrote a sample program to reproduce this
behavior and I will show a detailed description of it here.
This is a MFC dialog-based application with only one dialog and two
buttons: "Start" and "Exit".
When the user presses the start button, the application goes through a
for loop of allocating CString's and posts them to the UI thread.
When pressing EXIT, the application shuts down.
I have two threads: 1 is responsable w/ allocating the CStrings and 1
is responsable w/ freeing them (to simulate the 'delayed'
deallocation).
This is the code:
The thread that allocates the CStrings is created in OnInitDialog as
follows:
m_pWorkThread = AfxBeginThread((AFX_THREADPROC)WorkerThread, this,
THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
m_pWorkThread->m_bAutoDelete = FALSE;
m_pWorkThread->ResumeThread();
Here is the delete thread creation:
m_pDelWorkThread = AfxBeginThread((AFX_THREADPROC)DelWorkerThread,
this,
THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
m_pDelWorkThread->m_bAutoDelete = FALSE;
m_pDelWorkThread->ResumeThread();
The worker thread is implemented as follows:
DWORD CTestTDlg::WorkerThread(LPVOID lpVoid)
{
CTestTDlg* pThis = (CTestTDlg*)lpVoid;
pThis->DoWork();
return 0;
}
void CTestTDlg::DoWork()
{
HANDLE objects[2];
objects[0] = m_hQuitEvent;
objects[1] = m_hProcessEvent;
m_bWorkThreadRunning = TRUE;
while (m_bWorkThreadRunning)
{
switch (WaitForMultipleObjects(2, objects, FALSE, INFINITE))
{
case WAIT_OBJECT_0:
m_bWorkThreadRunning = FALSE;
break;
case WAIT_OBJECT_0+1:
Process();
break;
}
}
}
Process is implemented as this:
void CTestTDlg::Process()
{
for (int i = 0; i < 15000 && m_bWorkThreadRunning; ++i)
{
Sleep(0); // I know it is not a good idea to use sleep in multi-
threaded code.
// However, "Sleep(0)" should "yield" to
other threads since all the thread priorities are the same.
// Is this correct ?
CString* pString = new CString;
PostMessage(UserMsg_Add, (WPARAM)pString, 0);
}
}
Here is the handler of the UserMsg_Add:
LRESULT CTestTDlg::OnAdd(WPARAM wParam, LPARAM lParam)
{
CString* pStr = (CString*)(wParam);
int iUpperBound = m_Array.GetUpperBound();
m_Array.SetAtGrow(++iUpperBound, pStr); // m_Array is defined as:
CArray<CString*, CString*> m_Array;
return 0;
}
When the user presses the "Start" button, the following code is
called:
SetEvent(m_hProcessEvent);
m_hProcessEvent is an auto-reset event created in the constructor of
the dialog this way:
m_hProcessEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
Therefore, at this point, the for loop in the allocating worker thread
is running.
Now, if the user presses the Exit button this is what happens:
m_bQuit = TRUE; // This is a little bit suspect. You will see how I
use this later.
m_bWorkThreadRunning = FALSE;
SetEvent(m_hDelProcessEvent); // This is an auto-reset event that
tells the deallocation thread to perform the deallocations:
.... and nothing else. No exit point here; just a flag set (m_bQuit) to
inform everyone that we want to eventually exit.
Here is the implementation of the de-allocation thread:
DWORD CTestTDlg::DelWorkerThread(LPVOID lpVoid)
{
CTestTDlg* pThis = (CTestTDlg*)lpVoid;
pThis->DelDoWork();
return 0;
}
void CTestTDlg::DelDoWork()
{
HANDLE objects[2];
objects[0] = m_hDelQuitEvent;
objects[1] = m_hDelProcessEvent;
m_bDelWorkThreadRunning = TRUE;
while (m_bDelWorkThreadRunning)
{
switch (WaitForMultipleObjects(2, objects, FALSE, INFINITE))
{
case WAIT_OBJECT_0:
m_bDelWorkThreadRunning = FALSE;
break;
case WAIT_OBJECT_0+1:
DelProcess();
break;
}
}
}
And here is the DelProcess function and the most important part of the
program:
void CTestTDlg::DelProcess()
{
int iUpperBound = m_Array.GetUpperBound();
for (int i = 0; i <= iUpperBound; ++i)
{
Sleep(0); // Sleep(0) -- yield; w/o it, wouldn't this thread take
100% CPU while deleting ?
delete m_Array.GetAt(i); // deallocate all the CStrings
}
m_Array.RemoveAll();
if (m_bQuit) // As you noted earlier, this was set by the "Exit"
button handler.
{
SetEvent(m_hQuitEvent); // This tells the allocating thread to Exit.
DWORD dwRet = WaitForSingleObject(m_pWorkThread->m_hThread,
INFINITE); // WaitFor returns WAIT_OBJECT_0
if (dwRet == WAIT_OBJECT_0) delete m_pWorkThread; // statement
executed as dwRet is WAIT_OBJECT_0. confirmed w/ debug.
SetEvent(m_hDelQuitEvent); // I tell *this* thread to exit.
PostMessage(UserMsg_Exit, 0, 0); // This is the last message; should
tell the dialog to be dismissed.
}
}
Here is the message handler of UserMsg_Exit:
LRESULT CTestTDlg::OnExit(WPARAM wParam, LPARAM lParam)
{
// I know, I shouldn't use WaitFor in the UI thread. However,
w/o it, wouldn't deleting the thread directly be involved in a race-
condition ?
DWORD dwRet = WaitForSingleObject(m_pDelWorkThread-
m_hThread, iTimeout); // dwRet is set to WAIT_OBJECT_0 because the
thread exists normally.
if (dwRet == WAIT_OBJECT_0)
delete m_pDelWorkThread; // this statement is executed; confirmed
w/ debug.
else if (dwRet == WAIT_TIMEOUT)
{
// I use a timeout because I can't afford to wait
indefinetly in the UI thread and I somehow need to
// do something if things go terribly wrong. Anyway, my
code DOES NOT get here since dwRet is WAIT_OBJECT_0
// and this situation is outside the scope of our
discussion.
// I guess the proper solution would be to have this
thread as an autodelete thread ? This way, I don't have to
// wait for it and would be deallocated automatiicaly.
// ---- This way, I would avoid a WaitFor in the UI ----
}
OnOK();
return 0;
}
Basically, this is my structure which still causes lots of CString
memory leaks. Note: If I don't exit WHILE the worker thread is
running, everything is OK. This only happens when I press the "Start"
button and then quickly press the "Exit" button.
Note: I didn't show the "wiring" of the message handlers to the
messages; however, all the message handlers are called correctly as I
have confirmed this w/ the debugger.
Please also note: I have also tried modifying the code to have an
autodelete thread as the deallocation thread; this way, I avoided a
WaitFor in the UI altogether (even though, even in the initial case,
the WaitFor would return immediatly as the thread would already have
exited by the time I called it) and unfortunately, I still get lots of
CString's that are not deallocated.
Thanks for your patience :)
Marius