Commit 9f4b3702 authored by Jason Madden's avatar Jason Madden

More safely terminate process on Windows.

Fixes #1023

Also re-enable the POSIX version of those tests. They should (and do)
pass.
parent 1b6e4054
......@@ -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)
==================
......
......@@ -108,6 +108,8 @@ __extra__ = [
'CreateProcess',
'INFINITE',
'TerminateProcess',
'GetExitCodeProcess',
'STILL_ACTIVE',
# These were added for 3.5, but we make them available everywhere.
'run',
......@@ -995,7 +997,20 @@ class Popen(object):
def terminate(self):
"""Terminates the process
"""
TerminateProcess(self._handle, 1)
# 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
kill = terminate
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment