Solved

Killing worker thread

Posted on 2004-07-31
32
959 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
  • 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 500 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
 
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
How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

 
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 19

Expert Comment

by:drichards
ID: 11686753
0
 
LVL 3

Author Comment

by:RJSoft
ID: 11690692
drichards;

Thanks

much apreciated.

RJ
0

Featured Post

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Introduction This article is the first in a series of articles about the C/C++ Visual Studio Express debugger.  It provides a quick start guide in using the debugger. Part 2 focuses on additional topics in breakpoints.  Lastly, Part 3 focuses on th…
Container Orchestration platforms empower organizations to scale their apps at an exceptional rate. This is the reason numerous innovation-driven companies are moving apps to an appropriated datacenter wide platform that empowers them to scale at a …
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

747 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

Need Help in Real-Time?

Connect with top rated Experts

14 Experts available now in Live!

Get 1:1 Help Now