gh-154208: Fix Windows handle leak when a child process dies early#154365
Open
gianghungtien wants to merge 1 commit into
Open
gh-154208: Fix Windows handle leak when a child process dies early#154365gianghungtien wants to merge 1 commit into
gianghungtien wants to merge 1 commit into
Conversation
On Windows, ``multiprocessing.reduction.DupHandle`` duplicated a handle in the parent process and relied on the receiving process to steal it with ``DUPLICATE_CLOSE_SOURCE``. When the child was terminated before it could unpickle its arguments, that duplicate was never claimed and leaked in the parent for the rest of its lifetime. Creating and immediately terminating a ``multiprocessing.pool.Pool`` leaked four handles (both ends of the input and output queues) per pool. When a child process is being spawned, duplicate the handle straight into it instead, so the operating system releases the handle if the child dies early. This is the same approach already used for the pipe that sends the child its parameters (bpo-33929) and for semaphores. Handles sent to an already running process still use the steal mechanism.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Windows,
multiprocessing.reduction.DupHandleduplicates a handle in the parent process and relies on the receiving process to steal it later, viaDuplicateHandle(..., DUPLICATE_CLOSE_SOURCE)inDupHandle.detach():If the child process is terminated before it unpickles its arguments, it never steals those handles, and the parent's duplicates are leaked for the rest of its lifetime.
This is what the issue reports: creating a
Pooland immediately terminating it leaks 4 handles per pool — the two ends of_inqueueand the two ends of_outqueue, which are the fourPipeConnectionobjects pickled into the worker's arguments. (_change_notifieris not passed to workers, and the queue locks are unaffected becauseSemLock.__getstate__already usesduplicate_for_child().)The leak only occurs when the worker dies before unpickling. If the pool runs a task first, the child steals the handles and nothing leaks — which is why the issue's reproducer, which terminates immediately, leaks reliably.
Fix
When a handle is duplicated for a child process that is currently being spawned, duplicate it directly into the child instead. The child then uses the handle as-is, and if it dies early the operating system releases the handle along with the rest of its handle table.
This is not a new mechanism: it is exactly the fix made in bpo-33929 for the pipe used to send the child its parameters, as the comment in
popen_spawn_win32.pyrecords:multiprocessing.synchronize.SemLockalready passes its handle to children the same way, throughPopen.duplicate_for_child().The handle is duplicated with the requested
accessrather thanDUPLICATE_SAME_ACCESS, so a read-only pipe end stays read-only in the child, exactly as before. Handles sent to an already running process (send_handle(), or a connection sent through a queue) have no spawning popen and keep using the existing steal mechanism, so that path is unchanged.Verification
Reproducer from the issue, before:
After (the initial rise is one-time warm-up of the spawn machinery, then it is flat — it stays at 132 over 100 iterations too):
The added regression test spawns a child with a pipe end in its arguments, terminates it before it can unpickle them, and checks the process handle count does not grow. It fails on
main(AssertionError: 10 not less than or equal to 4) and passes with this change.test_multiprocessing_spawn(448 tests),test_concurrent_futuresandtest_multiprocessing_main_handlingall pass on Windows.Fixes #154208