[WIP] bpo-33966, multiprocessing: Fix another handle leak

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)

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)
 

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.