gh-100228: Warn from os.fork() if other threads exist. #100229
Conversation
Not comprehensive, best effort.
|
| Name | Link |
|---|---|
| a91a4d1 | |
| https://app.netlify.com/sites/python-cpython-preview/deploys/639aae96df45c9000879b05a |
* Only warn in the parent process. * Add a test to test_os that a non-Python thread triggers the warning. * Avoid the system calls to count threads if we've already warned once. * The macOS API calls were written blind based on docs, will fix later if CI fails.
…o warn/os.fork/threads
I don't want the array of thread info but it may insist?
Modules/posixmodule.c
Outdated
| Py_XDECREF(threading_active); | ||
| return; | ||
| } | ||
| // Worst case if someone replaced threading._active or threading._limbo |
There was a problem hiding this comment.
If it's not fatal that this happens, these shouldn't be asserts. Otherwise code that works fine will start aborting for the wrong reason when run with a debug build (or, for example, when imported in a build system that enables asserts when running tests cough).
Misc/NEWS.d/next/Library/2022-12-13-17-29-09.gh-issue-100228.bgtzMV.rst
Outdated
Show resolved
Hide resolved
Modules/posixmodule.c
Outdated
| if (num_python_threads > 1) { | ||
| PyErr_WarnFormat( | ||
| PyExc_DeprecationWarning, 1, | ||
| "multi-threaded process, %s() may cause deadlocks.", name); |
There was a problem hiding this comment.
Should we point to more information to understand this issue somewhere, like a FAQ entry? I know we don't usually do this but this is somewhat of an esoteric issue, might be deep inside a library (e.g. something using multiprocessing), and we have been trying to improve error messages to be more helpful...
There was a problem hiding this comment.
I don't have a good canonical link to propose right now. The other thing that often happens is that the error message winds up with a Google search result of a stackoverflow or similar live content user moderated question linking to the current best sources of info.
For multiprocessing, I intend to do the warning about start_method fork as the default being used. In that specific default-fork situation where multiprocessing will provide its own DeprecationWarning, it probably makes sense to have multiprocessing silence this one on its call.
Let the thread free and clear things as it ends after we unblock it.
It could hide more than we intend and wouldn't do what I wanted in child processes that inherit the parents warnings state anyways.
…gtzMV.rst Co-authored-by: T. Wouters <thomas@python.org>
Co-authored-by: T. Wouters <thomas@python.org>
Not comprehensive, best effort warning. There are cases when threads exist that this code currently does not detect.
Starting with a
DeprecationWarningfor now, it is less disruptive than something likeRuntimeWarningand most likely to only be seen in people's CI tests - a good place to start with this messaging.TODO: