views:

621

answers:

3

My application creates a thread and that runs in the background all the time. I can only terminate the thread manually, not from within the thread callback function. At the moment I am using TerminateThread() to kill that thread but it's causing it to hang sometimes. I know there is a way to use events and WaitForSingleObject() to make the thread terminate gracefully but I can't find an example about that.

Please, code is needed here.

+5  A: 

TerminateThread is a bad idea, especially if your thread uses synchronization objects such as mutexes. It can lead to unreleased memory and deadlocks, so you're correct that you need to do something else.

Typically, the way that a thread terminates is simply to return from the function that defines the thread. Generally, the main thread signals the worker thread to exit using an event object or a simply an integer or boolean value. If the worker thread is waiting in a WaitForSingleObject, you may need to change it to a WaitForMultipleObjects, where one of the objects is an event. The main thread would call SetEvent and the worker thread would wake up and return.

We really can't provide any useful code unless you show us what you're doing. Depending on what the worker thread is doing and how your main thread is communicating information to it, it could look very different.

Also, under MSVC, you need to use _beginthreadex instead of CreateThread in order to avoid memory leaks in the CRT. See MSKB #104641.

Update:

One use of worker thread is as a "timer", to do some operation on regular intervals. At the most trivial:

for (;;) {
    switch (WaitForSingleObject(kill_event, timeout)) {
        case WAIT_TIMEOUT: /*do timer action*/ break;
        default: return 0; /* exit the thread */
    }
}

Another use is to do something on-demand. Basically the same, but with the timeout set to INFINITE and doing some action on WAIT_OBJECT_0 instead of WAIT_TIMEOUT. In this case you would need two events, one to make the thread wake up and do some action, another to make it wake up and quit:

HANDLE handles[2] = { action_handle, quit_handle };
for (;;) {
    switch (WaitForMultipleObject(handles, 2, FALSE, INFINITE)) {
        case WAIT_OBJECT_0 + 0: /* do action */ break;
        default:
        case WAIT_OBJECT_0 + 1: /* quit */ break;
    }
}

Note that it's important that the loop do something reasonable if WFSO/WFMO return an error instead of one of the expected results. In both examples above, we simply treat an error as if we had been signaled to quit.

Tim Sylvester
The thread callback function is a for(;;) that's all. nothing out of the ordinary. so, i'll repeat the question, using WaitForSingleObject() and SetEvent() how can i signal my thread that I need to finish? If I call SetEvet(FinishEvent) WaitForSingleObject() will return what? 0? 1? -1?
Uri
@Uri When you call `SetEvent`, `WaitForSingleObject` will return `WAIT_OBJECT_0` (which is 0). When you mentioned using `WaitForSingleObject` I assumed that meant you were already using the event to signal the worker thread to do something (other than exiting). Also note that in some cases you need to signal the wotker thread to exit and then wait for it to actually stop before continuing. (You can use `WaitForSingleObject` on the thread handle for this purpose).
Tim Sylvester
+1 This is the answer I would have given. Did it this way many times.
ur
The MSKB article you link indicates it only applies until MSVC 6.0 - does that leak still occur with modern versions?
bdonlan
So I am creating an event with CreateEvent(), then call SetEvent() and inside the callback function for my thread I call WaitForSingleObject() but that always returns an error 6, invalid handle. I am passing the handle returned by CreateEvent()
Uri
Oh, never mind. I was closing the handle to the event *before* the thread had time to do anything. thanks for the help
Uri
@Uri In your main thread are you calling `SetEvent` and then `CloseHandle`? If so, the handle is being closed before the worker thread gets the event signal. Technically, the thread getting an error and stopping means the event is doing what it's meant for, but the "correct" way to proceed is to set the event then wait for the thread to stop before closing the event handle.
Tim Sylvester
@bdonlan Interesting, I didn't see that. As far as I understand the issue, it's not "fixed" and never will be because it's not a bug. It's possible that MS has some workaround in later versions, but it's more likely that they just failed to update the KB article. I would still use `_beginthreadex` unless you find something specifically saying otherwise.
Tim Sylvester
A: 

Since you don't know what the thread is doing, there is no way to safely terminate the thread from outside.

Why do you think you cannot terminate it from within?

You can create an event prior to starting the thread and pass that event's handle to the thread. You call SetEvent() on that event from the main thread to signal the thread to stop and then WaitForSingleObject on the thread handle to wait for the thread to actually have finished. Within the threads loop, you call WaitForSingleObject() on the event, specifying a timeout of 0 (zero), so that the call returns immediately even if the event is not set. If that call returns WAIT_TIMEOUT, the event is not set, if it returns WAIT_OBJECT_0, it is set. In the latter case you return from the thread function.

I presume your thread isn't just burning CPU cycles in an endless loop, but does some waiting, maybe through calling Sleep(). If so, you can do the sleeping in WaitForSingleObject instead, by passing a timeout to it.

sbi
the thread is not doing anything, only checking that a specific program is always up and running.
Uri
A: 

What are you doing in the background thread? If you're looping over something, you can end the thread within itself by having a shared public static object (like a Boolean) that you set to true from the foreground thread and that the background thread checks for and exits cleanly when set to true.

asbjornu