René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Multiprocessing PipeConnection handles leak on failed subprocess spawn","articleBody":"# Bug report\n\n### Bug description:\n\nOn Windows, if a `multiprocessing.Process` fails to spawn, and it is given `multiprocessing.Pipe(duplex=True)` arguments, then the underlying handles pipe handles leak. One way to cause a subprocess to fail to spawn is forgetting to guard spawning code with `if __name__ == \"__main__\":`. Here is an MRE along those lines:\n\n```python\nimport multiprocessing\nimport traceback\n\nEXHIBIT_LEAK = True\n\n\ndef child(*pipes):\n    print(\"child\", pipes)\n    for pipe in pipes:\n        pipe.close()\n\n\ndef parent(*pipes):\n    print(\"parent\", pipes)\n    p = multiprocessing.Process(target=child, args=pipes)\n    p.start()\n    for pipe in pipes:\n        pipe.close()\n    p.join()\n\n\nif EXHIBIT_LEAK or __name__ == \"__main__\":\n    child_send_pipe, recv_pipe = multiprocessing.Pipe(duplex=True)\n    send_pipe, child_recv_pipe = multiprocessing.Pipe(duplex=True)\n    parent(child_send_pipe, child_recv_pipe)\n    print(\"after\")\n    try:\n        send_pipe.send_bytes(b\"test\")\n    except:\n        traceback.print_exc()\n    else:\n        print(\"send failed to raise\")\n    try:\n        recv_pipe.poll()\n    except:\n        traceback.print_exc()\n    else:\n        print(\"recv failed to raise\")\n\n```\n\nI noticed this because it makes the death of the process impossible to detect by looking only at the pipes, leading to an [ugly workaround](https://github.com/richardsheridan/trio-parallel/blob/22b96bd63931db1d4d6f6982adf65a9a0679bab4/trio_parallel/_proc.py#L72-L93) that I only recently realized was actually leaking resources.\n\nThis is in principle the same bug as bpo-33929. To be honest I don't understand the fix there enough to know if it could be generalized to `PipeConnection` objects, but it would likely prevent leaks from all sorts of handle stealing edge cases. Otherwise, workaround for the specific case of recursive spawning could be achieved by signaling the unpickling error back to `parent` during the `Process.start()` method. The state of the system as the MRE failure occurs can be summarized as:\n\n- `parent` calls `Process.start` -\u003e `self._Popen(self)` -\u003e`CreateProcess`\n- `parent` dumps `prep_data` and then `process_obj` sequentially `to_child` with big buffer, so it's nonblocking\n- dump of `process_obj` induces duplication of `PipeConnection` handle via `DupeHandle`\n- `child` runs `mp.spawn.spawn_main` -\u003e `_main` \n- `child` drains data (fd `from_parent` of GH-7921) only to `reduction.pickle.load(from_parent)`\n- `child` unpickles `__main__` module\n- `child` notices issue in `Process.start` -\u003e `self._Popen(self)` -\u003e `spawn.get_preparation_data` -\u003e `_check_not_importing_main`\n- `child` raises and never unpickles the arguments and so never steals the handle via `DupeHandle.detach()`\n- `parent` calls `poll` on read end of `PipeConnection`\n- `parent` never gets expected `BrokenPipeError`\n\nIf `Process.start` can raise and close [`from_parent`](https://github.com/python/cpython/blob/c87233fd3fa77067013c35328f8c4884f0567a59/Lib/multiprocessing/spawn.py#L127) (actually I'm not sure if `from_parent` or `fd` or both must be closed) in `child` before [`dump(process_obj, to_child)`](https://github.com/python/cpython/blob/c87233fd3fa77067013c35328f8c4884f0567a59/Lib/multiprocessing/popen_spawn_win32.py#L95) in `parent`, all stealing leaks would prevented. However, that would require some sort of IO wait between [line 94 and 95](https://github.com/python/cpython/blob/c87233fd3fa77067013c35328f8c4884f0567a59/Lib/multiprocessing/popen_spawn_win32.py#L94-L95). My first thought was to make another private pipe pair only for the purpose of signaling `parent` if `prep_data` were successfully unpickled or not. `parent` would then either read a sentinel out of the pipe or raise some kind of exception. Also, wouldn't it be better in principle for `start` to raise an exception and clean up if it knows it failed to start?\n\n\n\n### CPython versions tested on:\n\n3.8, 3.9, 3.10, 3.11, 3.12\n\n### Operating systems tested on:\n\nWindows","author":{"url":"https://github.com/richardsheridan","@type":"Person","name":"richardsheridan"},"datePublished":"2024-01-29T15:37:46.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/114720/cpython/issues/114720"}