W32: safer process termination

diff --git a/contrib/Makefile.am b/contrib/Makefile.am
index 0d653933086df30504636ca833f7017e9e44f30b..aaab287a8456152104d8a680c935f0ec5f9df275 100644 (file)
--- a/contrib/Makefile.am
+++ b/contrib/Makefile.am
@@ -13,6 +13,7 @@ timeout_watchdog_SOURCES = \
 endif
 
 noinst_SCRIPTS = \
+ terminate.py \
  gnunet_pyexpect.py \
  gnunet_janitor.py
 
@@ -28,6 +29,7 @@ EXTRA_DIST = \
  hostlist.cgi \
  hostlist.php \
  report.sh \
+ terminate.py.in \
  gnunet_pyexpect.py.in \
  gnunet_janitor.py.in \
  gnunet-gns-import.sh

diff --git a/contrib/gnunet_janitor.py.in b/contrib/gnunet_janitor.py.in
index 056ab9bc4ae161cc093ad7e49f954eb837cfa47c..69186fde6d1052f110f58daf8306ae50ccdd35ef 100644 (file)
--- a/contrib/gnunet_janitor.py.in
+++ b/contrib/gnunet_janitor.py.in
@@ -30,13 +30,11 @@ import sys
 import shutil
 import time
 import signal
+import terminate
 
 if os.name == 'nt':
   from win32com.client import GetObject
   WMI = GetObject('winmgmts:')
-  killsignal = signal.SIGTERM # any valid value will result in TerminateProcess()
-else:
-  killsignal = signal.SIGKILL  
 
 def get_process_list ():
   result = []
@@ -63,7 +61,7 @@ def main ():
     if re.match (r'gnunet-service-arm', p[1]):
       print ("killing arm process {0:5} {1}".format (p[0], p[1]))
       try:
-        os.kill (int (p[0]), killsignal)
+        terminate.safe_terminate_process_by_pid (int (p[0]), 1)
       except OSError as e:
         print ("failed: {0}".format (e))
         pass
@@ -71,7 +69,7 @@ def main ():
     if not re.match (r'gnunet-service-arm', p[1]):
       print ("killing non-arm process {0:5} {1}".format (p[0], p[1]))
       try:
-        os.kill (int (p[0]), killsignal)
+        terminate.safe_terminate_process_by_pid (int (p[0]), 1)
       except OSError as e:
         print ("failed: {0}".format (e))
         pass

diff --git a/src/include/winproc.h b/src/include/winproc.h
index 3670a74b001260f2a3b12c5fedce4f17b0935737..6cbe56248d4cc8373b1cfc45e939bdacfa304139 100644 (file)
--- a/src/include/winproc.h
+++ b/src/include/winproc.h
@@ -227,6 +227,7 @@ extern "C"
   int GNInitWinEnv ();
   void GNShutdownWinEnv ();
 
+  BOOL SafeTerminateProcess (HANDLE hProcess, UINT uExitCode, DWORD dwTimeout);
 #ifdef __cplusplus
 }
 #endif

diff --git a/src/util/os_priority.c b/src/util/os_priority.c
index 173d8814a7ec3fba5d763dfb1047c0723107e359..b0668e20d67d5f50258422d68c0d2e33660704ef 100644 (file)
--- a/src/util/os_priority.c
+++ b/src/util/os_priority.c
@@ -461,10 +461,16 @@ GNUNET_OS_process_kill (struct GNUNET_OS_Process *proc, int sig)
       if (0 != GetExitCodeProcess (proc->handle, &exitcode))
         must_kill = (exitcode == STILL_ACTIVE) ? GNUNET_YES : GNUNET_NO;
       if (GNUNET_YES == must_kill)
-        if (0 == TerminateProcess (proc->handle, 0))
+        if (0 == SafeTerminateProcess (proc->handle, 0, 0))
         {
-          SetErrnoFromWinError (GetLastError ());
-          return -1;
+          DWORD error_code = GetLastError ();
+          if (error_code != WAIT_TIMEOUT) /* OK, since timeout is 0 */
+          {
+            LOG (GNUNET_ERROR_TYPE_WARNING,
+                "SafeTermiateProcess failed with code %lu\n", error_code);
+            SetErrnoFromWinError (error_code);
+            return -1;
+          }
         }
     }
     return 0;
