Skip to content

gh-109793: Allow Switching Interpreters During Finalization #109794

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
17 changes: 17 additions & 0 deletions Include/cpython/pyatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -501,3 +501,20 @@ static inline void _Py_atomic_fence_release(void);
#else
# error "no available pyatomic implementation for this platform/compiler"
#endif


// --- aliases ---------------------------------------------------------------

#if SIZEOF_LONG == 8
# define _Py_atomic_load_ulong _Py_atomic_load_uint64
# define _Py_atomic_load_ulong_relaxed _Py_atomic_load_uint64_relaxed
# define _Py_atomic_store_ulong _Py_atomic_store_uint64
# define _Py_atomic_store_ulong_relaxed _Py_atomic_store_uint64_relaxed
#elif SIZEOF_LONG == 4
# define _Py_atomic_load_ulong _Py_atomic_load_uint32
# define _Py_atomic_load_ulong_relaxed _Py_atomic_load_uint32_relaxed
# define _Py_atomic_store_ulong _Py_atomic_store_uint32
# define _Py_atomic_store_ulong_relaxed _Py_atomic_store_uint32_relaxed
#else
# error "long must be 4 or 8 bytes in size"
#endif // SIZEOF_LONG
16 changes: 16 additions & 0 deletions Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ struct _is {
and _PyInterpreterState_SetFinalizing()
to access it, don't access it directly. */
_Py_atomic_address _finalizing;
/* The ID of the OS thread in which we are finalizing. */
unsigned long _finalizing_id;

struct _gc_runtime_state gc;

Expand Down Expand Up @@ -215,9 +217,23 @@ _PyInterpreterState_GetFinalizing(PyInterpreterState *interp) {
return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing);
}

static inline unsigned long
_PyInterpreterState_GetFinalizingID(PyInterpreterState *interp) {
return _Py_atomic_load_ulong_relaxed(&interp->_finalizing_id);
}

static inline void
_PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) {
_Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate);
if (tstate == NULL) {
_Py_atomic_store_ulong_relaxed(&interp->_finalizing_id, 0);
}
else {
// XXX Re-enable this assert once gh-109860 is fixed.
//assert(tstate->thread_id == PyThread_get_thread_ident());
_Py_atomic_store_ulong_relaxed(&interp->_finalizing_id,
tstate->thread_id);
}
}


Expand Down
8 changes: 6 additions & 2 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ _Py_IsMainInterpreter(PyInterpreterState *interp)
static inline int
_Py_IsMainInterpreterFinalizing(PyInterpreterState *interp)
{
return (_PyRuntimeState_GetFinalizing(interp->runtime) != NULL &&
interp == &interp->runtime->_main_interpreter);
/* bpo-39877: Access _PyRuntime directly rather than using
tstate->interp->runtime to support calls from Python daemon threads.
After Py_Finalize() has been called, tstate can be a dangling pointer:
point to PyThreadState freed memory. */
return (_PyRuntimeState_GetFinalizing(&_PyRuntime) != NULL &&
interp == &_PyRuntime._main_interpreter);
}


Expand Down
16 changes: 16 additions & 0 deletions Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ typedef struct pyruntimestate {
Use _PyRuntimeState_GetFinalizing() and _PyRuntimeState_SetFinalizing()
to access it, don't access it directly. */
_Py_atomic_address _finalizing;
/* The ID of the OS thread in which we are finalizing. */
unsigned long _finalizing_id;

struct pyinterpreters {
PyThread_type_lock mutex;
Expand Down Expand Up @@ -303,9 +305,23 @@ _PyRuntimeState_GetFinalizing(_PyRuntimeState *runtime) {
return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->_finalizing);
}

static inline unsigned long
_PyRuntimeState_GetFinalizingID(_PyRuntimeState *runtime) {
return _Py_atomic_load_ulong_relaxed(&runtime->_finalizing_id);
}

static inline void
_PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
_Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);
if (tstate == NULL) {
_Py_atomic_store_ulong_relaxed(&runtime->_finalizing_id, 0);
}
else {
// XXX Re-enable this assert once gh-109860 is fixed.
//assert(tstate->thread_id == PyThread_get_thread_ident());
_Py_atomic_store_ulong_relaxed(&runtime->_finalizing_id,
tstate->thread_id);
}
}

#ifdef __cplusplus
Expand Down
21 changes: 21 additions & 0 deletions Lib/test/test_interpreters.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import contextlib
import os
import sys
import threading
from textwrap import dedent
import unittest
Expand Down Expand Up @@ -487,6 +488,26 @@ def task():
pass


class FinalizationTests(TestBase):

def test_gh_109793(self):
import subprocess
argv = [sys.executable, '-c', '''if True:
import _xxsubinterpreters as _interpreters
interpid = _interpreters.create()
raise Exception
''']
proc = subprocess.run(argv, capture_output=True, text=True)
self.assertIn('Traceback', proc.stderr)
if proc.returncode == 0 and support.verbose:
print()
print("--- cmd unexpected succeeded ---")
print(f"stdout:\n{proc.stdout}")
print(f"stderr:\n{proc.stderr}")
print("------")
self.assertEqual(proc.returncode, 1)


class TestIsShareable(TestBase):

def test_default_shareables(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The main thread no longer exits prematurely when a subinterpreter
is cleaned up during runtime finalization. The bug was a problem
particularly because, when triggered, the Python process would
always return with a 0 exitcode, even if it failed.
17 changes: 16 additions & 1 deletion Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2964,11 +2964,26 @@ _PyThreadState_MustExit(PyThreadState *tstate)
tstate->interp->runtime to support calls from Python daemon threads.
After Py_Finalize() has been called, tstate can be a dangling pointer:
point to PyThreadState freed memory. */
unsigned long finalizing_id = _PyRuntimeState_GetFinalizingID(&_PyRuntime);
PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
if (finalizing == NULL) {
// XXX This isn't completely safe from daemon thraeds,
// since tstate might be a dangling pointer.
finalizing = _PyInterpreterState_GetFinalizing(tstate->interp);
finalizing_id = _PyInterpreterState_GetFinalizingID(tstate->interp);
}
return (finalizing != NULL && finalizing != tstate);
// XXX else check &_PyRuntime._main_interpreter._initial_thread
if (finalizing == NULL) {
return 0;
}
else if (finalizing == tstate) {
return 0;
}
else if (finalizing_id == PyThread_get_thread_ident()) {
/* gh-109793: we must have switched interpreters. */
return 0;
}
return 1;
}


Expand Down