Remove deferred reference count increments and make the global reference pool optional (#4095)

* Add feature controlling the global reference pool to enable avoiding its overhead.

* Document reference-pool feature in the performance guide.

* Invert semantics of feature to disable reference pool so the new behaviour becomes opt-in

* Remove delayed reference count increments as we cannot prevent reference count errors as long as these are available

* Adjust tests to be compatible with disable-reference-pool feature

* Adjust tests to be compatible with py-clone feature

* Adjust the GIL benchmark to the updated reference pool semantics.

* Further extend and clarify the documentation of the py-clone and disable-reference-pool features

* Replace disable-reference-pool feature by pyo3_disable_reference_pool conditional compilation flag

Such a flag is harder to use and thereby also harder to abuse. This seems
appropriate as this is purely a performance-oriented change which show only be
enabled by leaf crates and brings with it additional highly implicit sources of
process aborts.

* Add pyo3_leak_on_drop_without_reference_pool to turn aborts into leaks when the global reference pool is disabled and the GIL is not held

Cargo.toml

@@ -108,6 +108,9 @@ auto-initialize = []
 # Allows use of the deprecated "GIL Refs" APIs.
 gil-refs = []
 
+# Enables `Clone`ing references to Python objects `Py<T>` which panics if the GIL is not held.
+py-clone = []
+
 # Optimizes PyObject to Vec conversion and so on.
 nightly = []
 
@@ -129,6 +132,7 @@ full = [
     "num-bigint",
     "num-complex",
     "num-rational",
+    "py-clone",
     "rust_decimal",
     "serde",
     "smallvec",

examples/Cargo.toml

@@ -10,5 +10,5 @@ pyo3 = { path = "..", features = ["auto-initialize", "extension-module"] }
 [[example]]
 name = "decorator"
 path = "decorator/src/lib.rs"
-crate_type = ["cdylib"]
+crate-type = ["cdylib"]
 doc-scrape-examples = true

guide/src/class.md

@@ -249,7 +249,7 @@ fn return_myclass() -> Py<MyClass> {
 
 let obj = return_myclass();
 
-Python::with_gil(|py| {
+Python::with_gil(move |py| {
     let bound = obj.bind(py); // Py<MyClass>::bind returns &Bound<'py, MyClass>
     let obj_ref = bound.borrow(); // Get PyRef<T>
     assert_eq!(obj_ref.num, 1);
@@ -280,6 +280,8 @@ let py_counter: Py<FrozenCounter> = Python::with_gil(|py| {
 });
 
 py_counter.get().value.fetch_add(1, Ordering::Relaxed);
+
+Python::with_gil(move |_py| drop(py_counter));
 ```
 
 Frozen classes are likely to become the default thereby guiding the PyO3 ecosystem towards a more deliberate application of interior mutability. Eventually, this should enable further optimizations of PyO3's internals and avoid downstream code paying the cost of interior mutability when it is not actually required.

guide/src/faq.md

@@ -127,12 +127,10 @@ If you don't want that cloning to happen, a workaround is to allocate the field
 ```rust
 # use pyo3::prelude::*;
 #[pyclass]
-#[derive(Clone)]
 struct Inner {/* fields omitted */}
 
 #[pyclass]
 struct Outer {
-    #[pyo3(get)]
     inner: Py<Inner>,
 }
 
@@ -144,6 +142,11 @@ impl Outer {
             inner: Py::new(py, Inner {})?,
         })
     }
+
+    #[getter]
+    fn inner(&self, py: Python<'_>) -> Py<Inner> {
+        self.inner.clone_ref(py)
+    }
 }
 ```
 This time `a` and `b` *are* the same object:

guide/src/features.md

@@ -75,6 +75,14 @@ This feature is a backwards-compatibility feature to allow continued use of the
 
 This feature and the APIs it enables is expected to be removed in a future PyO3 version.
 
+### `py-clone`
+
+This feature was introduced to ease migration. It was found that delayed reference counts cannot be made sound and hence `Clon`ing an instance of `Py<T>` must panic without the GIL being held. To avoid migrations introducing new panics without warning, the `Clone` implementation itself is now gated behind this feature.
+
+### `pyo3_disable_reference_pool`
+
+This is a performance-oriented conditional compilation flag, e.g. [set via `$RUSTFLAGS`][set-configuration-options], which disabled the global reference pool and the assocaited overhead for the crossing the Python-Rust boundary. However, if enabled, `Drop`ping an instance of `Py<T>` without the GIL being held will abort the process.
+
 ### `macros`
 
 This feature enables a dependency on the `pyo3-macros` crate, which provides the procedural macros portion of PyO3's API:
@@ -195,3 +203,5 @@ struct User {
 ### `smallvec`
 
 Adds a dependency on [smallvec](https://docs.rs/smallvec) and enables conversions into its [`SmallVec`](https://docs.rs/smallvec/latest/smallvec/struct.SmallVec.html) type.
+
+[set-configuration-options]: https://doc.rust-lang.org/reference/conditional-compilation.html#set-configuration-options

guide/src/memory.md

@@ -212,7 +212,8 @@ This example wasn't very interesting.  We could have just used a GIL-bound
 we are *not* holding the GIL?
 
 ```rust
-# #![allow(unused_imports)]
+# #![allow(unused_imports, dead_code)]
+# #[cfg(not(pyo3_disable_reference_pool))] {
 # use pyo3::prelude::*;
 # use pyo3::types::PyString;
 # fn main() -> PyResult<()> {
@@ -239,12 +240,14 @@ Python::with_gil(|py|
 # }
 # Ok(())
 # }
+# }
 ```
 
 When `hello` is dropped *nothing* happens to the pointed-to memory on Python's
 heap because nothing _can_ happen if we're not holding the GIL.  Fortunately,
-the memory isn't leaked.  PyO3 keeps track of the memory internally and will
-release it the next time we acquire the GIL.
+the memory isn't leaked. If the `pyo3_disable_reference_pool` conditional compilation flag
+is not enabled, PyO3 keeps track of the memory internally and will release it
+the next time we acquire the GIL.
 
 We can avoid the delay in releasing memory if we are careful to drop the
 `Py<Any>` while the GIL is held.

guide/src/migration.md

@@ -35,7 +35,16 @@ fn increment(x: u64, amount: Option<u64>) -> u64 {
     x + amount.unwrap_or(1)
 }
 ```
+</details>
 
+### `Py::clone` is now gated behind the `py-clone` feature
+<details open>
+<summary><small>Click to expand</small></summary>
+If you rely on `impl<T> Clone for Py<T>` to fulfil trait requirements imposed by existing Rust code written without PyO3-based code in mind, the newly introduced feature `py-clone` must be enabled.
+
+However, take care to note that the behaviour is different from previous versions. If `Clone` was called without the GIL being held, we tried to delay the application of these reference count increments until PyO3-based code would re-acquire it. This turned out to be impossible to implement in a sound manner and hence was removed. Now, if `Clone` is called without the GIL being held, we panic instead for which calling code might not be prepared.
+
+Related to this, we also added a `pyo3_disable_reference_pool` conditional compilation flag which removes the infrastructure necessary to apply delayed reference count decrements implied by `impl<T> Drop for Py<T>`. They do not appear to be a soundness hazard as they should lead to memory leaks in the worst case. However, the global synchronization adds significant overhead to cross the Python-Rust boundary. Enabling this feature will remove these costs and make the `Drop` implementation abort the process if called without the GIL being held instead.
 </details>
 
 ## from 0.20.* to 0.21
@@ -676,7 +685,7 @@ drop(second);
 
 The replacement is [`Python::with_gil`](https://docs.rs/pyo3/0.18.3/pyo3/marker/struct.Python.html#method.with_gil) which is more cumbersome but enforces the proper nesting by design, e.g.
 
-```rust
+```rust,ignore
 # #![allow(dead_code)]
 # use pyo3::prelude::*;
 
@@ -701,7 +710,7 @@ let second = Python::with_gil(|py| Object::new(py));
 drop(first);
 drop(second);
 
-// Or it ensure releasing the inner lock before the outer one.
+// Or it ensures releasing the inner lock before the outer one.
 Python::with_gil(|py| {
     let first = Object::new(py);
     let second = Python::with_gil(|py| Object::new(py));

guide/src/performance.md

@@ -96,3 +96,47 @@ impl PartialEq<Foo> for FooBound<'_> {
     }
 }
 ```
+
+## Disable the global reference pool
+
+PyO3 uses global mutable state to keep track of deferred reference count updates implied by `impl<T> Drop for Py<T>` being called without the GIL being held. The necessary synchronization to obtain and apply these reference count updates when PyO3-based code next acquires the GIL is somewhat expensive and can become a significant part of the cost of crossing the Python-Rust boundary.
+
+This functionality can be avoided by setting the `pyo3_disable_reference_pool` conditional compilation flag. This removes the global reference pool and the associated costs completely. However, it does _not_ remove the `Drop` implementation for `Py<T>` which is necessary to interoperate with existing Rust code written without PyO3-based code in mind. To stay compatible with the wider Rust ecosystem in these cases, we keep the implementation but abort when `Drop` is called without the GIL being held. If `pyo3_leak_on_drop_without_reference_pool` is additionally enabled, objects dropped without the GIL being held will be leaked instead which is always sound but might have determinal effects like resource exhaustion in the long term.
+
+This limitation is important to keep in mind when this setting is used, especially when embedding Python code into a Rust application as it is quite easy to accidentally drop a `Py<T>` (or types containing it like `PyErr`, `PyBackedStr` or `PyBackedBytes`) returned from `Python::with_gil` without making sure to re-acquire the GIL beforehand. For example, the following code
+
+```rust,ignore
+# use pyo3::prelude::*;
+# use pyo3::types::PyList;
+let numbers: Py<PyList> = Python::with_gil(|py| PyList::empty_bound(py).unbind());
+
+Python::with_gil(|py| {
+    numbers.bind(py).append(23).unwrap();
+});
+
+Python::with_gil(|py| {
+    numbers.bind(py).append(42).unwrap();
+});
+```
+
+will abort if the list not explicitly disposed via
+
+```rust
+# use pyo3::prelude::*;
+# use pyo3::types::PyList;
+let numbers: Py<PyList> = Python::with_gil(|py| PyList::empty_bound(py).unbind());
+
+Python::with_gil(|py| {
+    numbers.bind(py).append(23).unwrap();
+});
+
+Python::with_gil(|py| {
+    numbers.bind(py).append(42).unwrap();
+});
+
+Python::with_gil(move |py| {
+    drop(numbers);
+});
+```
+
+[conditional-compilation]: https://doc.rust-lang.org/reference/conditional-compilation.html

newsfragments/4095.added.md

@@ -0,0 +1 @@
+Add `pyo3_disable_reference_pool` conditional compilation flag to avoid the overhead of the global reference pool at the cost of known limitations as explained in the performance section of the guide.

newsfragments/4095.changed.md

@@ -0,0 +1 @@
+`Clone`ing pointers into the Python heap has been moved behind the `py-clone` feature, as it must panic without the GIL being held as a soundness fix.

pyo3-benches/benches/bench_gil.rs

@@ -1,4 +1,4 @@
-use codspeed_criterion_compat::{criterion_group, criterion_main, BatchSize, Bencher, Criterion};
+use codspeed_criterion_compat::{criterion_group, criterion_main, Bencher, Criterion};
 
 use pyo3::prelude::*;
 
@@ -9,14 +9,8 @@ fn bench_clean_acquire_gil(b: &mut Bencher<'_>) {
 
 fn bench_dirty_acquire_gil(b: &mut Bencher<'_>) {
     let obj = Python::with_gil(|py| py.None());
-    b.iter_batched(
-        || {
-            // Clone and drop an object so that the GILPool has work to do.
-            let _ = obj.clone();
-        },
-        |_| Python::with_gil(|_| {}),
-        BatchSize::NumBatches(1),
-    );
+    // Drop the returned clone of the object so that the reference pool has work to do.
+    b.iter(|| Python::with_gil(|py| obj.clone_ref(py)));
 }
 
 fn criterion_benchmark(c: &mut Criterion) {

pyo3-build-config/src/lib.rs

@@ -165,6 +165,8 @@ pub fn print_expected_cfgs() {
     println!("cargo:rustc-check-cfg=cfg(GraalPy)");
     println!("cargo:rustc-check-cfg=cfg(py_sys_config, values(\"Py_DEBUG\", \"Py_REF_DEBUG\", \"Py_TRACE_REFS\", \"COUNT_ALLOCS\"))");
     println!("cargo:rustc-check-cfg=cfg(invalid_from_utf8_lint)");
+    println!("cargo:rustc-check-cfg=cfg(pyo3_disable_reference_pool)");
+    println!("cargo:rustc-check-cfg=cfg(pyo3_leak_on_drop_without_reference_pool)");
 
     // allow `Py_3_*` cfgs from the minimum supported version up to the
     // maximum minor version (+1 for development for the next)

src/conversions/std/option.rs

@@ -61,7 +61,7 @@ mod tests {
             assert_eq!(option.as_ptr(), std::ptr::null_mut());
 
             let none = py.None();
-            option = Some(none.clone());
+            option = Some(none.clone_ref(py));
 
             let ref_cnt = none.get_refcnt(py);
             assert_eq!(option.as_ptr(), none.as_ptr());

src/err/err_state.rs

@@ -5,7 +5,6 @@ use crate::{
     Bound, IntoPy, Py, PyAny, PyObject, PyTypeInfo, Python,
 };
 
-#[derive(Clone)]
 pub(crate) struct PyErrStateNormalized {
     #[cfg(not(Py_3_12))]
     ptype: Py<PyType>,
@@ -63,6 +62,19 @@ impl PyErrStateNormalized {
             ptraceback: Py::from_owned_ptr_or_opt(py, ptraceback),
         }
     }
+
+    pub fn clone_ref(&self, py: Python<'_>) -> Self {
+        Self {
+            #[cfg(not(Py_3_12))]
+            ptype: self.ptype.clone_ref(py),
+            pvalue: self.pvalue.clone_ref(py),
+            #[cfg(not(Py_3_12))]
+            ptraceback: self
+                .ptraceback
+                .as_ref()
+                .map(|ptraceback| ptraceback.clone_ref(py)),
+        }
+    }
 }
 
 pub(crate) struct PyErrStateLazyFnOutput {

src/err/mod.rs

@@ -837,7 +837,7 @@ impl PyErr {
     /// ```
     #[inline]
     pub fn clone_ref(&self, py: Python<'_>) -> PyErr {
-        PyErr::from_state(PyErrState::Normalized(self.normalized(py).clone()))
+        PyErr::from_state(PyErrState::Normalized(self.normalized(py).clone_ref(py)))
     }
 
     /// Return the cause (either an exception instance, or None, set by `raise ... from ...`)

src/gil.rs

@@ -1,6 +1,8 @@
 //! Interaction with Python's global interpreter lock
 
 use crate::impl_::not_send::{NotSend, NOT_SEND};
+#[cfg(pyo3_disable_reference_pool)]
+use crate::impl_::panic::PanicTrap;
 use crate::{ffi, Python};
 use std::cell::Cell;
 #[cfg(debug_assertions)]
@@ -233,42 +235,32 @@ impl Drop for GILGuard {
 // Vector of PyObject
 type PyObjVec = Vec<NonNull<ffi::PyObject>>;
 
+#[cfg(not(pyo3_disable_reference_pool))]
 /// Thread-safe storage for objects which were inc_ref / dec_ref while the GIL was not held.
 struct ReferencePool {
-    // .0 is INCREFs, .1 is DECREFs
-    pointer_ops: sync::Mutex<(PyObjVec, PyObjVec)>,
+    pending_decrefs: sync::Mutex<PyObjVec>,
 }
 
+#[cfg(not(pyo3_disable_reference_pool))]
 impl ReferencePool {
     const fn new() -> Self {
         Self {
-            pointer_ops: sync::Mutex::new((Vec::new(), Vec::new())),
+            pending_decrefs: sync::Mutex::new(Vec::new()),
         }
     }
 
-    fn register_incref(&self, obj: NonNull<ffi::PyObject>) {
-        self.pointer_ops.lock().unwrap().0.push(obj);
-    }
-
     fn register_decref(&self, obj: NonNull<ffi::PyObject>) {
-        self.pointer_ops.lock().unwrap().1.push(obj);
+        self.pending_decrefs.lock().unwrap().push(obj);
     }
 
     fn update_counts(&self, _py: Python<'_>) {
-        let mut ops = self.pointer_ops.lock().unwrap();
-        if ops.0.is_empty() && ops.1.is_empty() {
+        let mut pending_decrefs = self.pending_decrefs.lock().unwrap();
+        if pending_decrefs.is_empty() {
             return;
         }
 
-        let (increfs, decrefs) = mem::take(&mut *ops);
-        drop(ops);
-
-        // Always increase reference counts first - as otherwise objects which have a
-        // nonzero total reference count might be incorrectly dropped by Python during
-        // this update.
-        for ptr in increfs {
-            unsafe { ffi::Py_INCREF(ptr.as_ptr()) };
-        }
+        let decrefs = mem::take(&mut *pending_decrefs);
+        drop(pending_decrefs);
 
         for ptr in decrefs {
             unsafe { ffi::Py_DECREF(ptr.as_ptr()) };
@@ -276,8 +268,10 @@ impl ReferencePool {
     }
 }
 
+#[cfg(not(pyo3_disable_reference_pool))]
 unsafe impl Sync for ReferencePool {}
 
+#[cfg(not(pyo3_disable_reference_pool))]
 static POOL: ReferencePool = ReferencePool::new();
 
 /// A guard which can be used to temporarily release the GIL and restore on `Drop`.
@@ -302,6 +296,7 @@ impl Drop for SuspendGIL {
             ffi::PyEval_RestoreThread(self.tstate);
 
             // Update counts of PyObjects / Py that were cloned or dropped while the GIL was released.
+            #[cfg(not(pyo3_disable_reference_pool))]
             POOL.update_counts(Python::assume_gil_acquired());
         }
     }
@@ -376,6 +371,7 @@ impl GILPool {
     pub unsafe fn new() -> GILPool {
         increment_gil_count();
         // Update counts of PyObjects / Py that have been cloned or dropped since last acquisition
+        #[cfg(not(pyo3_disable_reference_pool))]
         POOL.update_counts(Python::assume_gil_acquired());
         GILPool {
             start: OWNED_OBJECTS
@@ -434,11 +430,13 @@ impl Drop for GILPool {
 ///
 /// # Safety
 /// The object must be an owned Python reference.
+#[cfg(feature = "py-clone")]
+#[track_caller]
 pub unsafe fn register_incref(obj: NonNull<ffi::PyObject>) {
     if gil_is_acquired() {
         ffi::Py_INCREF(obj.as_ptr())
     } else {
-        POOL.register_incref(obj);
+        panic!("Cannot clone pointer into Python heap without the GIL being held.");
     }
 }
 
@@ -450,11 +448,21 @@ pub unsafe fn register_incref(obj: NonNull<ffi::PyObject>) {
 ///
 /// # Safety
 /// The object must be an owned Python reference.
+#[track_caller]
 pub unsafe fn register_decref(obj: NonNull<ffi::PyObject>) {
     if gil_is_acquired() {
         ffi::Py_DECREF(obj.as_ptr())
     } else {
+        #[cfg(not(pyo3_disable_reference_pool))]
         POOL.register_decref(obj);
+        #[cfg(all(
+            pyo3_disable_reference_pool,
+            not(pyo3_leak_on_drop_without_reference_pool)
+        ))]
+        {
+            let _trap = PanicTrap::new("Aborting the process to avoid panic-from-drop.");
+            panic!("Cannot drop pointer into Python heap without the GIL being held.");
+        }
     }
 }
 
@@ -508,15 +516,18 @@ fn decrement_gil_count() {
 mod tests {
     #[allow(deprecated)]
     use super::GILPool;
-    use super::{gil_is_acquired, GIL_COUNT, POOL};
+    #[cfg(not(pyo3_disable_reference_pool))]
+    use super::POOL;
+    use super::{gil_is_acquired, GIL_COUNT};
+    #[cfg(not(pyo3_disable_reference_pool))]
+    use crate::ffi;
     use crate::types::any::PyAnyMethods;
-    use crate::{ffi, PyObject, Python};
+    use crate::{PyObject, Python};
     #[cfg(feature = "gil-refs")]
     use {super::OWNED_OBJECTS, crate::gil};
 
+    #[cfg(not(pyo3_disable_reference_pool))]
     use std::ptr::NonNull;
-    #[cfg(not(target_arch = "wasm32"))]
-    use std::sync;
 
     fn get_object(py: Python<'_>) -> PyObject {
         py.eval_bound("object()", None, None).unwrap().unbind()
@@ -531,30 +542,20 @@ mod tests {
         len
     }
 
-    fn pool_inc_refs_does_not_contain(obj: &PyObject) -> bool {
-        !POOL
-            .pointer_ops
-            .lock()
-            .unwrap()
-            .0
-            .contains(&unsafe { NonNull::new_unchecked(obj.as_ptr()) })
-    }
-
+    #[cfg(not(pyo3_disable_reference_pool))]
     fn pool_dec_refs_does_not_contain(obj: &PyObject) -> bool {
         !POOL
-            .pointer_ops
+            .pending_decrefs
             .lock()
             .unwrap()
-            .1
             .contains(&unsafe { NonNull::new_unchecked(obj.as_ptr()) })
     }
 
-    #[cfg(not(target_arch = "wasm32"))]
+    #[cfg(all(not(pyo3_disable_reference_pool), not(target_arch = "wasm32")))]
     fn pool_dec_refs_contains(obj: &PyObject) -> bool {
-        POOL.pointer_ops
+        POOL.pending_decrefs
             .lock()
             .unwrap()
-            .1
             .contains(&unsafe { NonNull::new_unchecked(obj.as_ptr()) })
     }
 
@@ -629,20 +630,20 @@ mod tests {
             let reference = obj.clone_ref(py);
 
             assert_eq!(obj.get_refcnt(py), 2);
-            assert!(pool_inc_refs_does_not_contain(&obj));
+            #[cfg(not(pyo3_disable_reference_pool))]
             assert!(pool_dec_refs_does_not_contain(&obj));
 
             // With the GIL held, reference count will be decreased immediately.
             drop(reference);
 
             assert_eq!(obj.get_refcnt(py), 1);
-            assert!(pool_inc_refs_does_not_contain(&obj));
+            #[cfg(not(pyo3_disable_reference_pool))]
             assert!(pool_dec_refs_does_not_contain(&obj));
         });
     }
 
     #[test]
-    #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
+    #[cfg(all(not(pyo3_disable_reference_pool), not(target_arch = "wasm32")))] // We are building wasm Python with pthreads disabled
     fn test_pyobject_drop_without_gil_doesnt_decrease_refcnt() {
         let obj = Python::with_gil(|py| {
             let obj = get_object(py);
@@ -650,7 +651,6 @@ mod tests {
             let reference = obj.clone_ref(py);
 
             assert_eq!(obj.get_refcnt(py), 2);
-            assert!(pool_inc_refs_does_not_contain(&obj));
             assert!(pool_dec_refs_does_not_contain(&obj));
 
             // Drop reference in a separate thread which doesn't have the GIL.
@@ -659,7 +659,6 @@ mod tests {
             // The reference count should not have changed (the GIL has always
             // been held by this thread), it is remembered to release later.
             assert_eq!(obj.get_refcnt(py), 2);
-            assert!(pool_inc_refs_does_not_contain(&obj));
             assert!(pool_dec_refs_contains(&obj));
             obj
         });
@@ -667,9 +666,7 @@ mod tests {
         // Next time the GIL is acquired, the reference is released
         Python::with_gil(|py| {
             assert_eq!(obj.get_refcnt(py), 1);
-            let non_null = unsafe { NonNull::new_unchecked(obj.as_ptr()) };
-            assert!(!POOL.pointer_ops.lock().unwrap().0.contains(&non_null));
-            assert!(!POOL.pointer_ops.lock().unwrap().1.contains(&non_null));
+            assert!(pool_dec_refs_does_not_contain(&obj));
         });
     }
 
@@ -725,19 +722,16 @@ mod tests {
         assert!(!gil_is_acquired());
     }
 
+    #[cfg(feature = "py-clone")]
     #[test]
+    #[should_panic]
     fn test_allow_threads_updates_refcounts() {
         Python::with_gil(|py| {
             // Make a simple object with 1 reference
             let obj = get_object(py);
             assert!(obj.get_refcnt(py) == 1);
-            // Clone the object without the GIL to use internal tracking
-            let escaped_ref = py.allow_threads(|| obj.clone());
-            // But after the block the refcounts are updated
-            assert!(obj.get_refcnt(py) == 2);
-            drop(escaped_ref);
-            assert!(obj.get_refcnt(py) == 1);
-            drop(obj);
+            // Clone the object without the GIL which should panic
+            py.allow_threads(|| obj.clone());
         });
     }
 
@@ -752,6 +746,7 @@ mod tests {
         })
     }
 
+    #[cfg(feature = "py-clone")]
     #[test]
     fn test_clone_with_gil() {
         Python::with_gil(|py| {
@@ -765,147 +760,8 @@ mod tests {
         })
     }
 
-    #[cfg(not(target_arch = "wasm32"))]
-    struct Event {
-        set: sync::Mutex<bool>,
-        wait: sync::Condvar,
-    }
-
-    #[cfg(not(target_arch = "wasm32"))]
-    impl Event {
-        const fn new() -> Self {
-            Self {
-                set: sync::Mutex::new(false),
-                wait: sync::Condvar::new(),
-            }
-        }
-
-        fn set(&self) {
-            *self.set.lock().unwrap() = true;
-            self.wait.notify_all();
-        }
-
-        fn wait(&self) {
-            drop(
-                self.wait
-                    .wait_while(self.set.lock().unwrap(), |s| !*s)
-                    .unwrap(),
-            );
-        }
-    }
-
-    #[test]
-    #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
-    fn test_clone_without_gil() {
-        use crate::{Py, PyAny};
-        use std::{sync::Arc, thread};
-
-        // Some events for synchronizing
-        static GIL_ACQUIRED: Event = Event::new();
-        static OBJECT_CLONED: Event = Event::new();
-        static REFCNT_CHECKED: Event = Event::new();
-
-        Python::with_gil(|py| {
-            let obj: Arc<Py<PyAny>> = Arc::new(get_object(py));
-            let thread_obj = Arc::clone(&obj);
-
-            let count = obj.get_refcnt(py);
-            println!(
-                "1: The object has been created and its reference count is {}",
-                count
-            );
-
-            let handle = thread::spawn(move || {
-                Python::with_gil(move |py| {
-                    println!("3. The GIL has been acquired on another thread.");
-                    GIL_ACQUIRED.set();
-
-                    // Wait while the main thread registers obj in POOL
-                    OBJECT_CLONED.wait();
-                    println!("5. Checking refcnt");
-                    assert_eq!(thread_obj.get_refcnt(py), count);
-
-                    REFCNT_CHECKED.set();
-                })
-            });
-
-            let cloned = py.allow_threads(|| {
-                println!("2. The GIL has been released.");
-
-                // Wait until the GIL has been acquired on the thread.
-                GIL_ACQUIRED.wait();
-
-                println!("4. The other thread is now hogging the GIL, we clone without it held");
-                // Cloning without GIL should not update reference count
-                let cloned = Py::clone(&*obj);
-                OBJECT_CLONED.set();
-                cloned
-            });
-
-            REFCNT_CHECKED.wait();
-
-            println!("6. The main thread has acquired the GIL again and processed the pool.");
-
-            // Total reference count should be one higher
-            assert_eq!(obj.get_refcnt(py), count + 1);
-
-            // Clone dropped
-            drop(cloned);
-            // Ensure refcount of the arc is 1
-            handle.join().unwrap();
-
-            // Overall count is now back to the original, and should be no pending change
-            assert_eq!(Arc::try_unwrap(obj).unwrap().get_refcnt(py), count);
-        });
-    }
-
-    #[test]
-    #[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
-    fn test_clone_in_other_thread() {
-        use crate::Py;
-        use std::{sync::Arc, thread};
-
-        // Some events for synchronizing
-        static OBJECT_CLONED: Event = Event::new();
-
-        let (obj, count, ptr) = Python::with_gil(|py| {
-            let obj = Arc::new(get_object(py));
-            let count = obj.get_refcnt(py);
-            let thread_obj = Arc::clone(&obj);
-
-            // Start a thread which does not have the GIL, and clone it
-            let t = thread::spawn(move || {
-                // Cloning without GIL should not update reference count
-                #[allow(clippy::redundant_clone)]
-                let _ = Py::clone(&*thread_obj);
-                OBJECT_CLONED.set();
-            });
-
-            OBJECT_CLONED.wait();
-            assert_eq!(count, obj.get_refcnt(py));
-
-            t.join().unwrap();
-            let ptr = NonNull::new(obj.as_ptr()).unwrap();
-
-            // The pointer should appear once in the incref pool, and once in the
-            // decref pool (for the clone being created and also dropped)
-            assert!(POOL.pointer_ops.lock().unwrap().0.contains(&ptr));
-            assert!(POOL.pointer_ops.lock().unwrap().1.contains(&ptr));
-
-            (obj, count, ptr)
-        });
-
-        Python::with_gil(|py| {
-            // Acquiring the gil clears the pool
-            assert!(!POOL.pointer_ops.lock().unwrap().0.contains(&ptr));
-            assert!(!POOL.pointer_ops.lock().unwrap().1.contains(&ptr));
-
-            // Overall count is still unchanged
-            assert_eq!(count, obj.get_refcnt(py));
-        });
-    }
-
     #[test]
+    #[cfg(not(pyo3_disable_reference_pool))]
     fn test_update_counts_does_not_deadlock() {
         // update_counts can run arbitrary Python code during Py_DECREF.
         // if the locking is implemented incorrectly, it will deadlock.

src/instance.rs

@@ -81,10 +81,11 @@ where
     /// struct Foo {/* fields omitted */}
     ///
     /// # fn main() -> PyResult<()> {
-    /// Python::with_gil(|py| -> PyResult<Py<Foo>> {
+    /// let foo: Py<Foo> = Python::with_gil(|py| -> PyResult<_> {
     ///     let foo: Bound<'_, Foo> = Bound::new(py, Foo {})?;
     ///     Ok(foo.into())
     /// })?;
+    /// # Python::with_gil(move |_py| drop(foo));
     /// # Ok(())
     /// # }
     /// ```
@@ -865,7 +866,9 @@ impl<T> IntoPy<PyObject> for Borrowed<'_, '_, T> {
 ///     // All of these are valid syntax
 ///     let second = Py::clone_ref(&first, py);
 ///     let third = first.clone_ref(py);
+///     #[cfg(feature = "py-clone")]
 ///     let fourth = Py::clone(&first);
+///     #[cfg(feature = "py-clone")]
 ///     let fifth = first.clone();
 ///
 ///     // Disposing of our original `Py<PyDict>` just decrements the reference count.
@@ -873,7 +876,9 @@ impl<T> IntoPy<PyObject> for Borrowed<'_, '_, T> {
 ///
 ///     // They all point to the same object
 ///     assert!(second.is(&third));
+///     #[cfg(feature = "py-clone")]
 ///     assert!(fourth.is(&fifth));
+///     #[cfg(feature = "py-clone")]
 ///     assert!(second.is(&fourth));
 /// });
 /// # }
@@ -935,10 +940,11 @@ where
     /// struct Foo {/* fields omitted */}
     ///
     /// # fn main() -> PyResult<()> {
-    /// Python::with_gil(|py| -> PyResult<Py<Foo>> {
+    /// let foo = Python::with_gil(|py| -> PyResult<_> {
     ///     let foo: Py<Foo> = Py::new(py, Foo {})?;
     ///     Ok(foo)
     /// })?;
+    /// # Python::with_gil(move |_py| drop(foo));
     /// # Ok(())
     /// # }
     /// ```
@@ -1244,6 +1250,7 @@ where
     /// });
     ///
     /// cell.get().value.fetch_add(1, Ordering::Relaxed);
+    /// # Python::with_gil(move |_py| drop(cell));
     /// ```
     #[inline]
     pub fn get(&self) -> &T
@@ -1804,9 +1811,12 @@ where
 }
 
 /// If the GIL is held this increments `self`'s reference count.
-/// Otherwise this registers the [`Py`]`<T>` instance to have its reference count
-/// incremented the next time PyO3 acquires the GIL.
+/// Otherwise, it will panic.
+///
+/// Only available if the `py-clone` feature is enabled.
+#[cfg(feature = "py-clone")]
 impl<T> Clone for Py<T> {
+    #[track_caller]
     fn clone(&self) -> Self {
         unsafe {
             gil::register_incref(self.0);
@@ -1815,8 +1825,16 @@ impl<T> Clone for Py<T> {
     }
 }
 
-/// Dropping a `Py` instance decrements the reference count on the object by 1.
+/// Dropping a `Py` instance decrements the reference count
+/// on the object by one if the GIL is held.
+///
+/// Otherwise and by default, this registers the underlying pointer to have its reference count
+/// decremented the next time PyO3 acquires the GIL.
+///
+/// However, if the `pyo3_disable_reference_pool` conditional compilation flag
+/// is enabled, it will abort the process.
 impl<T> Drop for Py<T> {
+    #[track_caller]
     fn drop(&mut self) {
         unsafe {
             gil::register_decref(self.0);
@@ -2039,7 +2057,9 @@ mod tests {
             Py::from(native)
         });
 
-        assert_eq!(Python::with_gil(|py| dict.get_refcnt(py)), 1);
+        Python::with_gil(move |py| {
+            assert_eq!(dict.get_refcnt(py), 1);
+        });
     }
 
     #[test]

src/marker.rs

@@ -1178,7 +1178,6 @@ impl<'unbound> Python<'unbound> {
 mod tests {
     use super::*;
     use crate::types::{IntoPyDict, PyList};
-    use std::sync::Arc;
 
     #[test]
     fn test_eval() {
@@ -1264,11 +1263,12 @@ mod tests {
         });
     }
 
+    #[cfg(not(pyo3_disable_reference_pool))]
     #[test]
     fn test_allow_threads_pass_stuff_in() {
         let list = Python::with_gil(|py| PyList::new_bound(py, vec!["foo", "bar"]).unbind());
         let mut v = vec![1, 2, 3];
-        let a = Arc::new(String::from("foo"));
+        let a = std::sync::Arc::new(String::from("foo"));
 
         Python::with_gil(|py| {
             py.allow_threads(|| {

src/pybacked.rs

@@ -13,7 +13,7 @@ use crate::{
 /// A wrapper around `str` where the storage is owned by a Python `bytes` or `str` object.
 ///
 /// This type gives access to the underlying data via a `Deref` implementation.
-#[derive(Clone)]
+#[cfg_attr(feature = "py-clone", derive(Clone))]
 pub struct PyBackedStr {
     #[allow(dead_code)] // only held so that the storage is not dropped
     storage: Py<PyAny>,
@@ -88,7 +88,7 @@ impl FromPyObject<'_> for PyBackedStr {
 /// A wrapper around `[u8]` where the storage is either owned by a Python `bytes` object, or a Rust `Box<[u8]>`.
 ///
 /// This type gives access to the underlying data via a `Deref` implementation.
-#[derive(Clone)]
+#[cfg_attr(feature = "py-clone", derive(Clone))]
 pub struct PyBackedBytes {
     #[allow(dead_code)] // only held so that the storage is not dropped
     storage: PyBackedBytesStorage,
@@ -96,7 +96,7 @@ pub struct PyBackedBytes {
 }
 
 #[allow(dead_code)]
-#[derive(Clone)]
+#[cfg_attr(feature = "py-clone", derive(Clone))]
 enum PyBackedBytesStorage {
     Python(Py<PyBytes>),
     Rust(Arc<[u8]>),
@@ -336,6 +336,7 @@ mod test {
         is_sync::<PyBackedBytes>();
     }
 
+    #[cfg(feature = "py-clone")]
     #[test]
     fn test_backed_str_clone() {
         Python::with_gil(|py| {
@@ -398,6 +399,7 @@ mod test {
         })
     }
 
+    #[cfg(feature = "py-clone")]
     #[test]
     fn test_backed_bytes_from_bytes_clone() {
         Python::with_gil(|py| {
@@ -410,6 +412,7 @@ mod test {
         });
     }
 
+    #[cfg(feature = "py-clone")]
     #[test]
     fn test_backed_bytes_from_bytearray_clone() {
         Python::with_gil(|py| {

src/tests/hygiene/pyclass.rs

@@ -25,7 +25,7 @@ pub struct Bar {
     a: u8,
     #[pyo3(get, set)]
     b: Foo,
-    #[pyo3(get, set)]
+    #[pyo3(set)]
     c: ::std::option::Option<crate::Py<Foo2>>,
 }
 

src/types/capsule.rs

@@ -489,7 +489,7 @@ mod tests {
             cap.into()
         });
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             let f = unsafe { cap.bind(py).reference::<fn(u32) -> u32>() };
             assert_eq!(f(123), 123);
         });
@@ -553,7 +553,7 @@ mod tests {
             cap.into()
         });
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             let ctx: &Vec<u8> = unsafe { cap.bind(py).reference() };
             assert_eq!(ctx, &[1, 2, 3, 4]);
         })
@@ -572,7 +572,7 @@ mod tests {
             cap.into()
         });
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             let ctx_ptr: *mut c_void = cap.bind(py).context().unwrap();
             let ctx = unsafe { *Box::from_raw(ctx_ptr.cast::<&Vec<u8>>()) };
             assert_eq!(ctx, &vec![1_u8, 2, 3, 4]);
@@ -589,7 +589,7 @@ mod tests {
             context.send(true).unwrap();
         }
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             let name = CString::new("foo").unwrap();
             let cap = PyCapsule::new_bound_with_destructor(py, 0, Some(name), destructor).unwrap();
             cap.set_context(Box::into_raw(Box::new(tx)).cast()).unwrap();

src/types/iterator.rs

@@ -195,7 +195,7 @@ mod tests {
             );
         });
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             assert_eq!(count, obj.get_refcnt(py));
         });
     }

src/types/sequence.rs

@@ -823,7 +823,7 @@ mod tests {
             assert!(seq.get_item(1).unwrap().as_ptr() == obj.as_ptr());
         });
 
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             assert_eq!(1, obj.get_refcnt(py));
         });
     }

tests/test_buffer_protocol.rs

@@ -122,7 +122,7 @@ fn test_releasebuffer_unraisable_error() {
         let capture = UnraisableCapture::install(py);
 
         let instance = Py::new(py, ReleaseBufferError {}).unwrap();
-        let env = [("ob", instance.clone())].into_py_dict_bound(py);
+        let env = [("ob", instance.clone_ref(py))].into_py_dict_bound(py);
 
         assert!(capture.borrow(py).capture.is_none());
 

tests/test_bytes.rs

@@ -48,4 +48,6 @@ fn test_py_as_bytes() {
     let data = Python::with_gil(|py| pyobj.as_bytes(py));
 
     assert_eq!(data, b"abc");
+
+    Python::with_gil(move |_py| drop(pyobj));
 }

tests/test_class_basics.rs

@@ -172,6 +172,7 @@ fn empty_class_in_module() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[pyclass]
 struct ClassWithObjectField {
     // It used to be that PyObject was not supported with (get, set)
@@ -180,6 +181,7 @@ struct ClassWithObjectField {
     value: PyObject,
 }
 
+#[cfg(feature = "py-clone")]
 #[pymethods]
 impl ClassWithObjectField {
     #[new]
@@ -188,6 +190,7 @@ impl ClassWithObjectField {
     }
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn class_with_object_field() {
     Python::with_gil(|py| {
@@ -229,7 +232,7 @@ impl UnsendableChild {
 }
 
 fn test_unsendable<T: PyClass + 'static>() -> PyResult<()> {
-    let obj = Python::with_gil(|py| -> PyResult<_> {
+    let (keep_obj_here, obj) = Python::with_gil(|py| -> PyResult<_> {
         let obj: Py<T> = PyType::new_bound::<T>(py).call1((5,))?.extract()?;
 
         // Accessing the value inside this thread should not panic
@@ -241,14 +244,13 @@ fn test_unsendable<T: PyClass + 'static>() -> PyResult<()> {
             .is_err();
 
         assert!(!caught_panic);
-        Ok(obj)
-    })?;
 
-    let keep_obj_here = obj.clone();
+        Ok((obj.clone_ref(py), obj))
+    })?;
 
     let caught_panic = std::thread::spawn(move || {
         // This access must panic
-        Python::with_gil(|py| {
+        Python::with_gil(move |py| {
             obj.borrow(py);
         });
     })
@@ -549,6 +551,8 @@ fn access_frozen_class_without_gil() {
     });
 
     assert_eq!(py_counter.get().value.load(Ordering::Relaxed), 1);
+
+    Python::with_gil(move |_py| drop(py_counter));
 }
 
 #[test]

tests/test_class_conversion.rs

@@ -54,12 +54,14 @@ impl SubClass {
     }
 }
 
+#[cfg(feature = "py-clone")]
 #[pyclass]
 struct PolymorphicContainer {
     #[pyo3(get, set)]
     inner: Py<BaseClass>,
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_polymorphic_container_stores_base_class() {
     Python::with_gil(|py| {
@@ -76,6 +78,7 @@ fn test_polymorphic_container_stores_base_class() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_polymorphic_container_stores_sub_class() {
     Python::with_gil(|py| {
@@ -103,6 +106,7 @@ fn test_polymorphic_container_stores_sub_class() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_polymorphic_container_does_not_accept_other_types() {
     Python::with_gil(|py| {

tests/test_gc.rs

@@ -445,6 +445,7 @@ impl DropDuringTraversal {
     }
 }
 
+#[cfg(not(pyo3_disable_reference_pool))]
 #[test]
 fn drop_during_traversal_with_gil() {
     let drop_called = Arc::new(AtomicBool::new(false));
@@ -476,6 +477,7 @@ fn drop_during_traversal_with_gil() {
     assert!(drop_called.load(Ordering::Relaxed));
 }
 
+#[cfg(not(pyo3_disable_reference_pool))]
 #[test]
 fn drop_during_traversal_without_gil() {
     let drop_called = Arc::new(AtomicBool::new(false));

tests/test_methods.rs

@@ -874,6 +874,7 @@ fn test_from_sequence() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[pyclass]
 struct r#RawIdents {
     #[pyo3(get, set)]
@@ -882,6 +883,7 @@ struct r#RawIdents {
     r#subsubtype: PyObject,
 }
 
+#[cfg(feature = "py-clone")]
 #[pymethods]
 impl r#RawIdents {
     #[new]
@@ -946,6 +948,7 @@ impl r#RawIdents {
     }
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_raw_idents() {
     Python::with_gil(|py| {

tests/test_no_imports.rs

@@ -143,12 +143,14 @@ fn test_basic() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[pyo3::pyclass]
 struct NewClassMethod {
     #[pyo3(get)]
     cls: pyo3::PyObject,
 }
 
+#[cfg(feature = "py-clone")]
 #[pyo3::pymethods]
 impl NewClassMethod {
     #[new]
@@ -160,6 +162,7 @@ impl NewClassMethod {
     }
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_new_class_method() {
     pyo3::Python::with_gil(|py| {

tests/test_sequence.rs

@@ -248,12 +248,14 @@ fn test_inplace_repeat() {
 
 // Check that #[pyo3(get, set)] works correctly for Vec<PyObject>
 
+#[cfg(feature = "py-clone")]
 #[pyclass]
 struct GenericList {
     #[pyo3(get, set)]
     items: Vec<PyObject>,
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_generic_list_get() {
     Python::with_gil(|py| {
@@ -266,6 +268,7 @@ fn test_generic_list_get() {
     });
 }
 
+#[cfg(feature = "py-clone")]
 #[test]
 fn test_generic_list_set() {
     Python::with_gil(|py| {

tests/test_serde.rs

@@ -11,7 +11,7 @@ mod test_serde {
     }
 
     #[pyclass]
-    #[derive(Debug, Clone, Serialize, Deserialize)]
+    #[derive(Debug, Serialize, Deserialize)]
     struct User {
         username: String,
         group: Option<Py<Group>>,
@@ -27,7 +27,8 @@ mod test_serde {
         };
         let friend2 = User {
             username: "friend 2".into(),
-            ..friend1.clone()
+            group: None,
+            friends: vec![],
         };
 
         let user = Python::with_gil(|py| {