@@ -1386,7 +1392,7 @@ start_process (int pipe_control,
     /* If we can't pass on the socket(s), the child will block forever,
      * better put it out of its misery.
      */
-    TerminateProcess (gnunet_proc->handle, 0);
+    SafeTerminateProcess (gnunet_proc->handle, 0, 0);
     CloseHandle (gnunet_proc->handle);
     if (NULL != gnunet_proc->control_pipe)
       GNUNET_DISK_file_close (gnunet_proc->control_pipe);

diff --git a/src/util/win.cc b/src/util/win.cc
index 1f660720e55912f62981f113b4f82f321e38765b..7b67c9e83a25e4cd2fae09572af9f85e2e2517bb 100644 (file)
--- a/src/util/win.cc
+++ b/src/util/win.cc
@@ -1261,6 +1261,75 @@ char *winErrorStr(const char *prefix, int dwErr)
   return ret;\r
 }\r
 \r
+/**
+ * Terminate a process by creating a remote thread within it,
+ * which proceeds to call ExitProcess() inside that process.
+ * Safer than TerminateProcess ().
+ *
+ * Code is from From http://private-storm.de/2009/08/11/case-terminateprocess/
+ *
+ * @param hProcess handle of a process to terminate
+ * @param uExitCode exit code to use for ExitProcess()
+ * @param dwTimeout number of ms to wait for the process to terminate
+ * @return TRUE on success, FALSE on failure (check last error for the code)
+ */
+BOOL\r
+SafeTerminateProcess (HANDLE hProcess, UINT uExitCode, DWORD dwTimeout)\r
+{\r
+  DWORD dwTID, dwCode, dwErr = 0;\r
+  HANDLE hProcessDup = INVALID_HANDLE_VALUE;\r
+  HANDLE hRT = NULL;\r
+  HINSTANCE hKernel = GetModuleHandle ("Kernel32");\r
+  BOOL bSuccess = FALSE;\r
+\r
+  BOOL bDup = DuplicateHandle (GetCurrentProcess (), hProcess,\r
+      GetCurrentProcess (), &hProcessDup, PROCESS_ALL_ACCESS,\r
+      FALSE, 0);\r
+\r
+  /* Detect the special case where the process is\r
+   * already dead...\r
+   */\r
+  if (GetExitCodeProcess (bDup ? hProcessDup : hProcess, &dwCode) &&\r
+      (STILL_ACTIVE ==  dwCode))\r
+  {\r
+    FARPROC pfnExitProc;\r
+\r
+    pfnExitProc = GetProcAddress (hKernel, "ExitProcess");\r
+\r
+    hRT = CreateRemoteThread ((bDup) ? hProcessDup : hProcess, NULL, 0,\r
+        (LPTHREAD_START_ROUTINE) pfnExitProc, (PVOID) uExitCode, 0, &dwTID);\r
+\r
+    dwErr = GetLastError ();\r
+  }\r
+  else\r
+  {\r
+    dwErr = ERROR_PROCESS_ABORTED;\r
+  }\r
+\r
+  if (hRT)\r
+  {\r
+    /* Must wait process to terminate to\r
+     * guarantee that it has exited...\r
+     */\r
+    DWORD dwWaitResult = WaitForSingleObject ((bDup) ? hProcessDup : hProcess,\r
+        dwTimeout);\r
+    if (dwWaitResult == WAIT_TIMEOUT)\r
+      dwErr = WAIT_TIMEOUT;\r
+    else\r
+      dwErr = GetLastError ();\r
+\r
+    CloseHandle (hRT);\r
+    bSuccess = dwErr == NO_ERROR;\r
+  }\r
+\r
+  if (bDup)\r
+    CloseHandle (hProcessDup);\r
+\r
+  SetLastError (dwErr);\r
+\r
+  return bSuccess;\r
+}
+\r
 } /* extern "C" */\r
 \r
 #endif\r