Prevent dropping unsendable classes on other threads.

We already have checks in place to avoid borrowing these classes on other
threads but it was still possible to send them to another thread and drop them
there (while holding the GIL).

This change avoids running the `Drop` implementation in such a case even though
Python will still free the underlying memory. This might leak resources owned by
the object, but it avoids undefined behaviour due to access the unsendable type
from another thread.

This does assume that the object was not unsafely integrated into an intrusive
data structures which still point to the now freed memory. In that case, the
only recourse would be to abort the process as freeing the memory is unavoidable
when the tp_dealloc slot is called. (And moving it elsewhere into a new
allocation would still break any existing pointers.)

newsfragments/3176.fixed.md

@@ -0,0 +1 @@
+Avoid running the `Drop` implementations of unsendable classes on other threads

src/impl_/pyclass.rs

@@ -1,5 +1,5 @@
 use crate::{
-    exceptions::{PyAttributeError, PyNotImplementedError, PyValueError},
+    exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError},
     ffi,
     impl_::freelist::FreeList,
     impl_::pycell::{GetBorrowChecker, PyClassMutability},
@@ -884,6 +884,7 @@ impl<T> PyClassNewTextSignature<T> for &'_ PyClassImplCollector<T> {
 #[doc(hidden)]
 pub trait PyClassThreadChecker<T>: Sized {
     fn ensure(&self);
+    fn can_drop(&self, py: Python<'_>) -> bool;
     fn new() -> Self;
     private_decl! {}
 }
@@ -894,6 +895,9 @@ pub struct ThreadCheckerStub<T: Send>(PhantomData<T>);
 
 impl<T: Send> PyClassThreadChecker<T> for ThreadCheckerStub<T> {
     fn ensure(&self) {}
+    fn can_drop(&self, _py: Python<'_>) -> bool {
+        true
+    }
     #[inline]
     fn new() -> Self {
         ThreadCheckerStub(PhantomData)
@@ -903,6 +907,9 @@ impl<T: Send> PyClassThreadChecker<T> for ThreadCheckerStub<T> {
 
 impl<T: PyNativeType> PyClassThreadChecker<T> for ThreadCheckerStub<crate::PyObject> {
     fn ensure(&self) {}
+    fn can_drop(&self, _py: Python<'_>) -> bool {
+        true
+    }
     #[inline]
     fn new() -> Self {
         ThreadCheckerStub(PhantomData)
@@ -924,6 +931,18 @@ impl<T> PyClassThreadChecker<T> for ThreadCheckerImpl<T> {
             std::any::type_name::<T>()
         );
     }
+    fn can_drop(&self, py: Python<'_>) -> bool {
+        if thread::current().id() != self.0 {
+            PyRuntimeError::new_err(format!(
+                "{} is unsendbale, but is dropped on another thread!",
+                std::any::type_name::<T>()
+            ))
+            .write_unraisable(py, None);
+            return false;
+        }
+
+        true
+    }
     fn new() -> Self {
         ThreadCheckerImpl(thread::current().id(), PhantomData)
     }
@@ -944,6 +963,9 @@ impl<T: PyClass + Send, U: PyClassBaseType> PyClassThreadChecker<T>
     fn ensure(&self) {
         self.1.ensure();
     }
+    fn can_drop(&self, py: Python<'_>) -> bool {
+        self.1.can_drop(py)
+    }
     fn new() -> Self {
         ThreadCheckerInherited(PhantomData, U::ThreadChecker::new())
     }

src/pycell.rs

@@ -937,7 +937,9 @@ where
     unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) {
         // Safety: Python only calls tp_dealloc when no references to the object remain.
         let cell = &mut *(slf as *mut PyCell<T>);
-        ManuallyDrop::drop(&mut cell.contents.value);
+        if cell.contents.thread_checker.can_drop(py) {
+            ManuallyDrop::drop(&mut cell.contents.value);
+        }
         cell.contents.dict.clear_dict(py);
         cell.contents.weakref.clear_weakrefs(slf, py);
         <T::BaseType as PyClassBaseType>::LayoutAsBase::tp_dealloc(py, slf)

tests/test_class_basics.rs

@@ -526,3 +526,41 @@ fn access_frozen_class_without_gil() {
 
     assert_eq!(py_counter.get().value.load(Ordering::Relaxed), 1);
 }
+
+#[test]
+fn drop_unsendable_elsewhere() {
+    use std::sync::{
+        atomic::{AtomicBool, Ordering},
+        Arc,
+    };
+    use std::thread::spawn;
+
+    #[pyclass(unsendable)]
+    struct Unsendable {
+        dropped: Arc<AtomicBool>,
+    }
+
+    impl Drop for Unsendable {
+        fn drop(&mut self) {
+            self.dropped.store(true, Ordering::SeqCst);
+        }
+    }
+
+    let dropped = Arc::new(AtomicBool::new(false));
+
+    let unsendable = Python::with_gil(|py| {
+        let dropped = dropped.clone();
+
+        Py::new(py, Unsendable { dropped }).unwrap()
+    });
+
+    spawn(move || {
+        Python::with_gil(move |_py| {
+            drop(unsendable);
+        });
+    })
+    .join()
+    .unwrap();
+
+    assert!(!dropped.load(Ordering::SeqCst));
+}