Repository navigation
gh-144513: Skip critical section locking during stop-the-world - #144524
colesbury merged 3 commits into
Conversation
Sorry, something went wrong.
When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world.
|
A few notes:
|
Sorry, something went wrong.
| } | ||
| // If the world is stopped, we don't need to acquire the lock because | ||
| // there are no other threads that could be accessing the object. | ||
| if (tstate->interp->stoptheworld.world_stopped) { |
There was a problem hiding this comment.
Is this safe to read non-atomically (in theory as well as in practice), or is it just that we don't have a way of reading a bool atomically? I see comments about HEAD_LOCK protecting these values but in practice world_stopped is written to without the lock, and read all over the place, without apparent synchronization.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, it's safe (as long as you have a valid PyThreadState). It's only written to when once all other threads are stopped, so no extra synchronization is needed.
Sorry, something went wrong.
There was a problem hiding this comment.
I should probably make this more clear in the _stoptheworld_state comment.
Sorry, something went wrong.
| // All threads: acquire critical section and hold it long enough to | ||
| // trigger TIME_TO_BE_FAIR_NS, which causes direct handoff on unlock. | ||
| Py_BEGIN_CRITICAL_SECTION(test_data->obj); | ||
| pysleep(10); |
There was a problem hiding this comment.
Add a comment mentioning TIME_TO_BE_FAIR_NS is currently 1ms, and this is 10x that, so the relationship is clear in the future?
Sorry, something went wrong.
…ythongh-144524) When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world.
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
|
Sorry, @colesbury, I could not cleanly backport this to 3.14 due to a conflict. cherry_picker 0fdf6a9a71f9b3c9adc0406542902e028431e7ca 3.14 |
Sorry, something went wrong.
…ythongh-144524) When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world.
…-world (pythongh-144524) When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world. (cherry picked from commit 0fdf6a9)
|
GH-145570 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
…ythongh-144524) When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world.
When the interpreter is in a stop-the-world pause, critical sections don't need to acquire locks since no other threads can be running. This avoids a potential deadlock where lock fairness hands off ownership to a thread that has already suspended for stop-the-world.