?
Solved

Killing worker thread

Posted on 2004-07-31
32
Medium Priority
?
966 Views
Last Modified: 2012-08-13
I want to kill a worker thread that is hanging with waitForSingleObject.
I want to kill it from the parent dialog.

Basically I created a thread using CreateThread.
In that thread I call ShellExecute to run another application.
I used WaitForSingleObject INFINITY to cause my app to
wait for the results of the external application.

The external application creates a file. The WaitForSingleObject
is necessary because the next line of code in the thread uses that
file to load a list box.

The thread was neccessary becaue I wanted to display an animated
Please wait dialog. This dialog is modless.

I believe I needed it to be modless because when it was modal it
simply stalled the other processes from going.

So basically a function in my main dialog creates two threads.
The modeless dialog and the Thread.

When the Thread ends it uses SendMessage to close the Please wait dialog
Also it uses a pointer that was passed to lparam that allows it to call
a function to load the list box.

res is the return value of ShellExecute.

else //res==TRUE
{
WaitForSingleObject(si.hProcess,INFINITE);
if(SendMessage( pParms->PleaseHWND, WM_CLOSE, 0, 0 )!=0)
MessageBox(NULL,"failed to call wm_close","Error",MB_OK);
pParms->pSc->LoadListFromOutputTXT();
ReturnFlag=0;
}//endelse

Then the thread func returns zero. When done normally.

But it seems that I can not close the thread early if I add a button to
the Please Wait dialog to instruct the thread to close early.

I am stuck not knowing what to do. I need the thread to call the application up to create the text file for the listbox to use. I would like to have a button on the please wait dialog to stop in case this is not what the user wants.

I am wondering now if I should have used afxthread thing or should I have a GUI thread?

Any comments apreciated.
RJ
0
Comment
Question by:RJSoft
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 18
  • 11
  • 3
32 Comments
 
LVL 86

Expert Comment

by:jkr
ID: 11685273
You could go the "brutal" way and call 'TerminateThread()'
0
 
LVL 86

Accepted Solution

by:
jkr earned 2000 total points
ID: 11685285
>>In that thread I call ShellExecute to run another application.
>>I used WaitForSingleObject INFINITY to cause my app to
>>wait for the results of the external application.

That makes a "nicer" solution possible - use a termination event, e.g.

HANDLE hEvent = CreateEvent ( NULL, FALSE, FALSE, NULL);

CreateThread ( NULL, 0, MyThreadProc, (LPVOID) hEvent, 0, &dwTID);

//...

ULONG WINAPI MyThreadProc ( LPVOID pv) {

HANDLE hEvent = (HANDLE) pv;

// start external app

while ( WAIT_TIMEOUT == WaitForSingleObject ( hProcess, 1000)) {

    // check event state
    if ( WAIT_TIMEOUT != WaitForSingleObject ( hEvent, 1)) ExitThread ( -1);
}

// ...
}

and call 'SetEvent()' when your button is pressed, causing the thread to terminate.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11685300
Thanks. I give this a try for sure.
RJ
0
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
LVL 86

Expert Comment

by:jkr
ID: 11685327
The 'termination event' is the 'common' solution for such a problem, however it requires periodically checking the event's state. Since you can break down a 'WaitForSingleObject ( hProcess, INFINITE)' into smaller waiting periods without any noticeable performance penalty, this should be the way to go.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686324
Ok I changed my code as stated but now it hangs as though the external app never gets done.

Also it seems SetEvent has no effect in breaking the loop.

I feel like I got the handles wrong or something. So I tried different combinations.

The above code snippet you did not have a return handle for CreateThread. I assumed you meant to put one there for it.

Here is the thread code.

///////////////////////////////////////////THREAD FUNC

//This is global structure so that ThreadFunc can make calls to parent thread functions
//It is instantiated with new on the heap. It is later deleted when parent is closed.
//pParms = new PARMS;

struct PARMS
{
HWND PleaseHWND;
HWND ParentHWND;
char TheParms[MYMAXPATH];
SearcherDlg *pSc;
};
struct PARMS *pParms;




