From 407820d3b5fac2ade3e5185145daea96f95e3a77 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 10 Sep 2026 17:13:55 -0600 Subject: [PATCH 1/4] Record successful attach only after attaching in SuspendAttach --- pytests/src/misc.rs | 70 +++++++++++++++++++++++++++++++++++++++++-- src/internal/state.rs | 2 +- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 68b4bd08da1..cd66fa5f438 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -1,3 +1,5 @@ +use std::cell::RefCell; + use pyo3::{ prelude::*, types::{PyDict, PyString}, @@ -15,6 +17,24 @@ struct LockHolder { sender: std::sync::mpsc::Sender<()>, } +#[pyclass] +struct FinalizationLockHolder { + sender: Option>, + done: SyncReceiver<()>, + wait_for_thread: bool, +} + +impl Drop for FinalizationLockHolder { + fn drop(&mut self) { + self.sender.take(); + if self.wait_for_thread { + self.done + .recv_timeout(std::time::Duration::from_secs(10)) + .ok(); + } + } +} + // This will repeatedly attach and detach from the Python interpreter // once the LockHolder is dropped. #[pyfunction] @@ -42,22 +62,66 @@ impl std::ops::Deref for SyncReceiver { } } -// SAFETY: only used to allow the receiver to be used after detaching +// SAFETY: each wrapped receiver is accessed from only one thread. unsafe impl Sync for SyncReceiver {} +struct FinalizationThreadLocal { + object: Option>, + done: std::sync::mpsc::Sender<()>, +} + +#[pyclass] +struct MustDropWhileAttached; + +impl Drop for MustDropWhileAttached { + fn drop(&mut self) { + // SAFETY: PyGILState_Check can always be called. + if unsafe { pyo3::ffi::PyGILState_Check() } == 0 { + std::process::abort(); + } + } +} + +impl Drop for FinalizationThreadLocal { + fn drop(&mut self) { + self.object.take(); + self.done.send(()).ok(); + } +} + +thread_local! { + static DETACH_DURING_FINALIZATION_CONTEXT: RefCell> = const { RefCell::new(None) }; +} + #[pyfunction] -fn detach_during_finalization() -> LockHolder { +fn detach_during_finalization(py: Python<'_>) -> FinalizationLockHolder { let (sender, receiver) = std::sync::mpsc::channel(); + let (ready_sender, ready_receiver) = std::sync::mpsc::channel(); + let (done_sender, done_receiver) = std::sync::mpsc::channel(); let receiver = SyncReceiver(receiver); std::thread::spawn(move || { Python::attach(|py| { + DETACH_DURING_FINALIZATION_CONTEXT.with_borrow_mut(|context| { + *context = Some(FinalizationThreadLocal { + object: Some(Py::new(py, MustDropWhileAttached).unwrap().into_any()), + done: done_sender, + }); + }); + ready_sender.send(()).unwrap(); py.detach(|| { receiver.recv().ok(); // Interpreter is finalizing while we try to reattach after returning }); }); }); - LockHolder { sender } + py.detach(move || ready_receiver.recv()).unwrap(); + FinalizationLockHolder { + sender: Some(sender), + done: SyncReceiver(done_receiver), + // Older CPython releases run TLS destructors here on macOS and musl. + wait_for_thread: cfg!(any(target_os = "macos", target_env = "musl")) + && py.version_info() < (3, 13, 8), + } } #[pyfunction] diff --git a/src/internal/state.rs b/src/internal/state.rs index 728dbc291ca..44acc9edf89 100644 --- a/src/internal/state.rs +++ b/src/internal/state.rs @@ -300,9 +300,9 @@ impl SuspendAttach { impl Drop for SuspendAttach { fn drop(&mut self) { - ATTACH_COUNT.with(|c| c.set(self.count)); // SAFETY: tstate come from call to PyEval_SaveThread and it was not re-attached yet unsafe { ffi::PyEval_RestoreThread(self.tstate) }; + ATTACH_COUNT.with(|c| c.set(self.count)); // Update counts of `Py` that were dropped while not attached. #[cfg(not(pyo3_disable_reference_pool))] { From dfba405f2a8037689d2a3a2d354f2ca14195cc36 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 10 Sep 2026 19:29:59 -0600 Subject: [PATCH 2/4] fix limited API builds --- pytests/src/misc.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index cd66fa5f438..09e5d0b2fe7 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -75,9 +75,12 @@ struct MustDropWhileAttached; impl Drop for MustDropWhileAttached { fn drop(&mut self) { - // SAFETY: PyGILState_Check can always be called. - if unsafe { pyo3::ffi::PyGILState_Check() } == 0 { - std::process::abort(); + #[cfg(not(Py_LIMITED_API))] + { + // SAFETY: PyGILState_Check can always be called. + if unsafe { pyo3::ffi::PyGILState_Check() } == 0 { + std::process::abort(); + } } } } From 1046b2bb715719d786f2b6c67cd8fa46359e9946 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 10 Sep 2026 19:35:36 -0600 Subject: [PATCH 3/4] add release note --- newsfragments/6404.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/6404.fixed.md diff --git a/newsfragments/6404.fixed.md b/newsfragments/6404.fixed.md new file mode 100644 index 00000000000..9f85271d790 --- /dev/null +++ b/newsfragments/6404.fixed.md @@ -0,0 +1 @@ +Fix crash when a detached thread is terminated while trying to reattach during interpreter finalization. From 63cb82b224b824987c58df59ee57faa417e4ddc4 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 10 Sep 2026 19:56:57 -0600 Subject: [PATCH 4/4] simplify test --- pytests/src/misc.rs | 84 ++++++--------------------------------------- 1 file changed, 11 insertions(+), 73 deletions(-) diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 09e5d0b2fe7..1fa19ab65d2 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -1,4 +1,4 @@ -use std::cell::RefCell; +use std::cell::Cell; use pyo3::{ prelude::*, @@ -17,24 +17,6 @@ struct LockHolder { sender: std::sync::mpsc::Sender<()>, } -#[pyclass] -struct FinalizationLockHolder { - sender: Option>, - done: SyncReceiver<()>, - wait_for_thread: bool, -} - -impl Drop for FinalizationLockHolder { - fn drop(&mut self) { - self.sender.take(); - if self.wait_for_thread { - self.done - .recv_timeout(std::time::Duration::from_secs(10)) - .ok(); - } - } -} - // This will repeatedly attach and detach from the Python interpreter // once the LockHolder is dropped. #[pyfunction] @@ -51,80 +33,36 @@ fn hammer_attaching_in_thread() -> LockHolder { LockHolder { sender } } -/// Wrapper to mark Receiver as Sync. -struct SyncReceiver(std::sync::mpsc::Receiver); - -impl std::ops::Deref for SyncReceiver { - type Target = std::sync::mpsc::Receiver; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -// SAFETY: each wrapped receiver is accessed from only one thread. -unsafe impl Sync for SyncReceiver {} - -struct FinalizationThreadLocal { - object: Option>, - done: std::sync::mpsc::Sender<()>, -} - #[pyclass] struct MustDropWhileAttached; impl Drop for MustDropWhileAttached { fn drop(&mut self) { - #[cfg(not(Py_LIMITED_API))] - { - // SAFETY: PyGILState_Check can always be called. - if unsafe { pyo3::ffi::PyGILState_Check() } == 0 { - std::process::abort(); - } - } - } -} - -impl Drop for FinalizationThreadLocal { - fn drop(&mut self) { - self.object.take(); - self.done.send(()).ok(); + // SAFETY: always callable; fatal error (abort) if the thread is not attached. + unsafe { pyo3::ffi::PyThreadState_Get() }; } } thread_local! { - static DETACH_DURING_FINALIZATION_CONTEXT: RefCell> = const { RefCell::new(None) }; + // Dropped when the thread exits, which on older CPython happens inside + // PyEval_RestoreThread when reattaching during finalization. + static DROPPED_ON_THREAD_EXIT: Cell>> = const { Cell::new(None) }; } #[pyfunction] -fn detach_during_finalization(py: Python<'_>) -> FinalizationLockHolder { +fn detach_during_finalization(py: Python<'_>) -> LockHolder { let (sender, receiver) = std::sync::mpsc::channel(); let (ready_sender, ready_receiver) = std::sync::mpsc::channel(); - let (done_sender, done_receiver) = std::sync::mpsc::channel(); - let receiver = SyncReceiver(receiver); std::thread::spawn(move || { Python::attach(|py| { - DETACH_DURING_FINALIZATION_CONTEXT.with_borrow_mut(|context| { - *context = Some(FinalizationThreadLocal { - object: Some(Py::new(py, MustDropWhileAttached).unwrap().into_any()), - done: done_sender, - }); - }); + DROPPED_ON_THREAD_EXIT.set(Some(Py::new(py, MustDropWhileAttached).unwrap())); ready_sender.send(()).unwrap(); - py.detach(|| { - receiver.recv().ok(); - // Interpreter is finalizing while we try to reattach after returning - }); + py.detach(move || receiver.recv().ok()); + // Interpreter is finalizing while we try to reattach after returning }); }); py.detach(move || ready_receiver.recv()).unwrap(); - FinalizationLockHolder { - sender: Some(sender), - done: SyncReceiver(done_receiver), - // Older CPython releases run TLS destructors here on macOS and musl. - wait_for_thread: cfg!(any(target_os = "macos", target_env = "musl")) - && py.version_info() < (3, 13, 8), - } + LockHolder { sender } } #[pyfunction]