summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTerry Jan Reedy <tjreedy@udel.edu>2018-06-27 12:45:01 -0400
committerGitHub <noreply@github.com>2018-06-27 12:45:01 -0400
commitf331080ecb52ca9ce522c41622afd057ced567b3 (patch)
tree15069d49d7c5be1e52ec37a2afea91764efafc51
parent28bd988bd7ffcee2204f0fcbfdb42f90734f8431 (diff)
downloadcpython-git-revert-7960-backport-2cc9d21-3.7.tar.gz
Revert "bpo-33929: multiprocessing: fix handle leak on race condition (GH-7921)"revert-7960-backport-2cc9d21-3.7
This reverts commit 8b1ebcd7cb3319273ea635df78ebf9ad40171514.
-rw-r--r--Lib/multiprocessing/popen_spawn_win32.py15
-rw-r--r--Lib/multiprocessing/reduction.py10
-rw-r--r--Lib/multiprocessing/spawn.py10
-rw-r--r--Misc/NEWS.d/next/Library/2018-06-26-02-09-18.bpo-33929.OcCLah.rst5
4 files changed, 6 insertions, 34 deletions
diff --git a/Lib/multiprocessing/popen_spawn_win32.py b/Lib/multiprocessing/popen_spawn_win32.py
index 3b92c8a2b4..3e42e9c338 100644
--- a/Lib/multiprocessing/popen_spawn_win32.py
+++ b/Lib/multiprocessing/popen_spawn_win32.py
@@ -18,12 +18,6 @@ TERMINATE = 0x10000
WINEXE = (sys.platform == 'win32' and getattr(sys, 'frozen', False))
WINSERVICE = sys.executable.lower().endswith("pythonservice.exe")
-
-def _close_handles(*handles):
- for handle in handles:
- _winapi.CloseHandle(handle)
-
-
#
# We define a Popen class similar to the one from subprocess, but
# whose constructor takes a process object as its argument.
@@ -38,12 +32,8 @@ class Popen(object):
def __init__(self, process_obj):
prep_data = spawn.get_preparation_data(process_obj._name)
- # read end of pipe will be duplicated by the child process
+ # read end of pipe will be "stolen" by the child process
# -- see spawn_main() in spawn.py.
- #
- # bpo-33929: Previously, the read end of pipe was "stolen" by the child
- # process, but it leaked a handle if the child process had been
- # terminated before it could steal the handle from the parent process.
rhandle, whandle = _winapi.CreatePipe(None, 0)
wfd = msvcrt.open_osfhandle(whandle, 0)
cmd = spawn.get_command_line(parent_pid=os.getpid(),
@@ -66,8 +56,7 @@ class Popen(object):
self.returncode = None
self._handle = hp
self.sentinel = int(hp)
- self.finalizer = util.Finalize(self, _close_handles,
- (self.sentinel, int(rhandle)))
+ self.finalizer = util.Finalize(self, _winapi.CloseHandle, (self.sentinel,))
# send information to child
set_spawning_popen(self)
diff --git a/Lib/multiprocessing/reduction.py b/Lib/multiprocessing/reduction.py
index 473fd59df6..deca19ccad 100644
--- a/Lib/multiprocessing/reduction.py
+++ b/Lib/multiprocessing/reduction.py
@@ -68,16 +68,12 @@ if sys.platform == 'win32':
__all__ += ['DupHandle', 'duplicate', 'steal_handle']
import _winapi
- def duplicate(handle, target_process=None, inheritable=False,
- *, source_process=None):
+ def duplicate(handle, target_process=None, inheritable=False):
'''Duplicate a handle. (target_process is a handle not a pid!)'''
- current_process = _winapi.GetCurrentProcess()
- if source_process is None:
- source_process = current_process
if target_process is None:
- target_process = current_process
+ target_process = _winapi.GetCurrentProcess()
return _winapi.DuplicateHandle(
- source_process, handle, target_process,
+ _winapi.GetCurrentProcess(), handle, target_process,
0, inheritable, _winapi.DUPLICATE_SAME_ACCESS)
def steal_handle(source_pid, handle):
diff --git a/Lib/multiprocessing/spawn.py b/Lib/multiprocessing/spawn.py
index 2de4cb7f63..1f4f3f496f 100644
--- a/Lib/multiprocessing/spawn.py
+++ b/Lib/multiprocessing/spawn.py
@@ -96,15 +96,7 @@ def spawn_main(pipe_handle, parent_pid=None, tracker_fd=None):
assert is_forking(sys.argv), "Not forking"
if sys.platform == 'win32':
import msvcrt
- import _winapi
-
- if parent_pid is not None:
- source_process = _winapi.OpenProcess(
- _winapi.PROCESS_DUP_HANDLE, False, parent_pid)
- else:
- source_process = None
- new_handle = reduction.duplicate(pipe_handle,
- source_process=source_process)
+ new_handle = reduction.steal_handle(parent_pid, pipe_handle)
fd = msvcrt.open_osfhandle(new_handle, os.O_RDONLY)
else:
from . import semaphore_tracker
diff --git a/Misc/NEWS.d/next/Library/2018-06-26-02-09-18.bpo-33929.OcCLah.rst b/Misc/NEWS.d/next/Library/2018-06-26-02-09-18.bpo-33929.OcCLah.rst
deleted file mode 100644
index 6ddb17c81c..0000000000
--- a/Misc/NEWS.d/next/Library/2018-06-26-02-09-18.bpo-33929.OcCLah.rst
+++ /dev/null
@@ -1,5 +0,0 @@
-multiprocessing: Fix a race condition in Popen of
-multiprocessing.popen_spawn_win32. The child process now duplicates the read
-end of pipe instead of "stealing" it. Previously, the read end of pipe was
-"stolen" by the child process, but it leaked a handle if the child process had
-been terminated before it could steal the handle from the parent process.