Replace disable-reference-pool feature by disable_pyo3_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.

Cargo.toml

@@ -109,9 +109,6 @@ gil-refs = []
 # Enables `Clone`ing references to Python objects `Py<T>` which panics if the GIL is not held.
 py-clone = []
 
-# Disables global reference pool which allows `Drop`ing references to Python objects `Py<T>` without the GIL being held.
-disable-reference-pool = []
-
 # Optimizes PyObject to Vec conversion and so on.
 nightly = []
 

guide/src/features.md

@@ -79,11 +79,9 @@ This feature and the APIs it enables is expected to be removed in a future PyO3
 
 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.
 
-### `disable-reference-pool`
+### `disable_pyo3_reference_pool`
 
-This is a performance-oriented feature 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.
-
-Since this feature does not really have additive semantics, it should only be enabled in leaf crates, i.e. in a crate producing a Python extension or embedding the Python interpreter.
+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`
 
@@ -201,3 +199,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

@@ -204,7 +204,7 @@ we are *not* holding the GIL?
 
 ```rust
 # #![allow(dead_code)]
-# #[cfg(not(feature = "disable-reference-pool"))] {
+# #[cfg(not(disable_pyo3_reference_pool))] {
 # use pyo3::prelude::*;
 # use pyo3::types::PyString;
 # fn main() -> PyResult<()> {
