Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colesbury merged 3 commits into from
Feb 6, 2026
+103 −1
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db08ab1
gh-144513: Skip critical section locking during stop-the-world
colesbury Feb 5, 2026
ba18b68
Fix MSVC build
colesbury Feb 5, 2026
5e62d5b
Update comments
colesbury Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Update comments
  • Loading branch information
@colesbury
colesbury committed Feb 6, 2026
commit 5e62d5b92e11edd213b55cf7f2d592a0ec90aa27
4 changes: 2 additions & 2 deletions Modules/_testinternalcapi/test_critical_sections.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,9 @@ thread_stw(void *arg)
}

// All threads: acquire critical section and hold it long enough to
// trigger TIME_TO_BE_FAIR_NS, which causes direct handoff on unlock.
// trigger TIME_TO_BE_FAIR_NS (1 ms), which causes direct handoff on unlock.
Py_BEGIN_CRITICAL_SECTION(test_data->obj);
pysleep(10);
pysleep(10); // 10 ms = 10 x TIME_TO_BE_FAIR_NS
Py_END_CRITICAL_SECTION();

PyGILState_Release(gil);
Expand Down
2 changes: 2 additions & 0 deletions Python/critical_section.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ _PyCriticalSection_BeginSlow(PyThreadState *tstate, PyCriticalSection *c, PyMute
}
// 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.
// Without this check, acquiring a critical section while the world is
// stopped could lead to a deadlock.
if (tstate->interp->stoptheworld.world_stopped) {
Copy link
Copy Markdown
Member

@Yhg1s Yhg1s Feb 6, 2026

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

@colesbury colesbury Feb 6, 2026

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

@colesbury colesbury Feb 6, 2026

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.

c->_cs_mutex = NULL;
c->_cs_prev = 0;
Expand Down
Loading