Skip to content

gh-144513: Skip critical section locking during stop-the-world - #144524

Merged
colesbury merged 3 commits into
python:mainfrom
colesbury:gh-144513-critical-section-stw
Feb 6, 2026
Merged

gh-144513: Skip critical section locking during stop-the-world#144524
colesbury merged 3 commits into
python:mainfrom
colesbury:gh-144513-critical-section-stw

Conversation

@colesbury

@colesburycolesbury commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

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.
@colesbury

Copy link
Copy Markdown
ContributorAuthor

A few notes:

  • Mostly written by Claude code
  • Test passes on PR and would deadlock on main without the associated fix
  • Part of the motivation is making frame locals (framelocalsproxy) access thread-safe. Some of the time we will want to use a STW pause, and allowing the existing critical sections within that pause will make the modifications simpler

@colesbury
colesbury marked this pull request as ready for review February 5, 2026 18:45
@colesbury
colesbury requested review from Yhg1s and mpageFebruary 5, 2026 18:46
}
// 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) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably make this more clear in the _stoptheworld_state comment.

// 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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment threadPython/critical_section.c
@colesbury
colesbury enabled auto-merge (squash) February 6, 2026 14:53
@colesbury
colesbury merged commit 0fdf6a9 into python:mainFeb 6, 2026
65 checks passed
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
…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.
@colesbury
colesbury deleted the gh-144513-critical-section-stw branch February 20, 2026 14:51
@colesburycolesbury added the needs backport to 3.14 bugs and security fixes label Mar 5, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @colesbury, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 0fdf6a9a71f9b3c9adc0406542902e028431e7ca 3.14

colesbury added a commit to colesbury/cpython that referenced this pull request Mar 5, 2026
…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.
colesbury added a commit to colesbury/cpython that referenced this pull request Mar 5, 2026
…-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)
@bedevere-app

Copy link
Copy Markdown

GH-145570 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Mar 5, 2026
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
…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.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@colesbury@Yhg1s@kumaraditya303