@@ -233,9 +233,9 @@ Python::with_gil(|py|
 
 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. If the `disable-reference-pool` feature is not enabled,
-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 `disable_pyo3_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

@@ -11,7 +11,7 @@ If you rely on `impl<T> Clone for Py<T>` to fulfil trait requirements imposed by
 
 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 `disable-reference-pool` feature 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.
+Related to this, we also added a `disable_pyo3_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

guide/src/performance.md

@@ -101,7 +101,7 @@ impl PartialEq<Foo> for FooBound<'_> {
 
 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 adding the `disable-reference-pool` feature. 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.
+This functionality can be avoided by setting the `disable_pyo3_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.
 
 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
 
@@ -138,3 +138,5 @@ Python::with_gil(move |py| {
     drop(numbers);
 });
 ```
+
+[conditional-compilation]: https://doc.rust-lang.org/reference/conditional-compilation.html

newsfragments/4095.added.md

@@ -1 +1 @@
-Add `disable-reference-pool` feature to avoid the overhead of the global reference pool at the cost of known limitations as explained in the performance section of the guide.
+Add `disable_pyo3_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.

src/gil.rs

@@ -1,11 +1,11 @@
 //! Interaction with Python's global interpreter lock
 
 use crate::impl_::not_send::{NotSend, NOT_SEND};
-#[cfg(feature = "disable-reference-pool")]
+#[cfg(disable_pyo3_reference_pool)]
 use crate::impl_::panic::PanicTrap;
 use crate::{ffi, Python};
 use parking_lot::Once;
-#[cfg(not(feature = "disable-reference-pool"))]
+#[cfg(not(disable_pyo3_reference_pool))]
 use parking_lot::{const_mutex, Mutex};
 use std::cell::Cell;
 #[cfg(debug_assertions)]
@@ -250,13 +250,13 @@ impl Drop for GILGuard {
 // Vector of PyObject
 type PyObjVec = Vec<NonNull<ffi::PyObject>>;
 
-#[cfg(not(feature = "disable-reference-pool"))]
+#[cfg(not(disable_pyo3_reference_pool))]
 /// Thread-safe storage for objects which were inc_ref / dec_ref while the GIL was not held.
 struct ReferencePool {
     pending_decrefs: Mutex<PyObjVec>,
 }
 
-#[cfg(not(feature = "disable-reference-pool"))]
+#[cfg(not(disable_pyo3_reference_pool))]
 impl ReferencePool {
     const fn new() -> Self {
         Self {
@@ -283,10 +283,10 @@ impl ReferencePool {
     }
 }
 
-#[cfg(not(feature = "disable-reference-pool"))]
+#[cfg(not(disable_pyo3_reference_pool))]
 unsafe impl Sync for ReferencePool {}
 
-#[cfg(not(feature = "disable-reference-pool"))]
+#[cfg(not(disable_pyo3_reference_pool))]
 static POOL: ReferencePool = ReferencePool::new();
 
 /// A guard which can be used to temporarily release the GIL and restore on `Drop`.
@@ -311,7 +311,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(feature = "disable-reference-pool"))]
+            #[cfg(not(disable_pyo3_reference_pool))]
             POOL.update_counts(Python::assume_gil_acquired());
         }
     }
@@ -386,7 +386,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(feature = "disable-reference-pool"))]
+        #[cfg(not(disable_pyo3_reference_pool))]
         POOL.update_counts(Python::assume_gil_acquired());
         GILPool {
             start: OWNED_OBJECTS
@@ -468,9 +468,9 @@ pub unsafe fn register_decref(obj: NonNull<ffi::PyObject>) {
     if gil_is_acquired() {
         ffi::Py_DECREF(obj.as_ptr())
     } else {
-        #[cfg(not(feature = "disable-reference-pool"))]
+        #[cfg(not(disable_pyo3_reference_pool))]
         POOL.register_decref(obj);
-        #[cfg(feature = "disable-reference-pool")]
+        #[cfg(disable_pyo3_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.");
@@ -527,7 +527,7 @@ fn decrement_gil_count() {
 mod tests {
     #[allow(deprecated)]
     use super::GILPool;
-    #[cfg(not(feature = "disable-reference-pool"))]
+    #[cfg(not(disable_pyo3_reference_pool))]
     use super::POOL;
     use super::{gil_is_acquired, GIL_COUNT, OWNED_OBJECTS};
     use crate::types::any::PyAnyMethods;
@@ -546,15 +546,15 @@ mod tests {
         len
     }
 
-    #[cfg(not(feature = "disable-reference-pool"))]
+    #[cfg(not(disable_pyo3_reference_pool))]
     fn pool_dec_refs_does_not_contain(obj: &PyObject) -> bool {
         !POOL
             .pending_decrefs
             .lock()
             .contains(&unsafe { NonNull::new_unchecked(obj.as_ptr()) })
     }
 
-    #[cfg(all(not(feature = "disable-reference-pool"), not(target_arch = "wasm32")))]
+    #[cfg(all(not(disable_pyo3_reference_pool), not(target_arch = "wasm32")))]
     fn pool_dec_refs_contains(obj: &PyObject) -> bool {
         POOL.pending_decrefs
             .lock()
@@ -630,20 +630,20 @@ mod tests {
             let reference = obj.clone_ref(py);
 
             assert_eq!(obj.get_refcnt(py), 2);
-            #[cfg(not(feature = "disable-reference-pool"))]
+            #[cfg(not(disable_pyo3_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);
-            #[cfg(not(feature = "disable-reference-pool"))]
+            #[cfg(not(disable_pyo3_reference_pool))]
             assert!(pool_dec_refs_does_not_contain(&obj));
         });
     }
 
     #[test]
-    #[cfg(all(not(feature = "disable-reference-pool"), not(target_arch = "wasm32")))] // We are building wasm Python with pthreads disabled
+    #[cfg(all(not(disable_pyo3_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);
@@ -761,7 +761,7 @@ mod tests {
     }
 
     #[test]
-    #[cfg(not(feature = "disable-reference-pool"))]
+    #[cfg(not(disable_pyo3_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

@@ -1835,8 +1835,8 @@ impl<T> Clone for Py<T> {
 /// 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 `disable-reference-pool` feature is enabled,
-/// it will abort the process.
+/// However, if the `disable_pyo3_reference_pool` conditional compilation flag
+/// is enabled, it will abort the process.
 impl<T> Drop for Py<T> {
     #[track_caller]
     fn drop(&mut self) {

src/marker.rs

@@ -1313,7 +1313,7 @@ mod tests {
         });
     }
 
-    #[cfg(not(feature = "disable-reference-pool"))]
+    #[cfg(not(disable_pyo3_reference_pool))]
     #[test]
     fn test_allow_threads_pass_stuff_in() {
         let list = Python::with_gil(|py| PyList::new_bound(py, vec!["foo", "bar"]).unbind());

tests/test_gc.rs

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