Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Conversation

@gpshead
Copy link
Member

@gpsheadgpshead commentedNov 7, 2024
edited by bedevere-appbot
Loading

Fixes themultiprocessing"forkserver" start method forkserver process to correctly inherit the parent'ssys.path during the importing ofmultiprocessing.set_forkserver_preload modules in the same manner assys.path is 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.path during forkserver preload imports instead of the absolute path of the directory that workers see.

A workaround for the bug that this fixes was to setPYTHONPATH in the environment before the forkserver process was started.

…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.
@gpsheadgpshead added type-bugAn unexpected behavior, bug, or error stdlibStandard Library Python modules in the Lib/ directory topic-multiprocessing needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes labelsNov 7, 2024
@gpsheadgpshead self-assigned thisNov 7, 2024
@gpsheadgpshead added 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section labelsNov 9, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@gpshead for commitcf07467 🤖

If you want to schedule another build, you need to add the🔨 test-with-refleak-buildbots label again.

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@gpshead for commitcf07467 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed 🔨 test-with-refleak-buildbotsTest PR w/ refleak buildbots; report in status section 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelsNov 9, 2024
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@gpshead
Copy link
MemberAuthor

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.

reading the code and old changes... It looks like there is no possible way formain_path= to be set on the multiprocessing'sforkserver.main() call since 2013's9a76735 for#64145.

theforkserver.main call is constructed inforkserver.py getting its two possible allowed named args fromspawn.get_preparation_data() which was changed to set"init_main_from_name" instead of"main_path" in the above change. Effectively making the main_path= code dead in forkserver.

I'll file a separate issue to track resolving that (remove it or recreate some missing functionality with a test).

@gpsheadgpshead merged commit9d08423 intopython:mainNov 9, 2024
36 checks passed
@miss-islington-app
Copy link

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 requestNov 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 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>
@miss-islington-app
Copy link

Sorry,@gpshead, I could not cleanly backport this to3.12 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 9d08423b6e0fa89ce9cfea08e580ed72e5db8c70 3.12

@bedevere-app
Copy link

GH-126632 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelNov 9, 2024
gpshead added a commit that referenced this pull requestNov 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 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>
@gpsheadgpshead deleted the bug/multiprocessing-forkserver-preload-sys-path branchNovember 9, 2024 23:48
gpshead added a commit to gpshead/cpython that referenced this pull requestNov 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 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>
@bedevere-app
Copy link

GH-126633 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelNov 9, 2024
gpshead added a commit that referenced this pull requestNov 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 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>
picnixz pushed a commit to picnixz/cpython that referenced this pull requestDec 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 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>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
…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>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

Assignees

@gpsheadgpshead

Labels

stdlibStandard Library Python modules in the Lib/ directorytopic-multiprocessingtype-bugAn unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gpshead@bedevere-bot@serhiy-storchaka

[8]ページ先頭

©2009-2025 Movatter.jp