Merge pull request #15 from dscho/dscho/ctrl-c-fixup

Ctrl+C revisited

Author: dscho

winsup/cygwin/common.din

@@ -33,7 +33,6 @@ sys_errlist = _sys_errlist DATA
 sys_nerr = _sys_nerr DATA
 sys_sigabbrev DATA
 sys_siglist DATA
-kill_process_tree DATA
 
 # Exported functions
 _Exit SIGFE

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,170 @@
+#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.
+ *
+ * This way of terminating the processes is not gentle: the processes get
+ * no chance of cleaning up after themselves (closing file handles, removing
+ * .lock files, terminating spawned processes (if any), etc).
+ */
+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;
+}
+
+/**
+ * Determine whether a process runs in the same architecture as the current
+ * one. That test is required before we assume that GetProcAddress() returns
+ * a valid address *for the target process*.
+ */
+static inline bool
+process_architecture_matches_current(HANDLE process)
+{
+  static BOOL current_is_wow = -1;
+  BOOL is_wow;
+
+  if (current_is_wow == -1 &&
+      !IsWow64Process (GetCurrentProcess (), &current_is_wow))
+        current_is_wow = -2;
+  if (current_is_wow == -2)
+    return false; /* could not determine current process' WoW-ness */
+  if (!IsWow64Process (process, &is_wow))
+    return false; /* cannot determine */
+  return is_wow == current_is_wow;
+}
+
+/**
+ * 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.
+ *
+ * The idea comes from 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
+ *
+ * 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)
+    {
+      /*
+       * We cannot determine the address of ExitProcess() for a process
+       * that does not match the current architecture (e.g. for a 32-bit
+       * process when we're running in 64-bit mode).
+       */
+      if (process_architecture_matches_current (process))
+        {
+          static LPTHREAD_START_ROUTINE exit_process_address;
+          if (!exit_process_address)
+            {
+              HINSTANCE kernel32 = GetModuleHandle ("kernel32");
+              exit_process_address = (LPTHREAD_START_ROUTINE)
+                GetProcAddress (kernel32, "ExitProcess");
+            }
+          DWORD thread_id;
+          HANDLE thread = !exit_process_address ? NULL :
+            CreateRemoteThread (process, NULL, 0, exit_process_address,
+                                (PVOID)exit_code, 0, &thread_id);
+
+          if (thread)
+            {
+              CloseHandle (thread);
+              /*
+               * Wait 10 seconds (arbitrary constant) for the process to
+               * finish; After that grace period, fall back to terminating
+               * non-gently.
+               */
+              if (WaitForSingleObject (process, 10000) == WAIT_OBJECT_0)
+                return 0;
+            }
+        }
+
+      return terminate_process_tree (process, exit_code);
+    }
+
+  return -1;
+}
+
+#endif

winsup/cygwin/include/cygwin/signal.h

@@ -410,12 +410,11 @@ int siginterrupt (int, int);
 #ifdef __INSIDE_CYGWIN__
 extern const char *sys_sigabbrev[];
 extern const char *sys_siglist[];
-extern void kill_process_tree(pid_t pid, int sig);
 #else
 extern const char __declspec(dllimport) *sys_sigabbrev[];
 extern const char __declspec(dllimport) *sys_siglist[];
-extern void __declspec(dllimport) kill_process_tree(pid_t pid, int sig);
 #endif
+void kill_process_tree(pid_t pid, int sig);
 
 #ifdef __cplusplus
 }

winsup/cygwin/include/cygwin/version.h

@@ -474,13 +474,12 @@ details. */
   307: Export timingsafe_bcmp, timingsafe_memcmp.
   308: Export dladdr.
   309: Export getloadavg.
-  310: Export kill_process_tree.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 310
+#define CYGWIN_VERSION_API_MINOR 309
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared

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)
 {

winsup/utils/kill.cc

@@ -17,6 +17,7 @@ details. */
 #include <cygwin/version.h>
 #include <getopt.h>
 #include <limits.h>
+#include <cygwin/exit_process.h>
 
 static char *prog_name;
 
@@ -173,7 +174,21 @@ forcekill (int pid, int sig, int wait)
   if (!wait || WaitForSingleObject (h, 200) != WAIT_OBJECT_0)
     {
       if (sig == SIGINT || sig == SIGTERM)
-        kill_process_tree (dwpid, sig);
+        {
+	  HANDLE cur = GetCurrentProcess (), h2;
+	  /* duplicate handle with access rights required for exit_process() */
+	  if (DuplicateHandle (cur, h, cur, &h2, PROCESS_CREATE_THREAD |
+			       PROCESS_QUERY_INFORMATION |
+			       PROCESS_VM_OPERATION |
+			       PROCESS_VM_WRITE | PROCESS_VM_READ |
+			       PROCESS_TERMINATE, FALSE, 0))
+	    {
+	      exit_process (h2, 128 + sig);
+	      CloseHandle (h2);
+	    }
+	  else
+	    terminate_process_tree(h, 128 + sig);
+	}
       else if (sig && !TerminateProcess (h, sig << 8)
           && WaitForSingleObject (h, 200) != WAIT_OBJECT_0)
         fprintf (stderr, "%s: couldn't kill pid %u, %u\n",