8000 gh-118727: Don't drop the GIL in `drop_gil()` unless the current thread holds it by swtaarrs · Pull Request #118745 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-118727: Don't drop the GIL in drop_gil() unless the current thread holds it #118745

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
merged 10 commits into from
May 23, 2024
Merged

gh-118727: Don't drop the GIL in drop_gil() unless the current thread holds it #118745

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d21956
Don't drop the GIL during thread deletion unless the current thread h…
swtaarrs May 7, 2024
b91a1d5
Store holds_gil flag on PyThreadState instead, and use it where appro…
swtaarrs May 8, 2024
402ab90
Merge remote-tracking branch 'upstream/main' into cpython-drop-gil
swtaarrs May 8, 2024
29c853d
Account for tstate == NULL case
swtaarrs May 8, 2024
694926f
Always track holds_gil, related cleanup
swtaarrs May 8, 2024
480c07d
Fix ASAN error, pull drop_gil_impl() back into drop_gil()
swtaarrs May 9, 2024
d28bf13
Rename thread_dying to final_release
swtaarrs May 9, 2024
43f64d7
Merge remote-tracking branch 'upstream/main' into cpython-drop-gil
swtaarrs May 9, 2024
d056a46
Restore drop_gil_impl(), clear GIL drop request when disabling GIL
swtaarrs May 13, 2024
f32a503
Merge remote-tracking branch 'upstream/main' into cpython-drop-gil
swtaarrs May 13, 2024
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
Next Next commit
Always track holds_gil, related cleanup
  • Loading branch information
@swtaarrs
swtaarrs committed May 8, 2024
commit 694926fac8fce7067260a967b8c93769277c2dce
4 changes: 0 additions & 4 deletions Include/cpython/pystate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,8 @@ struct _ts {
unsigned int bound_gilstate:1;
/* Currently in use (maybe holds the GIL). */
unsigned int active:1;
< 8000 /td> #ifdef Py_GIL_DISABLED
/* Currently holds the GIL. */
unsigned int holds_gil:1;
#else
unsigned int _unused:1;
#endif

/* various stages of finalization */
unsigned int finalizing:1;
Expand Down
26 changes: 12 additions & 14 deletions Python/ceval_gil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,9 @@ drop_gil_impl(PyThreadState *tstate, struct _gil_runtime_state *gil)
MUTEX_LOCK(gil->mutex);
_Py_ANNOTATE_RWLOCK_RELEASED(&gil->locked, /*is_write=*/1);
_Py_atomic_store_int_relaxed(&gil->locked, 0);
#ifdef Py_GIL_DISABLED
if (tstate != NULL) {
tstate->_status.holds_gil = 0;
}
#endif
COND_SIGNAL(gil->cond);
MUTEX_UNLOCK(gil->mutex);
}
Expand Down Expand Up @@ -377,9 +375,7 @@ take_gil(PyThreadState *tstate)
MUTEX_LOCK(gil->switch_mutex);
#endif
/* We now hold the GIL */
#ifdef Py_GIL_DISABLED
tstate->_status.holds_gil = 1;
#endif
_Py_atomic_store_int_relaxed(&gil->locked, 1);
_Py_ANNOTATE_RWLOCK_ACQUIRED(&gil->locked, /*is_write=*/1);

Expand All @@ -402,6 +398,8 @@ take_gil(PyThreadState *tstate)
in take_gil() while the main thread called
wait_for_thread_shutdown() from Py_Finalize(). */
MUTEX_UNLOCK(gil->mutex);
/* tstate could be a dangling pointer, so don't pass it to
drop_gil(). */
drop_gil(interp, NULL, 1);
PyThread_exit_thread();
}
Expand Down Expand Up @@ -458,17 +456,17 @@ PyEval_ThreadsInitialized(void)
static inline int
current_thread_holds_gil(struct _gil_runtime_state *gil, PyThreadState *tstate)
{
if (((PyThreadState*)_Py_atomic_load_ptr_relaxed(&gil->last_holder)) != tstate) {
#ifdef Py_GIL_DISABLED
assert(!tstate->_status.holds_gil);
#endif
return 0;
}
int holds_gil = tstate->_status.holds_gil;

// holds_gil is the source of truth; check that last_holder and gil->locked
// are consistent with it.
int locked = _Py_atomic_load_int_relaxed(&gil->locked);
#ifdef Py_GIL_DISABLED
assert(!tstate->_status.holds_gil || locked);
#endif
return locked;
int is_last_holder =
((PyThreadState*)_Py_atomic_load_ptr_relaxed(&gil->last_holder)) == tstate;
assert(!holds_gil || locked);
assert(!holds_gil || is_last_holder);

return holds_gil;
}
#endif

Expand Down
3 changes: 1 addition & 2 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2070,12 +2070,11 @@ _PyThreadState_Attach(PyThreadState *tstate)
}

#ifdef Py_GIL_DISABLED
if (_PyEval_IsGILEnabled(tstate) != tstate->_status.holds_gil) {
if (_PyEval_IsGILEnabled(tstate) && !tstate->_status.holds_gil) {
// The GIL was enabled between our call to _PyEval_AcquireLock()
// and when we attached (the GIL can't go from enabled to disabled
// here because only a thread holding the GIL can disable
// it). Detach and try again.
assert(!tstate->_status.holds_gil);
tstate_set_detached(tstate, _Py_THREAD_DETACHED);
tstate_deactivate(tstate);
current_fast_clear(&_PyRuntime);
Expand Down
0