gh-126688: Reinit import lock after fork #126692
Conversation
The PyMutex implementation supports unlocking after fork because we clear the list of watiers in parking_lot.c. This doesn't work as well for _PyRecursiveMutex because on some systems, such as SerenityOS, the thread id is not preserved across fork().
|
Thanks @oskar-skog. My first attempt had a bug and I also had to merge "main" back into the PR branch to fix some other test issues. Sorry if that complicated applying the diff to 3.13. You may already know this, but you can get the PR as a diff with a URL like https://github.com/python/cpython/pull/126692.patch. That's sometimes easier to apply than diffing between two commits involving merges. |
colesbury
requested review from
kumaraditya303,
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
|
No, it actually made it simpler to patch, and I didn't know you could just download a PR as a patch. |
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
ambv
added
needs backport to 3.13
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
The PyMutex implementation supports unlocking after fork because we clear the list of waiters in parking_lot.c. This doesn't work as well for _PyRecursiveMutex because on some systems, such as SerenityOS, the thread id is not preserved across fork(). (cherry picked from commit 5610860) Co-authored-by: Sam Gross <colesbury@gmail.com>
|
GH-126765 is a backport of this pull request to the 3.13 branch. |
The PyMutex implementation supports unlocking after fork because we clear the list of waiters in parking_lot.c. This doesn't work as well for _PyRecursiveMutex because on some systems, such as SerenityOS, the thread id is not preserved across fork(). (cherry picked from commit 5610860) Co-authored-by: Sam Gross <colesbury@gmail.com>
| // gh-126688: Thread id may change after fork() on some operating systems. | ||
| IMPORT_LOCK(interp).thread = PyThread_get_thread_ident_ex(); |
There was a problem hiding this comment.
Wouldn't this lead to deadlocks if some other thread happened to be holding the import lock when we forked? It seems like the child process needs to know the thread ID from which it forked.
There was a problem hiding this comment.
We acquire the import lock before forking so that no other thread holds it:
Lines 618 to 624 in 09c240f
There was a problem hiding this comment.
Hmm, looks like we always acquire the import lock in PyOS_BeforeFork(). It should be fine then.
The
PyMuteximplementation supports unlocking after fork because we clear the list of watiers in parking_lot.c. This doesn't work as well for_PyRecursiveMutexbecause on some systems, such as SerenityOS, the thread id is not preserved acrossfork().