DWORD WINAPI ThreadFunc2(LPVOID lp)
{
int ReturnFlag=0;
MyMessageBoxIdok My;
CPlayerDlg*Cp=(CPlayerDlg*)::GetParent(pParms->ParentHWND);
//
HANDLE hEvent = (HANDLE) lp;
//
SHELLEXECUTEINFO si;
memset(&si, 0, sizeof(si));
si.cbSize = sizeof(si);
si.hwnd =   pParms->ParentHWND;
si.lpVerb = "open";
si.lpFile = "XXXXX.exe";
si.lpParameters = pParms->TheParms;
si.nShow = SW_HIDE;//;SW_NORMAL;
si.fMask = SEE_MASK_NOCLOSEPROCESS;
BOOL res = ShellExecuteEx(&si);
//
if(res==FALSE)//app did not execute
{
            LPVOID lpMsgBuf;
            FormatMessage(
            FORMAT_MESSAGE_ALLOCATE_BUFFER |
            FORMAT_MESSAGE_FROM_SYSTEM |
            FORMAT_MESSAGE_IGNORE_INSERTS,
            NULL,
            GetLastError(),
            MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
            (LPTSTR) &lpMsgBuf,
            0,
            NULL
            );
            // Process any inserts in lpMsgBuf.
            // ...
            // Display the string.
            strcpy(My.T1,(LPCTSTR)lpMsgBuf);
            strcat(My.T1," ERROR 042");
            My.How=0;//-1;
            
            if(Cp->DoBark)My.Bark=1;
            My.DoModal();
            // Free the buffer.
            LocalFree( lpMsgBuf );
            
            if(SendMessage( pParms->PleaseHWND,WM_CLOSE, 0, 0 )!=0)
            MessageBox(NULL,"failed to call wm_closepleasewait","test",MB_OK);
            //pParms->pSc->DidASearch=1;
            ReturnFlag=1;

}//endiferror
//
else //res==TRUE App did execute
{
                                                                  //pParms->pSc->hT (This is handle from CreateThread)      
while ( WAIT_TIMEOUT == WaitForSingleObject ( pParms->pSc->hT, 1000))
{
// check event state
if ( WAIT_TIMEOUT != WaitForSingleObject ( hEvent, 1)) ExitThread (0);// -1);
}//endwhile

//This messagebox never shows up
MessageBox(NULL,"app done","done",MB_OK);

//To close Please wait dialog
if(SendMessage( pParms->PleaseHWND, WM_CLOSE, 0, 0 )!=0)
MessageBox(NULL,"failed to call wm_close","Error",MB_OK);

//To load up listbox from a file
pParms->pSc->LoadListFromOutputTXT();
ReturnFlag=0;
}//endelse

//Parent function that closes thread
pParms->pSc->CloseTheThread();
return ReturnFlag;
}//endfunc
/////////////////////////////////////////////////////ENDTHREADFUNC


///////////////part of dialog function that calls the thread up

ParentDialog...

//The please wait dialog (Pwait is public member owned by parent dialog)
if(Pwait)Pwait.DestroyWindow();
Pwait.Create(IDD_PLEASEWAITDIALOG);
Pwait.ShowWindow(SW_NORMAL);
//

//Load parametors into global structure
pParms->PleaseHWND=Pwait.m_hWnd;
pParms->ParentHWND=this->m_hWnd;
strcpy(pParms->TheParms,Parms);//parm string for ShellExecute
pParms->pSc=this; //pointer to this Parent dialog so ThreadFunc can make
//function calls that are owned by Parent dialog

//hEvent and hT are public members of this dialog
//hT is the handle to the thread.

DWORD lpThreadID;//Not sure what this is used for

hEvent = CreateEvent ( NULL, FALSE, FALSE, NULL);
hT=CreateThread ( NULL, 0, ThreadFunc2, (LPVOID) hEvent, 0, &lpThreadID);

}//endfunc

Any comments apreciated.
RJ
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686379
>>                                                       //pParms->pSc->hT (This is handle from CreateThread)    
>>while ( WAIT_TIMEOUT == WaitForSingleObject ( pParms->pSc->hT, 1000))

