User thread problem

[Roland]

Hello There
I am using an User interface thread. I'll just explain my
functionality and then move towards problem.
When the document is navigated completelly (WebBrowser),I am
starting a user interface thread and storing thread pointer in
m_pCurrentThread. When new document is navigated again ,I want to stop
the previous thread and start the new thread again. So I maintained
the 2 thread (previos and current ) in two varaibles,like this

void OnDocumnetComplete()
{
// m_pPrevThread and m_pCurrThread are NULL initially (in
constructor)

if (m_pPrevThread != NULL )
{
// If Previous Thread exists Then forse it to terminate
m_pPrevThread->m_bAbort=TRUE;
m_pPrevThread->PostThreadMessage(UWM_TERMINATE_THREAD,0,0);
}

// New thread is created
m_pCurrThread=(CDiscoveryThread*)AfxBeginThread(RUNTIME_CLASS
(CDiscoveryThread),THREAD_PRIORITY_NORMAL,0,CREATE_SUSPENDED);

if (m_pCurrThread)
{
m_pCurrThread->m_pParent=this;
m_pCurrThread->ResumeThread();
m_pPrevThread=m_pCurrThread;
}
}

But sometimes application is crashing arbitrally at
m_pThread->PostThreadMessgae(...);
when I saw the value of m_pPrevThread , it was valid but the handle of
thread was NULL. Can you tell me what would the possible reason of
this ? How can I get rid off this problem ? Can you suggest any
another idea to do this ?
Thansk in advance.

[Doug Harrison [MVP]]

Roland wrote:

It's probably crashing because you're allowing the default auto-delete
behavior. 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.

***** About using a quit-event instead of bool "quit" variables (assumes a
worker thread, though, or a message loop based on
MsgWaitForMultipleObjects):

Events are much more flexible. They support polling with
WaitForSingleObject, but you can also use them in WaitForMultipleObjects.
This comes in handy when you'd like to wait forever on some other object.
You can't use WaitForSingleObject(INFINITE), because you'd miss thread exit
requests that are made before the object you're waiting on is signaled. Some
people solve this problem by waiting a second or two and polling their
"quit" variable in a loop. But if you use a quit-event and store it as the
first object in the WFMO handle array, you can wait forever, which means you
wait more efficiently, and you normally don't need the "while (running)"
loop. My code tends to look like this:

// WaitAny is overloaded on up to 5 WaitableHandles, and it waits forever.
// quitEvent is an Event object, and Event is derived from WaitableHandle.
switch (WaitAny(quitEvent, other WaitableHandles))
{
case WAIT_OBJECT_0 :
clean up and exit;
...other cases
}

***** So, assuming you can arrange for your thread to exit in a timely
matter, I'd replace:

m_pPrevThread->m_bAbort=TRUE;
m_pPrevThread->PostThreadMessage(UWM_TERMINATE_THREAD,0,0);

with:

SetEvent(m_pPrevThread->quitEvent);
WaitForSingleObject(m_pPrevThread->m_hThread, INFINITE);
delete m_pPrevThread;
m_pPrevThread = 0;

If you can't arrange for your thread to exit quickly, or if there's any
possibility of indefinite waiting (including deadlock) when you call WFSO,
and you can't fix the problem, you could have your thread post a message
back to the "owner" thread right before it terminates. Then the owner could
join with the thread before deleting the thread object. See this KB article
for some notes on using PostThreadMessage:

PRB: PostThreadMessage Messages Lost When Posted to UI Thread
http://support.microsoft.com/default.aspx?scid=kb;en-us;183116

All that said, is there any reason you have to kill the original thread and
start a new one? Is there any reason you can't find a way to reuse the
original thread?

--
Doug Harrison
Microsoft MVP - Visual C++

[Roland]

I read your reply very carefully. One thing that I'd like to tell you
that my thread is doing some busy operation. I have set certain
checkpoints in thread where I am using PeekMessage to check existance
of Cancel Message in the message ( Main thread sends the Cancel
message ) and if there, it processes message and exits thread. But I'm
not sure when it will receive Cancel message and exit itself. So
before starting a new thread , I am passing Cancel message to previous
thread, starting new thread , after some time previous thread will be
exited after receiving Cancel message.
I set the m_bAutoDelete to FALSE as suggested by you and it
really solved my problem. But it may lead to memory leak. HOw can I
solve this problem now ?

if (m_pPrevThread != NULL )
{

// If Previous Thread exists Then forst terminate it
m_pPrevThread->m_bAbort=TRUE;
m_pPrevThread->PostThreadMessage(UWM_TERMINATE_THREAD,0,0);

}

