gh-135427: Fix DeprecationWarning for os.fork when run in threads with -Werror #136796
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
I have created a NEWS entry for this fix, but actually this change has indeed a little impact on Python users, so maybe |
…nto fix-issue-135427
|
Many tests are failing with a warning - as now the Many of the failing tests do not fail where one would expect to have more than one thread running. For example, in When printing there the number of threads using Therefore, we should find a way to suppress the warning of running a fork within a thread in all the tests that fail. Petr suggests to consider using the If anyone knows where this extra thread comes from, it will be appreciated if this is explained :-) |
That is ironically exactly why we have this warning - threads in a process can come from everywhere including external libraries outside of our control. I do think you're on the right track in terms of how to enable this to be something using |
…ead. Started to fix tests.
rani-pinchuk
requested review from
a team,
1st1,
asvetlov,
kumaraditya303,
rhettinger and
willingc
as code owners
| message=".*fork.*may lead to deadlocks in the child.*", | ||
| category=DeprecationWarning) | ||
| with warnings_helper.ignore_fork_in_thread_deprecation_warnings(): | ||
| super().setUpClass() |
There was a problem hiding this comment.
Could you do this in the other instances as well? (4 in this file & one in test_concurrent_futures/util.py?)
|
@rani-pinchuk, I think this is close to the finish line. Would you like me to fix up the remaining issue? |
|
Hi Petr, I am on holiday, so I can fix it only in this weekend. But if you
want to push it faster of course you can.
…On Tue, Aug 19, 2025, 10:57 AM Petr Viktorin ***@***.***> wrote:
*encukou* left a comment (python/cpython#136796)
<#136796 (comment)>
@rani-pinchuk <https://github.com/rani-pinchuk>, I think this is close to
the finish line. Would you like me to fix up the remaining issue?
—
Reply to this email directly, view it on GitHub
<#136796 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AH6O62VAUIB26FXWRUIBS5D3OLRI3AVCNFSM6AAAAACB4QZFAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCOJZHA3DCMRUGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
encukou
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit baa8446 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136796%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
encukou
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit b08021d 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136796%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
|
Shouldn't this be backported? |
|
Hi folks, I ran into an interesting failure with a new When it ran with a "fork" process pool it hung until it hit a 10m timeout and was killed. The output didn't include the deprecation warning or anything related to it and the hang seems to be caused by the Is this expected? Removing the decorator from tests elsewhere results in an immediate failure with the warning being clearly indicated as the source of the error, so it looks to me like the executor tests specifically are problematic. |
|
Thanks @rani-pinchuk for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Sorry, @rani-pinchuk and @encukou, I could not cleanly backport this to |
…ds with -Werror (pythonGH-136796) Don't ignore errors raised by `PyErr_WarnFormat` in `warn_about_fork_with_threads` Instead, ignore the warnings in all test code that forks. (That's a lot of functions.) In `test_support`, make `ignore_warnings` a context manager (as well as decorator), and add a `message` argument to it. Also add a `ignore_fork_in_thread_deprecation_warnings` helper for the deadlock-in-fork warning.
|
GH-140191 is a backport of this pull request to the 3.14 branch. |
|
I've managed to reproduce the hang mentioned above outside of unit tests: from concurrent.futures import ProcessPoolExecutor
import multiprocessing.forkserver
import threading
def noop():
pass
def do_fork():
ctx = multiprocessing.get_context('fork')
with ProcessPoolExecutor(mp_context=ctx) as exec:
f = exec.submit(noop)
f.result()
def run_with_thread(func):
barrier = threading.Barrier(2)
t = threading.Thread(target=barrier.wait)
t.start()
try:
func()
finally:
barrier.wait()
t.join()
def main():
multiprocessing.forkserver.ensure_running()
run_with_thread(do_fork)
if __name__ == '__main__':
main()Prior to this change this runs without producing any output. After the change, when run normally, it prints a warning and exits. However, if run with The trouble is that I'm not sure if this should be considered a regression, or even a problem at all, but it will be a potential change in behaviour for previously "working" code. If folks here think it would be helpful I'll open a separate issue for it. |
7 participants
This is a small fix as suggested in #135427 - using
PyErr_WriteUnraisable()instead ofPyErr_Clear().The fix includes a test which checks that the
DeprecationWarningis provided when usingforkorforkptywithin a thread, also when running with-Werror.@encukou