You need to wait for the process handle here:

while ( WAIT_TIMEOUT == WaitForSingleObject ( si.hProcess, 1000))
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686383
Ok. makes sense.
Thanks
RJ
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686470
Ok. Now the normal termination is ok. But the SetEvent fails. The message is that the handle is invalid.

hEvent is created in the function that starts up the thread as posted above.
Guess I should check if CreateEvent returned a good handle in the first place.

hEvent = CreateEvent ( NULL, FALSE, FALSE, NULL);
hT=CreateThread ( NULL, 0, ThreadFunc2, (LPVOID) hEvent, 0, &lpThreadID);

hEvent is owned by the calling dialog so the handle should be ok. hEvent is type HANDLE.

Any ideals?

RJ

//handle is invalid
void SearcherDlg::CallTheEvent()
{
      if(SetEvent(hEvent)==0)
      {
            LPVOID lpMsgBuf;
            FormatMessage(
            FORMAT_MESSAGE_ALLOCATE_BUFFER |
            FORMAT_MESSAGE_FROM_SYSTEM |
            FORMAT_MESSAGE_IGNORE_INSERTS,
            NULL,
            GetLastError(),
            MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
            (LPTSTR) &lpMsgBuf,
            0,
            NULL
            );
            // Process any inserts in lpMsgBuf.
            // ...
            // Display the string.
            
            MyMessageBoxIdok My;
            strcpy(My.T1,(LPCTSTR)lpMsgBuf);
            strcat(My.T1,"\nCALL EVENT ERROR");
            My.How=0;//-1;
            My.DoModal();
            // Free the buffer.
            LocalFree( lpMsgBuf );

      }//endif
}//endfunc
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686479
Is the event handle still valid when you are trying to set it?
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686490
I tested for NULL after I created the hEvent with
hEvent = CreateEvent ( NULL, FALSE, FALSE, NULL);
if(hEvent==NULL)
{
...
}

But it gives me no error.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686495
NULL is not a relevant test - what about

     if(SetEvent(hEvent)==0) {

         //...

    } else {

        // FAILURE

        DWORD dw = GetLast Error ();
    }
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686504
I changed it as you recommend. But still does not break the loop.

Also in my help doc it states that SetEvent returns 0 if failed. That is what I tested for previously and I think I should switch it back.

Here is my VC++6.0 doc about returns from SetEvent.

I am wondering now if I have to do more than just call SetEvent to break out of the loop

RJ


////////////////////////////////////////////////////
Return Values
If the function succeeds, the return value is nonzero.

If the function fails, the return value is zero. To get extended error information, call GetLastError.

Remarks
The state of a manual-reset event object remains signaled until it is set explicitly to the nonsignaled state by the ResetEvent function. Any number of waiting threads, or threads that subsequently begin wait operations for the specified event object by calling one of the wait functions, can be released while the object's state is signaled.

The state of an auto-reset event object remains signaled until a single waiting thread is released, at which time the system automatically sets the state to nonsignaled. If no threads are waiting, the event object's state remains signaled.
////////////////////////////////////////////////////////////////////////////
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686506
But the handle is invalid or so GetLastError states that.

//handle is invalid
void SearcherDlg::CallTheEvent()
{
     if(SetEvent(hEvent)==0)
     {
          LPVOID lpMsgBuf;
          FormatMessage(
          FORMAT_MESSAGE_ALLOCATE_BUFFER |
          FORMAT_MESSAGE_FROM_SYSTEM |
          FORMAT_MESSAGE_IGNORE_INSERTS,
          NULL,
          GetLastError(),
          MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
          (LPTSTR) &lpMsgBuf,
          0,
          NULL
          );
          // Process any inserts in lpMsgBuf.
          // ...
          // Display the string.
         
          MyMessageBoxIdok My;
          strcpy(My.T1,(LPCTSTR)lpMsgBuf);
          strcat(My.T1,"\nCALL EVENT ERROR");
          My.How=0;//-1;
          My.DoModal();
          // Free the buffer.
          LocalFree( lpMsgBuf );

     }//endif
}//endfunc

When I tested for NULL that was with the CreateEvent. Should I test for zero with CreateEvent?

RJ
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686507
Also I adjusted the 1000 to 100 just to see if it would normally pop out of the loop early as timed out. But I am not fully understanding how this works.

RJ
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686510
>> But the handle is invalid or so GetLastError states that.

That's what I was afraid of.

>>Should I test for zero with CreateEvent?

That's for sure. But, is it a local variable or how is it set up in general?
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686511
It did not pop out early with lower value of 100.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686517
hEvent is declared in the calling dialog as a public member. So I know this is not a scope problem as the variable should exist during the life of the thread creating dialog. Which is parent to all of it.

I will test for zero on CreateEvent.

RJ
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686518
>>But I am not fully understanding how this works

You mean

while ( WAIT_TIMEOUT == WaitForSingleObject ( hProcess, 1000)) {
}

?

This loop exists when the object (the process) finally terminated, just like

WaitForSingleObject ( hProcess, INFINITE);

would, but gives you the chance to check the termination even in predefined intervals.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686526
So the 1000 is the timespan of  intervals when the event is checked to see if it is signaled?
Is this true?

Also I checked for the return of CreateEvent to see if it returned zero it did not so I take it that the handle was ok.

Is this possibly a windows 98 thing. That is what I am developing on.

here is my help file for CreateEvent. It says I should test for NULL.

RJ

/////////////////////////
Return Values
If the function succeeds, the return value is a handle to the event object. If the named event object existed before the function call, the function returns a handle to the existing object and GetLastError returns ERROR_ALREADY_EXISTS.

If the function fails, the return value is NULL. To get extended error information, call GetLastError.

Remarks
The handle returned by CreateEvent has EVENT_ALL_ACCESS access to the new event object and can be used in any function that requires a handle to an event object.

Any thread of the calling process can specify the event-object handle in a call to one of the wait functions. The single-object wait functions return when the state of the specified object is signaled. The multiple-object wait functions can be instructed to return either when any one or when all of the specified objects are signaled. When a wait function returns, the waiting thread is released to continue its execution.

The initial state of the event object is specified by the bInitialState parameter. Use the SetEvent function to set the state of an event object to signaled. Use the ResetEvent function to reset the state of an event object to nonsignaled.

When the state of a manual-reset event object is signaled, it remains signaled until it is explicitly res
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686532
>> So the 1000 is the timespan of  intervals when the event is checked to see if it is signaled?

Yup, every second when the enclosing loop's body executes.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686533
I will add the hEvent to global memory and see if that helps. Maybe the handle is getting kicked out of memory.
RJ
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686535
1000ms, that is...
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686539
Oh ok. So I dont have to worry about the external app taking too long and the loop timing out.

You would think by the way that things are phrased that would happen.
WAIT_TIMEOUT

or is that an actual value that will cause the loop to break eventually?

RJ
0
 
LVL 86

Expert Comment

by:jkr
ID: 11686548
You could try to wait for the event a lil' longer than 1ms, but that's a wild guess now...
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686562
My external application uses the internet to obtain values so it can create the text file which get loaded into the listbox.

I could be having slow internet problems?

I put the hEvent handle in the global structure that is put in the heap. Also changed the interval value back to 1000.

Now it is hanging.

But I do not get the invalid handle message. Im going to put a MessageBox statement to make sure SetEvent is being called.
RJ
0
 
LVL 19

Expert Comment

by:drichards
ID: 11686627
You might also want to try WaitForMultipleObjects rather than the loop and two WaitForSingleObject calls:
-----------------------------------
HANDLE waitObjects[2];
waitObjects[0] = si.hProcess;
waitObjects[1] = hEvent;
// Could also do: HANDLE waitObjects[] = { si.hProcess, hEvent };
switch (WaitForMultipleObjects(2, waitObjects, FALSE, INFINITE))
// Use number of milliseconds instead of INFINITE if you want to time out
//  even if neither condition is signalled
{
case WAIT_OBJECT_0:
    // Process ended...
    break;
case (WAIT_OBJECT_0+1):
    // Termination event signalled...
    break;
case WAIT_TIMEOUT:
    // Wait timed out...
    break;
}
---------------------------------------

