Skip to content

bpo-39984: _PyThreadState_DeleteCurrent() takes tstate #19051

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.

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 1 commit into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ extern PyObject *_PyEval_EvalCode(
extern int _PyEval_ThreadsInitialized(_PyRuntimeState *runtime);
extern PyStatus _PyEval_InitThreads(PyThreadState *tstate);

extern void _PyEval_ReleaseLock(PyThreadState *tstate);


/* --- _Py_EnterRecursiveCall() ----------------------------------------- */

Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ PyAPI_FUNC(void) _PyErr_Print(PyThreadState *tstate);
PyAPI_FUNC(void) _PyErr_Display(PyObject *file, PyObject *exception,
PyObject *value, PyObject *tb);

PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(struct pyruntimestate *runtime);
PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(PyThreadState *tstate);

#ifdef __cplusplus
}
Expand Down
3 changes: 2 additions & 1 deletion Lib/test/test_capi.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ def test_no_FatalError_infinite_loop(self):
# This used to cause an infinite loop.
self.assertTrue(err.rstrip().startswith(
b'Fatal Python error: '
b'PyThreadState_Get: no current thread'))
b'PyThreadState_Get: '
b'current thread state is NULL (released GIL?)'))

def test_memoryview_from_NULL_pointer(self):
self.assertRaises(ValueError, _testcapi.make_memoryview_from_NULL_pointer)
Expand Down
2 changes: 1 addition & 1 deletion Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ t_bootstrap(void *boot_raw)
PyMem_DEL(boot_raw);
tstate->interp->num_threads--;
PyThreadState_Clear(tstate);
_PyThreadState_DeleteCurrent(runtime);
_PyThreadState_DeleteCurrent(tstate);
PyThread_exit_thread();
}

Expand Down
10 changes: 9 additions & 1 deletion Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ static void
ensure_tstate_not_null(const char *func, PyThreadState *tstate)
{
if (tstate == NULL) {
_Py_FatalErrorFunc(func, "current thread state is NULL");
_Py_FatalErrorFunc(func,
"current thread state is NULL (released GIL?)");
}
}

Expand Down Expand Up @@ -313,6 +314,13 @@ PyEval_ReleaseLock(void)
drop_gil(&runtime->ceval, tstate);
}

void
_PyEval_ReleaseLock(PyThreadState *tstate)
{
struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval;
drop_gil(ceval, tstate);
}

