Proper Cleanup if Thread Fails to Close - C++

When my application is ready to close the tread it created using CreateThread the following algorithm is executed:

_bCloseRequested = TRUE;
dwMsThen = ::GetTickCount();

do
{
    ::GetExitCodeThread( m_hThread, &dwExitCode );
    dwMsNow = ::GetTickCount();
}
while( (dwExitCode == STILL_ACTIVE) && ((dwMsNow - dwMsThen) < 50000UL) );

If the thread fails to close within the 5 allotted seconds, should the thread handle be closed, or allowed t开发者_StackOverflowo remain open? Thanks.

First, don't wait for a thread to finish like this. You will eat up all available CPU time just waiting, which has also the disadvantage that your thread will take longer to finish!

Use something like this instead:

WaitForSingleObject(m_hThread, 50000);

That said: whether you want to leave the thread running or not depends on what the thread does. Can it even run even though your main app starts doing something else? Does it have critical stuff (files, connections, databases, ...) open that would be left open if you kill the thread? You have to consider all of this before you decide whether to kill the thread or leave it running.

Just wait on the thread handle. If it takes too long, you should just timeout and terminate your app, and fix whatever bug makes the thread fail to exit.

static const DWORD TIMEOUT_VALUE(50000);

if (WaitforSingleObject(m_hThread, TIMEOUT_VALUE) != WAIT_OBJECT_0))
{
  // thread did not exit in time, log and exit process
}

Good question.

There are a couple of approaches to this.

The first approach is what I would consider to be the ideal approach. And that is to never terminate threads. The reasons for this are multiple, but here are some biggies:

1. If your thread owns a synchronization object, they won't be released
2. RAII objects don't get a chance to clean up
3. Allocated memory won't be freed
4. If you are in the middle of certian kernel calls, you could hose your entire application

So going with this approach, you would identify the reasons why the threads are not shutting down, and fix that problem. You may find that the problems run deep. You may find deadlocks, race conditions, etc. Static analysis can help to find these problems.

The ideal approach is the one you should always persue. And in doing this, it's best not to use a spin lock. Instead, Wait() on the thread handle with a timeout. By spinning, your'e wasting resources, and stealing time slices from the thread you're waiting for.

But in the real world, in production code, you need a fallback measure in case everything else fails. You should first try multiple methods to trigger your thread to shut itself down. If everything fails as an absolute last resort, kill the thread. But because of the dangers behind killing a zombie thread, once you've done this, you should restart your entire application. When you kill a thread, you can put your process in a non-deterministic state. So start over. Log an error message, shut the app down, and start again.

Neither. You should fix whatever is keeping the thread from exiting cleanly and simply join on it. Everything else is just a hack.