As for the event problem, what exactly are the values of hEvent immediately at creation and then at the call to SetEvent?  Also after casting it in the thread procedure?
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686639
Well got it to work. All I did was remove the all of the Event stuff. And passed NULL instead of hEvent to the ThreadFunc. Then I put a simple integer to break out of the loop.
Did not need to call ExitThread cause of the design of my program. Breaking the loop makes the thread exit anyway.

Points to jkr.

Thanks much.
RJ


else //res==TRUE (ShellExecute worked)
{
                                                                  
while ( WAIT_TIMEOUT == WaitForSingleObject ( si.hProcess, 500))
{
if(pParms->ExitFlag)break;
}//endwhile

if(SendMessage( pParms->PleaseHWND, WM_CLOSE, 0, 0 )!=0)
MessageBox(NULL,"failed to call wm_close","Error",MB_OK);
pParms->pSc->LoadListFromOutputTXT();
ReturnFlag=0;
}//endelse
//
pParms->pSc->CloseTheThread();
return ReturnFlag;
}//endfunc
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686658
drichards

Doesnt the INFINIT flag cause the switch to wait untill the called app is done?

switch (WaitForMultipleObjects(2, waitObjects, FALSE, INFINITE))

RJ
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11686668
drichards;

>>As for the event problem, what exactly are the values of hEvent immediately at creation and then at the call to SetEvent?  Also after casting it in the thread procedure?

It came back as normal. I even put an explicit call to GetLastError and it told me successful.


Also I am trying to understand your case statements.

Does WaitForMultipleObjects get called repeatedly. Or is this only a oneshot case statement. If so, does this mean that WaitForMultipleObjects recieves messages??

case WAIT_OBJECT_0:
    // Process ended...
    break;
case (WAIT_OBJECT_0+1):
    // Termination event signalled...
    break;
case WAIT_TIMEOUT:
    // Wait timed out...
    break;

I believe that the switch statement only gets executed once and that would be after the application object is finished. But my point is to terminate the thread early.

But I know I could be all wrong. Please explain.

RJ
0
 
LVL 19

Expert Comment

by:drichards
ID: 11686749
This one call takes care of normal thread termination AND early terminattion in either of two ways.  The INFINITE timeout does NOT cause it to wait until the called app is done.  It just means the app has to finish OR the hEvent has to be signalled.  Thie timeout is a third option (hence three cases in the switch).

WaitForMultipleObjects is a one-shot call.  It will wait for any one of the sync object handles to be signalled if you use FALSE as the third param or will wait until all of them are signalled if you use TRUE (TRUE not useful in this case).  The last param is how long you want it to wait.  It would eliminate the loop you've currently got as it will wait simultaneously on all the handles in the array.  Then you don't have to reak out of one wait to go check another as you're currently doing.

The switch statement will only be executed once when the call returns.  The switch is to determine what caused the call to return.  It can be any of the events.  It can be a timeout if you use something other than INFINITE as the last parameter.  With this one call you can get the thread to terminate early if either the termination event (hEvent) is signalled or if the timeout value in the last parameter is exceeded.  If you want it to terminate ONLY by user intervention or the process ending, use INFINITE as the wait.
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11690692
drichards;

Thanks

much apreciated.

RJ
0

Featured Post

Free Tool: Path Explorer

An intuitive utility to help find the CSS path to UI elements on a webpage. These paths are used frequently in a variety of front-end development and QA automation tasks.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Often, when implementing a feature, you won't know how certain events should be handled at the point where they occur and you'd rather defer to the user of your function or class. For example, a XML parser will extract a tag from the source code, wh…
  Included as part of the C++ Standard Template Library (STL) is a collection of generic containers. Each of these containers serves a different purpose and has different pros and cons. It is often difficult to decide which container to use and …
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.

765 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question