gh-104341: Call _PyEval_ReleaseLock() with NULL When Finalizing the Current Thread #105109
Conversation
| @@ -625,10 +632,12 @@ _PyEval_AcquireLock(PyThreadState *tstate) | |||
| } | |||
|
|
|||
| void | |||
| _PyEval_ReleaseLock(PyThreadState *tstate) | |||
| _PyEval_ReleaseLock(PyInterpreterState *interp, PyThreadState *tstate) | |||
There was a problem hiding this comment.
The other key part of this change is here, where we pass in the interpreter separately from the thread state, which allows tstate to be NULL.
Python/ceval_gil.c
Outdated
| @@ -278,6 +278,10 @@ static void recreate_gil(struct _gil_runtime_state *gil) | |||
| static void | |||
| drop_gil(struct _ceval_state *ceval, PyThreadState *tstate) | |||
| { | |||
| /* We shouldn't be using a thread state that isn't viable any more. */ | |||
There was a problem hiding this comment.
This comment is cryptic here. It doesn't say why it's here nor in which situation the "non-viable thread state" occurs.
There was a problem hiding this comment.
I've clarified the comment.
I think we're good to go: no objections from me. I never assume I can have full confidence about modifications to this code, but CI and buildbots and ultimately beta2 testing should reveal more if anything else lurks... |
|
Thanks you! |
|
Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
Sorry @ericsnowcurrently, I had trouble checking out the |
ericsnowcurrently
added
needs backport to 3.12
|
Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
… the Current Thread (pythongh-105109) This avoids the problematic race in drop_gil() by skipping the FORCE_SWITCHING code there for finalizing threads. (The idea for this approach came out of discussions with @markshannon.) (cherry picked from commit 3698fda) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
|
GH-105209 is a backport of this pull request to the 3.12 branch. |
…g the Current Thread (gh-105109) (gh-105209) This avoids the problematic race in drop_gil() by skipping the FORCE_SWITCHING code there for finalizing threads. (The idea for this approach came out of discussions with @markshannon.) (cherry picked from commit 3698fda) Co-authored-by: Eric Snow ericsnowcurrently@gmail.com
This avoids the problematic race in
drop_gil()by skipping theFORCE_SWITCHINGcode there for finalizing threads.This is a much simpler approach to solving the race than in other PRs I've posted. I'd still like to pursue some of those other ideas but that can be done separately for 3.13+.
(The idea for this approach came out of discussions with @markshannon.)