void
PyEval_AcquireThread(PyThreadState *tstate)
{
Expand Down
60 changes: 32 additions & 28 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ extern "C" {
_Py_atomic_store_relaxed(&(gilstate)->tstate_current, \
(uintptr_t)(value))

static void
ensure_tstate_not_null(const char *func, PyThreadState *tstate)
{
if (tstate == NULL) {
_Py_FatalErrorFunc(func,
"current thread state is NULL (released GIL?)");
}
}


/* Forward declarations */
static PyThreadState *_PyGILState_GetThisThreadState(struct _gilstate_runtime_state *gilstate);
static void _PyThreadState_Delete(PyThreadState *tstate, int check_current);
Expand Down Expand Up @@ -400,9 +410,7 @@ PyInterpreterState *
PyInterpreterState_Get(void)
{
PyThreadState *tstate = _PyThreadState_GET();
if (tstate == NULL) {
Py_FatalError("no current thread state");
}
ensure_tstate_not_null(__func__, tstate);
PyInterpreterState *interp = tstate->interp;
if (interp == NULL) {
Py_FatalError("no current interpreter");
Expand Down Expand Up @@ -819,9 +827,7 @@ tstate_delete_common(PyThreadState *tstate,
struct _gilstate_runtime_state *gilstate)
{
_PyRuntimeState *runtime = tstate->interp->runtime;
if (tstate == NULL) {
Py_FatalError("NULL tstate");
}
ensure_tstate_not_null(__func__, tstate);
PyInterpreterState *interp = tstate->interp;
if (interp == NULL) {
Py_FatalError("NULL interp");
Expand All @@ -835,8 +841,6 @@ tstate_delete_common(PyThreadState *tstate,
tstate->next->prev = tstate->prev;
HEAD_UNLOCK(runtime);

PyMem_RawFree(tstate);

if (gilstate->autoInterpreterState &&
PyThread_tss_get(&gilstate->autoTSSkey) == tstate)
{
Expand All @@ -855,6 +859,7 @@ _PyThreadState_Delete(PyThreadState *tstate, int check_current)
}
}
tstate_delete_common(tstate, gilstate);
PyMem_RawFree(tstate);
}


Expand All @@ -866,22 +871,22 @@ PyThreadState_Delete(PyThreadState *tstate)


void
_PyThreadState_DeleteCurrent(_PyRuntimeState *runtime)
_PyThreadState_DeleteCurrent(PyThreadState *tstate)
{
struct _gilstate_runtime_state *gilstate = &runtime->gilstate;
PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate);
if (tstate == NULL) {
Py_FatalError("no current tstate");
}
ensure_tstate_not_null(__func__, tstate);
struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
tstate_delete_common(tstate, gilstate);
_PyRuntimeGILState_SetThreadState(gilstate, NULL);
PyEval_ReleaseLock();
_PyEval_ReleaseLock(tstate);
PyMem_RawFree(tstate);
}

void
PyThreadState_DeleteCurrent(void)
{
_PyThreadState_DeleteCurrent(&_PyRuntime);
struct _gilstate_runtime_state *gilstate = &_PyRuntime.gilstate;
PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate);
_PyThreadState_DeleteCurrent(tstate);
}


Expand Down Expand Up @@ -938,9 +943,7 @@ PyThreadState *
PyThreadState_Get(void)
{
PyThreadState *tstate = _PyThreadState_GET();
if (tstate == NULL) {
Py_FatalError("no current thread");
}
ensure_tstate_not_null(__func__, tstate);
return tstate;
}

Expand Down Expand Up @@ -1342,8 +1345,8 @@ void
PyGILState_Release(PyGILState_STATE oldstate)
{
_PyRuntimeState *runtime = &_PyRuntime;
PyThreadState *tcur = PyThread_tss_get(&runtime->gilstate.autoTSSkey);
if (tcur == NULL) {
PyThreadState *tstate = PyThread_tss_get(&runtime->gilstate.autoTSSkey);
if (tstate == NULL) {
Py_FatalError("auto-releasing thread-state, "
"but no thread-state for this thread");
}
Expand All @@ -1353,26 +1356,27 @@ PyGILState_Release(PyGILState_STATE oldstate)
but while this is very new (April 2003), the extra check
by release-only users can't hurt.
*/
if (!PyThreadState_IsCurrent(tcur)) {
if (!PyThreadState_IsCurrent(tstate)) {
Py_FatalError("This thread state must be current when releasing");
}
assert(PyThreadState_IsCurrent(tcur));
--tcur->gilstate_counter;
assert(tcur->gilstate_counter >= 0); /* illegal counter value */
assert(PyThreadState_IsCurrent(tstate));
--tstate->gilstate_counter;
assert(tstate->gilstate_counter >= 0); /* illegal counter value */

/* If we're going to destroy this thread-state, we must
* clear it while the GIL is held, as destructors may run.
*/
if (tcur->gilstate_counter == 0) {
if (tstate->gilstate_counter == 0) {
/* can't have been locked when we created it */
assert(oldstate == PyGILState_UNLOCKED);
PyThreadState_Clear(tcur);
PyThreadState_Clear(tstate);
/* Delete the thread-state. Note this releases the GIL too!
* It's vital that the GIL be held here, to avoid shutdown
* races; see bugs 225673 and 1061968 (that nasty bug has a
* habit of coming back).
*/
_PyThreadState_DeleteCurrent(runtime);
assert(_PyRuntimeGILState_GetThreadState(&runtime->gilstate) == tstate);
_PyThreadState_DeleteCurrent(tstate);
}
/* Release the lock if necessary */
else if (oldstate == PyGILState_UNLOCKED)
Expand Down