Skip to content

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

Open
colesbury wants to merge 2 commits intopython:mainfrom
colesbury:gh-144513-critical-section-stw
Open

gh-144513: Skip critical section locking during stop-the-world#144524
colesbury wants to merge 2 commits intopython:mainfrom
colesbury:gh-144513-critical-section-stw

Conversation

@colesbury
Copy link
Contributor

@colesbury colesbury commented Feb 5, 2026

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
Contributor Author

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 mpage February 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
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
Contributor Author

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
Contributor Author

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
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?

Sign up for free to 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.

2 participants