Skip to content

Commit

Permalink
Kill also children of the process to be killed via Ctrl+C
Browse files Browse the repository at this point in the history
When a Win32 process needs to be terminated, the child processes it
spawned off also need to be terminated. This is no issue for MSys2
processes because MSys2 emulates Unix' signal system accurately, both
for the process sending the kill signal and the process receiving it.
Win32 processes do not have such a signal handler, though, instead
MSys2 shuts them down via `TerminateProcess()`.

As `TerminateProcess()` leaves the Win32 process no chance to do
anything, and also does not care about child processes, we have to grow
a different solution. For console processes, it should be enough to call
`GenerateConsoleCtrlEvent()`, but sadly even then this seems to handle
child processes correctly only on Windows 8 but not Windows 7.

So let's identify the tree of processes spawned directly and indirectly
from the process to be killed, and kill them all. To do so, we do not
use the NtQueryInformationProcess() function because 1) it is marked
internal and subject to change at any time of Microsoft's choosing, and
2) it does not even officially report the child/parent relationship (the
pid of the parent process is stored in the `Reserved3` slot of the
`PROCESS_BASIC_INFORMATION` structure).

Instead, we resort to the Toolhelp32 API -- which happily also works on
64-bit Windows -- to enumerate the process tree and reconstruct the
process tree rooted in the process we intend to kill.

This fixes the bug where interrupting `git clone https://...` would send
the spawned-off `git remote-https` process into the background instead of
interrupting it, i.e. the clone would continue and its progress would be
reported mercilessly to the console window without the user being able
to do anything about it (short of firing up the task manager and killing
the appropriate task manually).

Note that this special-handling is only necessary when *MSys* handles
the Ctrl+C event, e.g. when interrupting a process started from within
mintty or any other non-cmd-based terminal emulator. If the process was
started from within `cmd.exe`'s terminal window, child processes are
already killed appropriately upon Ctrl+C.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho committed Mar 22, 2015
1 parent eb1d093 commit c4ba4e3
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 1 deletion.
5 changes: 4 additions & 1 deletion winsup/cygwin/exceptions.cc
Expand Up @@ -1479,7 +1479,10 @@ sigpacket::process ()
if (have_execed)
{
sigproc_printf ("terminating captive process");
TerminateProcess (ch_spawn, sigExeced = si.si_signo);
if ((sigExeced = si.si_signo) == SIGINT)
kill_process_tree (GetProcessId (ch_spawn), sigExeced = si.si_signo);
else
TerminateProcess (ch_spawn, sigExeced = si.si_signo);
}
/* Dispatch to the appropriate function. */
sigproc_printf ("signal %d, signal handler %p", si.si_signo, handler);
Expand Down
1 change: 1 addition & 0 deletions winsup/cygwin/include/cygwin/signal.h
Expand Up @@ -400,6 +400,7 @@ extern const char *sys_siglist[];
extern const char __declspec(dllimport) *sys_sigabbrev[];
extern const char __declspec(dllimport) *sys_siglist[];
#endif
void kill_process_tree(pid_t pid, int sig);

#ifdef __cplusplus
}
Expand Down
57 changes: 57 additions & 0 deletions winsup/cygwin/signal.cc
Expand Up @@ -13,6 +13,7 @@ Cygwin license. Please consult the file "CYGWIN_LICENSE" for
details. */

#include "winsup.h"
#include <tlhelp32.h>
#include <stdlib.h>
#include <sys/cygwin.h>
#include "pinfo.h"
Expand Down Expand Up @@ -361,6 +362,62 @@ killpg (pid_t pgrp, int sig)
return kill (-pgrp, sig);
}

/**
* Terminates the process corresponding to the process ID and all of its
* directly and indirectly spawned subprocesses.
*/
extern "C" void
kill_process_tree(pid_t pid, int sig)
{
HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
PROCESSENTRY32 entry;
DWORD pids[16384];
int max_len = sizeof(pids) / sizeof(*pids), i, len;

pids[0] = (DWORD) pid;
len = 1;

/*
* Even if Process32First()/Process32Next() seem to traverse the
* processes in topological order (i.e. parent processes before
* child processes), there is nothing in the Win32 API documentation
* suggesting that this is guaranteed.
*
* Therefore, run through them at least twice and stop when no more
* process IDs were added to the list.
*/
for (;;) {
int orig_len = len;

memset(&entry, 0, sizeof(entry));
entry.dwSize = sizeof(entry);

if (!Process32First(snapshot, &entry))
break;

do {
for (i = len - 1; i >= 0; i--) {
if (pids[i] == entry.th32ProcessID)
break;
if (pids[i] == entry.th32ParentProcessID)
pids[len++] = entry.th32ProcessID;
}
} while (len < max_len && Process32Next(snapshot, &entry));

if (orig_len == len || len >= max_len)
break;
}

for (i = len - 1; i >= 0; i--) {
HANDLE process = OpenProcess(PROCESS_TERMINATE, FALSE, pids[i]);

if (process) {
TerminateProcess(process, sig << 8);
CloseHandle(process);
}
}
}

extern "C" void
abort (void)
{
Expand Down

0 comments on commit c4ba4e3

Please sign in to comment.