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. diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 68b4bd08da1..1fa19ab65d2 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -1,3 +1,5 @@ +use std::cell::Cell; + use pyo3::{ prelude::*, types::{PyDict, PyString}, @@ -31,32 +33,35 @@ 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; +#[pyclass] +struct MustDropWhileAttached; - fn deref(&self) -> &Self::Target { - &self.0 +impl Drop for MustDropWhileAttached { + fn drop(&mut self) { + // SAFETY: always callable; fatal error (abort) if the thread is not attached. + unsafe { pyo3::ffi::PyThreadState_Get() }; } } -// SAFETY: only used to allow the receiver to be used after detaching -unsafe impl Sync for SyncReceiver {} +thread_local! { + // 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() -> LockHolder { +fn detach_during_finalization(py: Python<'_>) -> LockHolder { let (sender, receiver) = std::sync::mpsc::channel(); - let receiver = SyncReceiver(receiver); + let (ready_sender, ready_receiver) = std::sync::mpsc::channel(); std::thread::spawn(move || { Python::attach(|py| { - py.detach(|| { - receiver.recv().ok(); - // Interpreter is finalizing while we try to reattach after returning - }); + DROPPED_ON_THREAD_EXIT.set(Some(Py::new(py, MustDropWhileAttached).unwrap())); + ready_sender.send(()).unwrap(); + py.detach(move || receiver.recv().ok()); + // Interpreter is finalizing while we try to reattach after returning }); }); + py.detach(move || ready_receiver.recv()).unwrap(); LockHolder { sender } } 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))] {