8

I am creating a thread using _beginthreadex function. The function address I am passing in it has an infinite while loop (while(1)) . I have the threadid and threadhandle.

I can use TerminateThread(threadhandle,1); But it is dangerous.

The safe way is to kill thread using _endthreadex but it can only be used from inside the thread, and I wanted to kill the thread from outside.

So please suggest if there is a safe way to close,end or kill the thread safely from outside using threadid or threadhandle.

undur_gongor
  • 15,657
  • 5
  • 63
  • 75
lsrawat
  • 157
  • 3
  • 9

3 Answers3

14

You should - literally - never use TerminateThread(). And I'm not even joking. If you are terminating a thread from the outside, all resources reserved in it will be leaked, all state variables accessed inside will have an undetermined state and so on.

The solution for your problem might be signaling your thread to finish itself. It can be done by a volatile variable changed by thread-safe means (see InterlockedIncrement() on that), a Windows event, or something like that. If your thread has a message loop you can even do it by sending a message to ask it to stop.

mg30rg
  • 1,311
  • 13
  • 24
  • Yes I read all this on msdn also. But I thought may be if there is a way to do this (kill thread safely from outside). Actually to modify the while loop that is inside the thread was my last option. But it seems that's the only option. – lsrawat Mar 25 '15 at 09:37
  • @Israwat - Believe me, I develop heavily multithreaded applications (servers mostly), and killing a thread from the outside is a really bad practice leading to enormous headache. Also, if you are using a framework like MFC, you will leak a lot of memory, since MFC threads reserve a lot of memory for background MFC objects. – mg30rg Mar 25 '15 at 09:40
8

The proper way is to create an event "kill me", by using CreateEvent, then flag this event when you wish to kill the thread. Instead of having your thread wait while(1), have it wait while(WaitForSingleObject(hevent_killme, 0)). And then you can simply let the thread callback finish and return, no need to call _endthreadex or such.

Example of callback function:

static DWORD WINAPI thread_callback (LPVOID param)
{
  ...
  while(WaitForSingleObject(hevent_killme, 0) != WAIT_OBJECT_0)
  {
    // do stuff
  }

  return 0;
}

Caller:

HANDLE hevent_killme = CreateEvent(...);
...

void killthread (void)
{
  SetEvent(hevent_killme);
  WaitForSingleObject(hthread_the_thread, INFINITE);
  CloseHandle(hevent_killme);
  CloseHandle(hthread_the_thread);
}

Never use TerminateThread.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • while(1) loop is inside the thread function. I am not waiting for thread too terminate in an infinite while loop . – lsrawat Mar 25 '15 at 09:32
  • @lsrawat Yes I understood that, my answer is written with that assumption. I'll clarify the answer a bit. – Lundin Mar 25 '15 at 09:34
  • @Israwat - You don't6 have to. Lundin advises you to examine the state of the event, not the thread. – mg30rg Mar 25 '15 at 09:34
  • Ok now I under stood , I hope its similar to what mg30rg or simurg suggested in below answer. – lsrawat Mar 25 '15 at 09:40
  • @lsrawat Added an example. – Lundin Mar 25 '15 at 09:44
  • 2
    Use a bool variable, then test it everytime in the while(1) loop...? –  Mar 25 '15 at 09:44
  • @user9000 Events are better, as you can wait for them effectively with your thread/process asleep. Suppose you don't need your thread to work 100% of the time, for example. – Lundin Mar 25 '15 at 09:46
  • @Lundin I'd rather stay away from Winblows state of the art technologies and use an enum since Winblows successfully defeated the purpose of portable languages. Using Assembly nowadays is better than coding under Windows –  Mar 25 '15 at 09:47
  • @user9000 Or even better, don't use a 'while(1)'/'while(true)' loop, but one which has a proper exit condition. (More of an advice than a rule.) You will have a source code easier to maintain, and no additional headache when a problem like this occurs. – mg30rg Mar 25 '15 at 09:51
  • 1
    @user9000 Good for you. This question is however about the Windows-specific beginthread() function, so your rant isn't relevant. If it makes you happier, I believe the very same code could be written with POSIX functions instead. – Lundin Mar 25 '15 at 09:56
  • @Lundin Even if it's Windows specific, pointing OP at portable code is not irrelevant, because it's one of the purposes of C/C++ existent... Plus, yes, there is POSIX equivalent, pthread condition variables... It all depends on what hes doing exactly, so. Would suggest C++11 std::condition_variable instead –  Mar 25 '15 at 10:52
  • @user9000: To someone that doesn't understand the semantics of `volatile`, a portable solution might look simpler than it is. In reality, a portable solution generally adds complexity. The solution offered by Lundin is perfectly valid, and fairly simple to follow. – IInspectable Mar 25 '15 at 10:59
  • @IInspectable Never said it's invalid, I just wanted to state a point nothing more. –  Mar 25 '15 at 11:35
7

Instead of while(1), you can use while(continue_running), where continue_running is True when the thread loop should run. When you want to stop the thread, make the controlling thread set continue_running to False. Of course make sure that you properly guard continue_running with mutexes as it is a variable whose value can be modified from two threads.

simurg
  • 1,208
  • 7
  • 14
  • 2
    And remember to declare 'continue_running' as volatile, because otherwise optimisation might break your code. (In Visual Studio or gcc, in other IDEs it might differ.) – mg30rg Mar 25 '15 at 09:44
  • 1
    @mg30rg on most CPUs, bools are atomic operations –  Mar 25 '15 at 09:45
  • 3
    @user9000 - It is not about booleans but compiler optimization. If your compiler isn't avare of the multithreaded nature of your variable it might caches/replicates/multi-instatiates/skips/etc. it for performance reasons. – mg30rg Mar 25 '15 at 09:46
  • 2
    Thread-safety for such a variable might not be a concern anyhow, if there is only one thread setting it to a non-zero value, and another thread reading it. volatile to prevent incorrect optimizations is indeed good practice, but I think most compilers don't make such optimization assumption mistakes nowadays? – Lundin Mar 25 '15 at 09:48
  • 2
    @mg30rg True, it's always a good idea to take extra caution, you might also want to add that volatile will cause it to always read from memory as this is one of the reasons one would use volatile in a single/multi threaded application. –  Mar 25 '15 at 09:50
  • @Lundin In **most cases** (let's say 85%) it won't cause trouble. Int he remaining cases (let's say 15%) it causes a heisenbug. Does it really worth creating an opportunity of a heisenbug to save yourself some typing? – mg30rg Mar 25 '15 at 09:53
  • @Lundin - I have also met situations where not setting a variable volatile did not cause trouble for years and then on one day when a patch was added to a seemingly unconnected part of the code it started to act strange. – mg30rg Mar 25 '15 at 09:57
  • @mg30rg Ok was curious about the state of mainstream desktop OS compilers, I would have thought they'd solved this by now. I mainly work with embedded systems, where the compilers are generally less smart. In such systems you simply _must_ declare everything shared with a thread/callback/isr as volatile. – Lundin Mar 25 '15 at 10:17
  • @Lundin - As I hobby I develop PIC software, but PIC18 is not multi-threaded, so I didn't face this issue there. I also develop for my own Raspberry Pi, but I follow the same guidelines I do when I develop desktop sw. – mg30rg Mar 25 '15 at 10:29
  • 1
    @mg30rg You do have the same issue there, whenever you have an interrupt service routine. – Lundin Mar 25 '15 at 10:35
  • Why the deletion of my answer @martijn-pieters ? https://stackoverflow.com/users/100297/martijn-pieters – Pedro Vicente Sep 07 '20 at 00:34