Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-117378: Fix multiprocessing forkserver preload sys.path inheritance.#126538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-117378: Fix multiprocessing forkserver preload sys.path inheritance.#126538
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ritance.`sys.path` was not properly being sent from the parent process when launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python ~3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod.A workaround for the bug this fixes was to set PYTHONPATH in the environmentbefore the forkserver process was started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
bedevere-bot commentedNov 9, 2024
bedevere-bot commentedNov 9, 2024
serhiy-storchaka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM besides some minor suggestions.
Test for__main__ is not required if it is difficult to write it or it is too slow, but it would be better to add it.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
gpshead commentedNov 9, 2024
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). |
9d08423 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
…ritance. (pythonGH-126538)pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance.`sys.path` was not properly being sent from the parent process when launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.(cherry picked from commit9d08423)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 the3.13 branch. |
…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 launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.(cherry picked from commit9d08423)Co-authored-by: Gregory P. Smith <greg@krypto.org>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…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 launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.(cherry picked from commit9d08423)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 the3.12 branch. |
…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 launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.(cherry picked from commit9d08423)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…ritance. (pythonGH-126538)pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance.`sys.path` was not properly being sent from the parent process when launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…ritance. (pythonGH-126538)pythongh-117378: Fix multiprocessing forkserver preload sys.path inheritance.`sys.path` was not properly being sent from the parent process when launchingthe multiprocessing forkserver process to preload imports. This bug has beenthere since the forkserver start method was introduced in Python 3.4. It wasalways _supposed_ to inherit `sys.path` the same way the spawn method does.Observable behavior change: A `''` value in `sys.path` will now be replaced inthe forkserver's `sys.path` with an absolute pathname`os.path.abspath(os.getcwd())` saved at the time that `multiprocessing` wasimported in the parent process as it already was when using the spawn startmethod. **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 insome configurations.A workaround for the bug on impacted Pythons is to set PYTHONPATH in theenvironment before multiprocessing's forkserver process was started. Not perfectas that is then inherited by other children, etc, but likely good enough for manypeople's purposes.Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
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.