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
Next Next commit
Don't drop the GIL during thread deletion unless the current thread h… 
…olds it
  • Loading branch information
@swtaarrs
swtaarrs committed May 7, 2024
commit 1d219569859572dcae0776cff039215dae63353f
5 changes: 1 addition & 4 deletions Lib/test/test_importlib/test_threaded_import.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from test.support import verbose
from test.support.import_helper import forget, mock_register_at_fork
from test.support.os_helper import (TESTFN, unlink, rmtree)
from test.support import script_helper, threading_helper, requires_gil_enabled
from test.support import script_helper, threading_helper

threading_helper.requires_working_threading(module=True)

Expand Down Expand Up @@ -248,9 +248,6 @@ def test_concurrent_futures_circular_import(self):
'partial', 'cfimport.py')
script_helper.assert_python_ok(fn)

# gh-118727 and gh-118729: pool_in_threads.py may crash in free-threaded
# builds, which can hang the Tsan test so temporarily skip it for now.
@requires_gil_enabled("gh-118727: test may crash in free-threaded builds")
def test_multiprocessing_pool_circular_import(self):
# Regression test for bpo-41567
fn = os.path.join(os.path.dirname(__file__),
Expand Down
13 changes: 12 additions & 1 deletion Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1831,10 +1831,21 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate)
_Py_EnsureTstateNotNULL(tstate);
#ifdef Py_GIL_DISABLED
_Py_qsbr_detach(((_PyThreadStateImpl *)tstate)->qsbr);
// tstate_delete_common() removes tstate from consideration for
// stop-the-worlds. This means that another thread could enable the GIL
// before our call to _PyEval_ReleaseLock(), violating its invariant that
// the calling thread holds the GIL if and only if the GIL is enabled. Deal
// with this by deciding if we need to release the GIL before
// tstate_delete_common(), when the invariant is still true.
int holds_gil = _PyEval_IsGILEnabled(tstate);
Comment thread
swtaarrs marked this conversation as resolved.
Outdated
#else
int holds_gil = 1;
#endif
current_fast_clear(tstate->interp->runtime);
tstate_delete_common(tstate);
_PyEval_ReleaseLock(tstate->interp, NULL);
if (holds_gil) {
_PyEval_ReleaseLock(tstate->interp, NULL);
}
free_threadstate((_PyThreadStateImpl *)tstate);
}

Expand Down