Merge pull request #1025 from gevent/issue1023

More safely terminate process on Windows.

CHANGES.rst

@@ -65,6 +65,9 @@
   total speed up of about 35 times. It is now in the same ball park as
   the native :class:`threading.local` class. See :pr:`1024`.
 
+- More safely terminate process on Windows. Reported in :issue:`1023`
+  by Giacomo Debidda.
+
 1.2.2 (2017-06-05)
 ==================
 

src/gevent/subprocess.py

@@ -108,6 +108,7 @@ __extra__ = [
     'CreateProcess',
     'INFINITE',
     'TerminateProcess',
+    'STILL_ACTIVE',
 
     # These were added for 3.5, but we make them available everywhere.
     'run',
@@ -995,7 +996,21 @@ class Popen(object):
         def terminate(self):
             """Terminates the process
             """
+            # Don't terminate a process that we know has already died.
+            if self.returncode is not None:
+                return
+            try:
                 TerminateProcess(self._handle, 1)
+            except OSError as e:
+                # ERROR_ACCESS_DENIED (winerror 5) is received when the
+                # process already died.
+                if e.winerror != 5:
+                    raise
+                rc = GetExitCodeProcess(self._handle)
+                if rc == STILL_ACTIVE:
+                    raise
+                self.returncode = rc
+                self.result.set(self.returncode)
 
         kill = terminate
 

src/greentest/patched_tests_setup.py

@@ -171,14 +171,6 @@ disabled_tests = [
     'test_thread.TestForkInThread.test_forkinthread',
     # XXX needs investigating
 
-    'test_subprocess.POSIXProcessTestCase.test_terminate_dead',
-    'test_subprocess.POSIXProcessTestCase.test_send_signal_dead',
-    'test_subprocess.POSIXProcessTestCase.test_kill_dead',
-    # Don't exist in the test suite until 2.7.4+; with our monkey patch in place,
-    # they fail because the process they're looking for has been allowed to exit.
-    # Our monkey patch waits for the process with a watcher and so detects
-    # the exit before the normal polling mechanism would
-
     'test_subprocess.POSIXProcessTestCase.test_preexec_errpipe_does_not_double_close_pipes',
     # Does not exist in the test suite until 2.7.4+. Subclasses Popen, and overrides
     # _execute_child. But our version has a different parameter list than the
@@ -529,16 +521,6 @@ if sys.version_info[:2] >= (3, 4):
         # In any event, this test hangs forever
 
 
-        'test_subprocess.POSIXProcessTestCase.test_terminate_dead',
-        'test_subprocess.POSIXProcessTestCase.test_send_signal_dead',
-        'test_subprocess.POSIXProcessTestCase.test_kill_dead',
-        # With our monkey patch in place,
-        # they fail because the process they're looking for has been allowed to exit.
-        # Our monkey patch waits for the process with a watcher and so detects
-        # the exit before the normal polling mechanism would
-
-
-
         'test_subprocess.POSIXProcessTestCase.test_preexec_errpipe_does_not_double_close_pipes',
         # Subclasses Popen, and overrides _execute_child. Expects things to be done
         # in a particular order in an exception case, but we don't follow that