From 5fd8d4076eac06bbc6b17fc0cb0a40df9be137df Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 27 Jun 2018 12:39:14 +0200 Subject: [PATCH] bpo-33966, multiprocessing: Fix another handle leak When using a pool of processes on Windows, if the worker is terminated quickly, handles created by DupHandle() on reduction.dump() can remain open in the parent process causing a handles leak. Use a different strategy in the case: keep the handle open in the parent process for the lifetime of the worker, and the parent becomes responsible to close the handle when the worker completes. --- Lib/multiprocessing/popen_spawn_win32.py | 7 +++++-- Lib/multiprocessing/reduction.py | 19 ++++++++++++++++++- .../2018-06-27-12-42-18.bpo-33966.icIiCT.rst | 9 +++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-06-27-12-42-18.bpo-33966.icIiCT.rst diff --git a/Lib/multiprocessing/popen_spawn_win32.py b/Lib/multiprocessing/popen_spawn_win32.py index 3b92c8a2b4ae7f..8939c4680861bd 100644 --- a/Lib/multiprocessing/popen_spawn_win32.py +++ b/Lib/multiprocessing/popen_spawn_win32.py @@ -19,7 +19,7 @@ WINSERVICE = sys.executable.lower().endswith("pythonservice.exe") -def _close_handles(*handles): +def _close_handles(handles): for handle in handles: _winapi.CloseHandle(handle) @@ -66,8 +66,11 @@ def __init__(self, process_obj): self.returncode = None self._handle = hp self.sentinel = int(hp) + # List of handles closed by the finalizer. DupHandle adds handles + # to this list on reduction.dump(). + self._open_handles = [self.sentinel, int(rhandle)] self.finalizer = util.Finalize(self, _close_handles, - (self.sentinel, int(rhandle))) + (self._open_handles,)) # send information to child set_spawning_popen(self) diff --git a/Lib/multiprocessing/reduction.py b/Lib/multiprocessing/reduction.py index 473fd59df61b69..dda48a57147d83 100644 --- a/Lib/multiprocessing/reduction.py +++ b/Lib/multiprocessing/reduction.py @@ -104,6 +104,17 @@ def recv_handle(conn): class DupHandle(object): '''Picklable wrapper for a handle.''' def __init__(self, handle, access, pid=None): + spawning = context.get_spawning_popen() + # bpo-33966: Keep the handle open in the parent process. The parent + # is responsible to close the handle when the child process + # completes. + # + # Fallback on calling DuplicateHandle() with DUPLICATE_CLOSE_SOURCE + # in the child process if popen_spawn_win32.Popen is not used. The + # fallback leaks an open handle in the parent process if the child + # process is terminated before closing the handle. + self._close_source = not(spawning is not None + and hasattr(spawning, '_open_handles')) if pid is None: # We just duplicate the handle in the current process and # let the receiving process steal the handle. @@ -113,6 +124,8 @@ def __init__(self, handle, access, pid=None): self._handle = _winapi.DuplicateHandle( _winapi.GetCurrentProcess(), handle, proc, access, False, 0) + if not self._close_source: + spawning._open_handles.append(self._handle) finally: _winapi.CloseHandle(proc) self._access = access @@ -128,9 +141,13 @@ def detach(self): proc = _winapi.OpenProcess(_winapi.PROCESS_DUP_HANDLE, False, self._pid) try: + if self._close_source: + flags = _winapi.DUPLICATE_CLOSE_SOURCE + else: + flags = 0 return _winapi.DuplicateHandle( proc, self._handle, _winapi.GetCurrentProcess(), - self._access, False, _winapi.DUPLICATE_CLOSE_SOURCE) + self._access, False, flags) finally: _winapi.CloseHandle(proc) diff --git a/Misc/NEWS.d/next/Library/2018-06-27-12-42-18.bpo-33966.icIiCT.rst b/Misc/NEWS.d/next/Library/2018-06-27-12-42-18.bpo-33966.icIiCT.rst new file mode 100644 index 00000000000000..ad3ccd454c31fd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-06-27-12-42-18.bpo-33966.icIiCT.rst @@ -0,0 +1,9 @@ +multiprocessing: Fix a handle leak when a pool worker is terminated quickly. + +When using a pool of processes on Windows, if the worker is terminated +quickly, handles created by DupHandle() on reduction.dump() can remain open +in the parent process, causing a handles leak. + +Use a different strategy in the case: keep the handle open in the parent +process for the lifetime of the worker, and the parent becomes responsible +to close the handle when the worker completes.