Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
gh-141860: Add on_error= keyword arg tomultiprocessing.set_forkserver_preload#141859
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-141860: Add on_error= keyword arg tomultiprocessing.set_forkserver_preload#141859
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
2a184cd to39bc49eCompare39bc49e to656274aComparegpshead commentedNov 22, 2025
Used to rewrite my sandbox'ed Claude's commit as me for CLA purposes. |
dd6c71b to4994d3fCompareAdd a keyword-only `raise_exceptions` parameter (default False) to`multiprocessing.set_forkserver_preload()`. When True, ImportErrorexceptions during module preloading cause the forkserver to exit,breaking all use of the forkserver multiprocessing context. Thisallows developers to catch import errors during development ratherthan having them silently ignored.Implementation adds the parameter to both the ForkServer class methodand the BaseContext wrapper, passing it through to the forkservermain() function which conditionally raises ImportError instead ofignoring it.Tests are in new test_multiprocessing_forkserver/test_preload.py withproper resource cleanup using try/finally.Documentation describes the behavior, consequences (forkserver exit,EOFError/ConnectionError on subsequent use), and recommends use duringdevelopment.Based on original work by Nick Neumann inpythonGH-99515.Contributed by Nick Neumann.Co-authored-by: aggieNick02 <nick@pcpartpicker.com>Co-authored-by: Claude (Sonnet 4.5) <noreply@anthropic.com>Co-authored-by: Gregory P. Smith <greg@krypto.org>
Add skip decorators to exclude test_preload.py on Android, iOS,WASI, and other platforms that don't support fork, using theexisting has_fork_support check from test.support.Co-authored-by: Claude (Sonnet 4.5) <noreply@anthropic.com>Co-authored-by: Gregory P. Smith <greg@krypto.org>
6e975f3 to73f7489CompareAdd has_fork_support check at the package level in __init__.py toskip the entire test_multiprocessing_forkserver package on Android,iOS, WASI, and other platforms that don't support fork. This preventsimport errors before individual test modules are loaded.Remove the now-redundant skip decorators from test_preload.py sincethe package-level skip makes them unnecessary.Co-authored-by: Claude (Sonnet 4.5) <noreply@anthropic.com>Co-authored-by: Gregory P. Smith <greg@krypto.org>
73f7489 to5ce91baCompareChanged from a boolean raise_exceptions parameter to a more flexibleon_error parameter that accepts 'ignore', 'warn', or 'fail'.- 'ignore' (default): silently ignores import failures- 'warn': emits ImportWarning from forkserver subprocess- 'fail': raises exception, causing forkserver to exitAlso improved error messages by adding .add_note() to connectionfailures when on_error='fail' to guide users to check stderr.Updated both spawn.import_main_path() and module __import__()failure paths to support all three modes using match/case syntax.Co-authored-by: aggieNick02 <nick@pcpartpicker.com>Co-authored-by: Claude (Sonnet 4.5) <noreply@anthropic.com>Co-authored-by: Gregory P. Smith <greg@krypto.org>
multiprocessing.set_forkserver_preload- Remove unused warnings import- Use getattr() to safely check for __notes__ attribute- Add assertion for notes existence before checking content
Use warnings.catch_warnings() context manager to ensure ImportWarningis always emitted and never converted to an error, even when the testenvironment has warnings configured with -W error.
For consistency with module preload warnings, use 'Failed to preload__main__' instead of 'Failed to import __main__'.
Extract preload logic into a separate _handle_preload() function toenable targeted unit testing. Add comprehensive unit tests for bothmodule and __main__ preload with all three on_error modes.New tests:- test_handle_preload_main_on_error_{fail,warn,ignore}- test_handle_preload_module_on_error_{fail,warn,ignore}- test_handle_preload_main_valid- test_handle_preload_combinedTotal test count increased from 6 to 14 tests.Use delete=True (default) for NamedTemporaryFile instead of manuallymanaging cleanup with try/finally blocks. Keep file open during testand let context manager handle automatic deletion. Also remove now-unused os import.
- Remove comments that restate what the code obviously does- Change from 'from multiprocessing.forkserver import _handle_preload' to 'from multiprocessing import forkserver' and use qualified calls- Makes code cleaner and follows better import conventions
Remove catch_warnings() workaround from production code and insteadfix the root cause: tests were allowing sys.warnoptions to leak intothe forkserver subprocess via _args_from_interpreter_flags().When CI runs with -W error, the forkserver subprocess was inheritingthis flag and converting our ImportWarning calls into exceptions,causing it to crash.Solution: Save and clear sys.warnoptions in test setUp, restore intearDown. This gives the forkserver subprocess a clean warning statewhere warnings.warn() works as intended.Also remove unnecessary set_forkserver_preload([]) call from tearDown.
Explain why we catch broad Exception for import_main_path() (it usesrunpy.run_path() which can raise any exception) vs only ImportErrorfor regular __import__() calls.
Change from single quotes to double quotes when describing stringvalues like "ignore", "warn", and "fail" in docstrings anddocumentation for consistency.
Previous approach of clearing sys.warnoptions broke when CI used -bbflag, because _args_from_interpreter_flags() expects certain warningoptions to exist based on sys.flags settings.Instead of clearing completely, filter out only the specific warningoptions that would convert ImportWarning to errors:- 'error' (converts all warnings)- 'error::ImportWarning' (converts ImportWarning specifically)This preserves options like 'error::BytesWarning' that subprocess's_args_from_interpreter_flags() needs to remove, preventing ValueError.
Remove unnecessary comment and simplify the exception note to justsay 'Check stderr.' instead of the more verbose message.
Clarify that _send_value is a static method specifically to bepicklable as a Process target function.
Uh oh!
There was an error while loading.Please reload this page.
duaneg 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.
I think the approach is good, and doesn't need any changes aside from the test fixes already discussed.
I tried playing around with using an error handler callback instead of/in addition to a string to specify the strategy, but it gets quite messy. There are a couple of places where we want to check what type of error handling the user specified, and that is awkward when it is an arbitrary function.
There don't appear to be any tests for running from package entry points (i.e. with-m <main module>), but they might be a bit awkward to add and given how they work their code paths are possibly already effectively covered. Some quick manual testing seems to show everything functioning correctly.
My other refactoring suggestions are marginal and unimportant, feel free to ignore them.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Integrate sys_argv parameter from main branch (pythongh-135335) withour on_error parameter for set_forkserver_preload.Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
TestHandlePreload tests call spawn.import_main_path() which modifiessys.modules['__main__'] and appends to spawn.old_main_modules. Thisstate persisted after tests, causing subsequent forkserver tests totry loading __main__ from a deleted temp file. With on_error='ignore',the forkserver stayed broken causing process spawn failures and hangs.Fix by adding setUp/tearDown to TestHandlePreload that saves andrestores sys.modules['__main__'] and clears spawn.old_main_modules.Also add suppress_forkserver_stderr() context manager that injectsa stderr-suppressing module via the preload mechanism itself, avoidingnoisy output during tests that expect import failures.Thanks to Duane Griffin for identifying the root cause of the hang.
Replace suppress_forkserver_stderr() with capture_forkserver_stderr()that writes stderr to a temp file instead of /dev/null. This allowstests to assert on the actual warning/error messages.The capture module also enables ImportWarning via filterwarnings()since it's ignored by default in Python. Line buffering ensuresoutput is flushed, and forkserver.main() calls _flush_std_streams()after preloading which guarantees content is written before we read.Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract duplicated match on_error logic into a helper function.The warn_stacklevel parameter is required (no default) to ensurecallers explicitly specify the correct level for warning messages.Thanks to Duane Griffin for the suggestion.
bedevere-bot commentedJan 14, 2026
🤖 New build scheduled with the buildbot fleet by@gpshead for commit6ffe214 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141859%2Fmerge If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
c879b2a intopython:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Add a keyword-only
on_errorparameter tomultiprocessing.set_forkserver_preload(). This allows the user to have exceptions during optionalforkserverstart method module preloading cause the forkserver subprocess to warn (generally to stderr) or exit with an error (preventing use of the forkserver) instead of being silently ignored.Thisalso fixes an oversight, errors when preloading a
__main__module are now treated the similarly. Those would always raise unlike other modules in preload, but that had gone unnoticed as up until bug fix PRGH-135295 in 3.14.1 and 3.13.8, the__main__module was never actually preloaded.Based on original work by Nick Neumann@aggieNick02 inGH-99515.
📚 Documentation preview 📚:https://cpython-previews--141859.org.readthedocs.build/
Fixes#141860