// New thread is created
m_pCurrThread=(CDiscoveryThread*)AfxBeginThread
(RUNTIME_CLASS(CDiscoveryThread),THREAD_PRIORITY_NORMAL,0,CREATE_SUSPENDED);
if (m_pCurrThread)
{

m_pCurrThread->m_bAutoDelete=FALSE;

[Doug Harrison [MVP]]

Roland wrote:

If you aren't doing any other message processing, using an event object and
checking with WaitForSingleObject(hEvent, 0) would be a better choice in
general for reasons given in my last message. In addition, it would avoid a
race condition in posting the message, because a thread which has not
created a message queue cannot receive messages:

PostThreadMessage
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/WinUI/WindowsUserInterface/Windowing/MessagesandMessageQueues/MessagesandMessageQueuesReference/MessagesandMessageQueuesFunctions/PostThreadMessage.asp
<q>
The thread to which the message is posted must have created a message queue,
or else the call to PostThreadMessage fails. Use one of the following
methods to handle this situation:

Call PostThreadMessage. If it fails, call the Sleep function and call
PostThreadMessage again. Repeat until PostThreadMessage succeeds.
Create an event object, then create the thread. Use the WaitForSingleObject
function to wait for the event to be set to the signaled state before
calling PostThreadMessage. In the thread to which the message will be
posted, call PeekMessage as shown here to force the system to create the
message queue.
PeekMessage(&msg, NULL, WM_USER, WM_USER, PM_NOREMOVE)
Set the event, to indicate that the thread is ready to receive posted
messages.
</q>

I don't recommend the Sleep method, as it's rather indeterminate in its
behavior.

>But I'm
>not sure when it will receive Cancel message and exit itself. So
>before starting a new thread , I am passing Cancel message to previous
>thread, starting new thread , after some time previous thread will be
>exited after receiving Cancel message.
> I set the m_bAutoDelete to FALSE as suggested by you and it
>really solved my problem. But it may lead to memory leak. HOw can I
>solve this problem now ?

Like I said:

>> If you set m_bAutoDelete to false, you assume the responsibility for deleting the CWinThread".
>> ...

>>So, assuming you can arrange for your thread to exit in a timely matter

>> SetEvent(m_pPrevThread->quitEvent);
>> WaitForSingleObject(m_pPrevThread->m_hThread, INFINITE);
>> delete m_pPrevThread;
>> m_pPrevThread = 0;

>>...

>>If you can't arrange for your thread to exit quickly, or if there's any
>>possibility of indefinite waiting (including deadlock) when you call WFSO,
>>and you can't fix the problem, you could have your thread post a message
>>back to the "owner" thread right before it terminates. Then the owner could
>>join with the thread before deleting the thread object.

So at some point, you're going to have to delete the CWinThread object.

[Roland]

Ohh.. Thanku so much for your help
I did exactly same as you told.
As well as side by side I am just trying to fit this
functionality using one thread instead of two. BUt now I am facing one
wierd problem. I am posting message to thread using PostThreadMessage,
but somehow thread is not receiving it. I tried a lot to do this. But
I couldn't succeed. I am not displaying dialog or message box in the
thread. I have not overrideen any method of CWinThread ( Run(),
PreTranslateMessage()...). When I checked the return value of
PostThreadMessage , it showed me Error in ... in quick watch.
I am doing like this.

if (m_pCurrThread)
{
m_pCurrThread = new CMyThread();
m_pCurrThread->CreateThread();
m_pCurrThread->PostThreadMessage(UWM_START_THREAD,(WPARAM)0,(LPARAM)0);
}

In Thread I wrote handler

BEGIN_MESSAGE_MAP
ON_THREAD_MESSAGE(UWM_START_THREAD,OnStartThread)(#define
OnStartThread 100)
END_MESSAGE_MAP

void CMyThread::OnStartThread(WPARAM wParam,LPARAM lParam)
{
ATLTRACE("Thread started");
}

Can you please tell me that what would be the probable reason behind
this ?

[Doug Harrison [MVP]]

Roland wrote:

>Ohh.. Thanku so much for your help
>I did exactly same as you told.
> As well as side by side I am just trying to fit this
>functionality using one thread instead of two. BUt now I am facing one
>wierd problem. I am posting message to thread using PostThreadMessage,
>but somehow thread is not receiving it. I tried a lot to do this. But
>I couldn't succeed. I am not displaying dialog or message box in the
>thread. I have not overrideen any method of CWinThread ( Run(),
>PreTranslateMessage()...). When I checked the return value of
>PostThreadMessage , it showed me Error in ... in quick watch.
>I am doing like this.

What was the error?

>if (m_pCurrThread)
>{
> m_pCurrThread = new CMyThread();
> m_pCurrThread->CreateThread();
> m_pCurrThread->PostThreadMessage(UWM_START_THREAD,(WPARAM)0,(LPARAM)0);
>}

Did you read the documentation I posted which talked about PostThreadMessage
failing if the target thread doesn't have a message queue?

>In Thread I wrote handler
>
>BEGIN_MESSAGE_MAP
> ON_THREAD_MESSAGE(UWM_START_THREAD,OnStartThread)(#define
>OnStartThread 100)
>END_MESSAGE_MAP

That #define doesn't look right, and #defines don't belong within
BEGIN_MESSAGE_MAP/END_MESSAGE_MAP.

>void CMyThread::OnStartThread(WPARAM wParam,LPARAM lParam)
>{
> ATLTRACE("Thread started");
>}
>
>
>Can you please tell me that what would be the probable reason behind
>this ?

It would be useful to know the errors you're seeing, but my best guess is
you're trying to post a message to a thread which hasn't got a message
queue. I talked about that a couple of messages ago and posted a bit of the
documentation which explains how to avoid this problem.