summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Include/cpython/pyatomic.h17
-rw-r--r--Include/internal/pycore_interp.h16
-rw-r--r--Include/internal/pycore_pystate.h8
-rw-r--r--Include/internal/pycore_runtime.h16
-rw-r--r--Lib/test/test_interpreters.py21
-rw-r--r--Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst4
-rw-r--r--Python/pystate.c17
7 files changed, 96 insertions, 3 deletions
diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h
index ab18238..ce23e13 100644
--- a/Include/cpython/pyatomic.h
+++ b/Include/cpython/pyatomic.h
@@ -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
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index ba5764e..0912bd1 100644
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -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;
@@ -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);
+ }
}
diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index 9fc8ae9..2e568f8 100644
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -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);
}
diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h
index 0ddc405..cc3a342 100644
--- a/Include/internal/pycore_runtime.h
+++ b/Include/internal/pycore_runtime.h
@@ -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;
@@ -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
diff --git a/Lib/test/test_interpreters.py b/Lib/test/test_interpreters.py
index 90932c0..9c0dac7 100644
--- a/Lib/test/test_interpreters.py
+++ b/Lib/test/test_interpreters.py
@@ -1,5 +1,6 @@
import contextlib
import os
+import sys
import threading
from textwrap import dedent
import unittest
@@ -487,6 +488,26 @@ class StressTests(TestBase):
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):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst
new file mode 100644
index 0000000..d2dc4c8
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst
@@ -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.
diff --git a/Python/pystate.c b/Python/pystate.c
index dcc6c11..570b524 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -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;
}