Visitar URL original
gh-118727: Don't drop the GIL in `drop_gil()` unless the current thread holds it by swtaarrs · Pull Request #118745 · python/cpython · GitHub
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
colesbury 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
Rename thread_dying to final_release
  • Loading branch information
@swtaarrs
swtaarrs committed May 9, 2024
commit d28bf133656f5f729ef6330c9dcd0cd3b81f112c
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ extern void _PyEval_FiniGIL(PyInterpreterState *interp);
extern void _PyEval_AcquireLock(PyThreadState *tstate);

extern void _PyEval_ReleaseLock(PyInterpreterState *, PyThreadState *,
int thread_dying);
int final_release);

#ifdef Py_GIL_DISABLED
// Returns 0 or 1 if the GIL for the given thread's interpreter is disabled or
Expand Down
18 changes: 9 additions & 9 deletions Python/ceval_gil.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,19 +206,19 @@ static void recreate_gil(struct _gil_runtime_state *gil)
#endif

static void
drop_gil(PyInterpreterState *interp, PyThreadState *tstate, int thread_dying)
drop_gil(PyInterpreterState *interp, PyThreadState *tstate, int final_release)
{
struct _ceval_state *ceval = &interp->ceval;
/* If thread_dying is true, the caller is indicating that we're releasing
/* If final_release is true, the caller is indicating that we're releasing
the GIL for the last time in this thread. This is particularly
relevant when the current thread state is finalizing or its
interpreter is finalizing (either may be in an inconsistent
state). In that case the current thread will definitely
never try to acquire the GIL again. */
// XXX It may be more correct to check tstate->_status.finalizing.
// XXX assert(thread_dying || !tstate->_status.cleared);
// XXX assert(final_release || !tstate->_status.cleared);

assert(thread_dying || tstate != NULL);
assert(final_release || tstate != NULL);
struct _gil_runtime_state *gil = ceval->gil;
#ifdef Py_GIL_DISABLED
// Check if we have the GIL before dropping it. tstate will be NULL if
Expand All @@ -232,7 +232,7 @@ drop_gil(PyInterpreterState *interp, PyThreadState *tstate, int thread_dying)
Py_FatalError("drop_gil: GIL is not locked");
}

if (!thread_dying) {
if (!final_release) {
/* Sub-interpreter support: threads might have been switched
under our feet using PyThreadState_Swap(). Fix the GIL last
holder variable so that our heuristics work. */
Expand All @@ -252,10 +252,10 @@ drop_gil(PyInterpreterState *interp, PyThreadState *tstate, int thread_dying)
/* We might be releasing the GIL for the last time in this thread. In that
case there's a possible race with tstate->interp getting deleted after
gil->mutex is unlocked and before the following code runs, leading to a
crash. We can use thread_dying to indicate the thread is done with the
crash. We can use final_release to indicate the thread is done with the
GIL, and that's the only time we might delete the interpreter. See
https://github.com/python/cpython/issues/104341. */
if (!thread_dying &&
if (!final_release &&
_Py_eval_breaker_bit_is_set(tstate, _PY_GIL_DROP_REQUEST_BIT)) {
MUTEX_LOCK(gil->switch_mutex);
/* Not switched yet => wait */
Expand Down Expand Up @@ -582,11 +582,11 @@ _PyEval_AcquireLock(PyThreadState *tstate)
void
_PyEval_ReleaseLock(PyInterpreterState *interp,
PyThreadState *tstate,
int thread_dying)
int final_release)
{
assert(tstate != NULL);
assert(tstate->interp == interp);
drop_gil(interp, tstate, thread_dying);
drop_gil(interp, tstate, final_release);
}

void
Expand Down