gh-117378: Fix multiprocessing forkserver preload sys.path inheritance. #126538
Conversation
This file contains 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
…ritance. `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python ~3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. A workaround for the bug this fixes was to set PYTHONPATH in the environment before the forkserver process was started.
gpshead
added
type-bug
gpshead
commented
Nov 7, 2024
Lib/test/test_multiprocessing_forkserver/test_forkserver_main.py
Outdated
Show resolved
Hide resolved
gpshead
added
🔨 test-with-buildbots
bedevere-bot
removed
🔨 test-with-refleak-buildbots
serhiy-storchaka
approved these changes
Nov 9, 2024
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
reading the code and old changes... It looks like there is no possible way for the I'll file a separate issue to track resolving that (remove it or recreate some missing functionality with a test). |
|
Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Nov 9, 2024
…ritance. (pythonGH-126538) pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance. `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python 3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. **This will only be observable during forkserver preload imports**. The code invoked before calling things in another process already correctly sets `sys.path`. Which is likely why this went unnoticed for so long as a mere performance issue in some configurations. A workaround for the bug on impacted Pythons is to set PYTHONPATH in the environment before multiprocessing's forkserver process was started. Not perfect as that is then inherited by other children, etc, but likely good enough for many people's purposes. (cherry picked from commit 9d08423) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
Sorry, @gpshead, I could not cleanly backport this to |
|
GH-126632 is a backport of this pull request to the 3.13 branch. |
gpshead
added a commit
that referenced
this pull request
Nov 9, 2024
…eritance. (GH-126538) (GH-126632) gh-117378: Fix multiprocessing forkserver preload sys.path inheritance. (GH-126538) `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python 3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. **This will only be observable during forkserver preload imports**. The code invoked before calling things in another process already correctly sets `sys.path`. Which is likely why this went unnoticed for so long as a mere performance issue in some configurations. A workaround for the bug on impacted Pythons is to set PYTHONPATH in the environment before multiprocessing's forkserver process was started. Not perfect as that is then inherited by other children, etc, but likely good enough for many people's purposes. (cherry picked from commit 9d08423) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
gpshead
added a commit
to gpshead/cpython
that referenced
this pull request
Nov 9, 2024
…th inheritance. (pythonGH-126538) pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance. `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python 3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. **This will only be observable during forkserver preload imports**. The code invoked before calling things in another process already correctly sets `sys.path`. Which is likely why this went unnoticed for so long as a mere performance issue in some configurations. A workaround for the bug on impacted Pythons is to set PYTHONPATH in the environment before multiprocessing's forkserver process was started. Not perfect as that is then inherited by other children, etc, but likely good enough for many people's purposes. (cherry picked from commit 9d08423) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
|
GH-126633 is a backport of this pull request to the 3.12 branch. |
gpshead
added a commit
that referenced
this pull request
Nov 10, 2024
…eritance. (GH-126538) (GH-126633) gh-117378: Fix multiprocessing forkserver preload sys.path inheritance. `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python 3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. **This will only be observable during forkserver preload imports**. The code invoked before calling things in another process already correctly sets `sys.path`. Which is likely why this went unnoticed for so long as a mere performance issue in some configurations. A workaround for the bug on impacted Pythons is to set PYTHONPATH in the environment before multiprocessing's forkserver process was started. Not perfect as that is then inherited by other children, etc, but likely good enough for many people's purposes. (cherry picked from commit 9d08423) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
picnixz
pushed a commit
to picnixz/cpython
that referenced
this pull request
Dec 8, 2024
…ritance. (pythonGH-126538) pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance. `sys.path` was not properly being sent from the parent process when launching the multiprocessing forkserver process to preload imports. This bug has been there since the forkserver start method was introduced in Python 3.4. It was always _supposed_ to inherit `sys.path` the same way the spawn method does. Observable behavior change: A `''` value in `sys.path` will now be replaced in the forkserver's `sys.path` with an absolute pathname `os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` was imported in the parent process as it already was when using the spawn start method. **This will only be observable during forkserver preload imports**. The code invoked before calling things in another process already correctly sets `sys.path`. Which is likely why this went unnoticed for so long as a mere performance issue in some configurations. A workaround for the bug on impacted Pythons is to set PYTHONPATH in the environment before multiprocessing's forkserver process was started. Not perfect as that is then inherited by other children, etc, but likely good enough for many people's purposes. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
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.
Fixes the
multiprocessing"forkserver"start method forkserver process to correctly inherit the parent'ssys.pathduring the importing ofmultiprocessing.set_forkserver_preloadmodules in the same manner assys.pathis configured when executing work items in the worker processes.This bug could cause some forkserver module preloading to silently fail to be preloaded, leading to a performance degration in child processes due to additional repeated work. It could also have led to a side effect of
""still being insys.pathduring forkserver preload imports instead of the absolute path of the directory that workers see.A workaround for the bug that this fixes was to set
PYTHONPATHin the environment before the forkserver process was started.