squash! Kill also children of the process to be killed via Ctrl+C

Try to kill Win32 processes gently upon Ctrl+C

When a process is terminated via TerminateProcess(), it has no chance to
do anything in the way of cleaning up. This is particularly noticeable
when a lengthy Git for Windows process tries to update Git's index file
and leaves behind an index.lock file. Git's idea is to remove the stale
index.lock file in that case, using the signal and atexit handlers
available in Linux. But those signal handlers never run.

Note: this is not an 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()`.

Let's use a gentler method, described in the Dr Dobb's article "A Safer
Alternative to TerminateProcess()" by Andrew Tucker (July 1, 1999),
http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547

Essentially, we inject a new thread into the running process that does
nothing else than running the ExitProcess() function.

If that fails, or when the process still lives on after 10 seconds, we
fall back to calling TerminateProcess(), but in that case we take pains
to kill also processes directly or indirectly spawned by that process;
TerminateProcess() does not give any process a chance to terminate its
child processes. Originally, we wanted to call the function
`GenerateConsoleCtrlEvent()` for SIGTERM with console processes, but 1)
that may not work as it requires a process *group* ID (i.e. the process
ID of the initial process group member, which we may not have), and 2)
it does not kill the process *tree* on Windows 7 and earlier.

Note: 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.

While at it, we also fix the exit status: the convention is to add 128
to the signal number, to give exit code handlers a chance to detect an
"exit by signal" condition.

This fixes the MSYS2 side of 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 *MSYS2* 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 <[email protected]>

Author: dscho

winsup/cygwin/exceptions.cc

@@ -27,6 +27,7 @@ details. */
 #include "child_info.h"
 #include "ntdll.h"
 #include "exception.h"
+#include "cygwin/exit_process.h"
 
 /* Definitions for code simplification */
 #ifdef __x86_64__
@@ -1547,10 +1548,7 @@ sigpacket::process ()
   if (have_execed)
     {
       sigproc_printf ("terminating captive process");
-      if ((sigExeced = si.si_signo) == SIGINT)
-        kill_process_tree (GetProcessId (ch_spawn), sigExeced = si.si_signo);
-      else
-        TerminateProcess (ch_spawn, sigExeced = si.si_signo);
+      exit_process (ch_spawn, 128 + (sigExeced = si.si_signo));
     }
   /* Dispatch to the appropriate function. */
   sigproc_printf ("signal %d, signal handler %p", si.si_signo, handler);

winsup/cygwin/include/cygwin/exit_process.h

@@ -0,0 +1,121 @@
+// vim: ts=99 sw=2
+#ifndef EXIT_PROCESS_H
+#define EXIT_PROCESS_H
+
+/*
+ * This file contains functions to terminate a Win32 process, as gently as
+ * possible.
+ *
+ * At first, we will attempt to inject a thread that calls ExitProcess(). If
+ * that fails, we will fall back to terminating the entire process tree.
+ *
+ * As we do not want to export this function in the MSYS2 runtime, these
+ * functions are marked as file-local.
+ */
+
+#include <tlhelp32.h>
+
+/**
+ * Terminates the process corresponding to the process ID and all of its
+ * directly and indirectly spawned subprocesses.
+ */
+static int
+terminate_process_tree(HANDLE main_process, int exit_code)
+{
+  HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+  PROCESSENTRY32 entry;
+  DWORD pids[16384];
+  int max_len = sizeof(pids) / sizeof(*pids), i, len, ret = 0;
+  pid_t pid = GetProcessId(main_process);
+
+  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 = i == 0 ? main_process :
+        OpenProcess(PROCESS_TERMINATE, FALSE, pids[i]);
+
+      if (process)
+        {
+          if (!TerminateProcess(process, exit_code))
+            ret = -1;
+          CloseHandle(process);
+        }
+    }
+
+  return ret;
+}
+
+/**
+ * Inject a thread into the given process that runs ExitProcess().
+ *
+ * Note: as kernel32.dll is loaded before any process, the other process and
+ * this process will have ExitProcess() at the same address.
+ *
+ * This function expects the process handle to have the access rights for
+ * CreateRemoteThread(): PROCESS_CREATE_THREAD, PROCESS_QUERY_INFORMATION,
+ * PROCESS_VM_OPERATION, PROCESS_VM_WRITE, and PROCESS_VM_READ.
+ *
+ * If this method fails, we fall back to running terminate_process_tree().
+ */
+static int
+exit_process(HANDLE process, int exit_code)
+{
+  DWORD code;
+
+  if (GetExitCodeProcess (process, &code) && code == STILL_ACTIVE)
+    {
+      HINSTANCE kernel32 = GetModuleHandle("kernel32");
+      LPTHREAD_START_ROUTINE exit_process_address =
+        (LPTHREAD_START_ROUTINE) GetProcAddress (kernel32, "ExitProcess");
+      DWORD thread_id;
+      HANDLE thread = CreateRemoteThread (process, NULL, 0,
+                                          exit_process_address,
+                                          (PVOID)exit_code, 0, &thread_id);
+
+      if (!thread)
+        return terminate_process_tree(process, exit_code);
+      CloseHandle (thread);
+      if (WaitForSingleObject (process, 10000) == WAIT_OBJECT_0)
+        return 0;
+  }
+  return -1;
+}
+
+#endif

winsup/cygwin/signal.cc

@@ -10,7 +10,6 @@ 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"
@@ -359,62 +358,6 @@ 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)
 {