diff --git a/guide/src/migration.md b/guide/src/migration.md index 10b62002a02..9e8d29347fd 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -44,7 +44,7 @@ If you rely on `impl Clone for Py` 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 `pyo3_disable_reference_pool` conditional compilation flag which removes the infrastructure necessary to apply delayed reference count decrements implied by `impl Drop for Py`. 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 `pyo3_disable_reference_pool` conditional compilation flag which removes the infrastructure necessary to apply delayed reference count decrements implied by `impl Drop for Py`. 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 leak the object if called without the GIL being held instead. ## from 0.20.* to 0.21 diff --git a/guide/src/performance.md b/guide/src/performance.md index a21c4ef5832..cec5fa30bbd 100644 --- a/guide/src/performance.md +++ b/guide/src/performance.md @@ -101,7 +101,7 @@ impl PartialEq for FooBound<'_> { PyO3 uses global mutable state to keep track of deferred reference count updates implied by `impl Drop for Py` 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` 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 `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` 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 leak the object when `Drop` is called without the GIL being held. For testing purposes, the `pyo3_abort_on_drop_without_reference_pool` can be set additional to make `Drop` abort without the GIL being held and thereby surface any unexpected resource leaks. 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` (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 diff --git a/pyo3-build-config/src/lib.rs b/pyo3-build-config/src/lib.rs index 09cb7331730..5283d2b14d0 100644 --- a/pyo3-build-config/src/lib.rs +++ b/pyo3-build-config/src/lib.rs @@ -166,6 +166,7 @@ pub fn print_expected_cfgs() { 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_abort_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) diff --git a/src/gil.rs b/src/gil.rs index d11060c6179..2eb18b35c1b 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -1,7 +1,7 @@ //! Interaction with Python's global interpreter lock use crate::impl_::not_send::{NotSend, NOT_SEND}; -#[cfg(pyo3_disable_reference_pool)] +#[cfg(all(pyo3_disable_reference_pool, pyo3_abort_on_drop_without_reference_pool))] use crate::impl_::panic::PanicTrap; use crate::{ffi, Python}; use std::cell::Cell; @@ -455,7 +455,7 @@ pub unsafe fn register_decref(obj: NonNull) { } else { #[cfg(not(pyo3_disable_reference_pool))] POOL.register_decref(obj); - #[cfg(pyo3_disable_reference_pool)] + #[cfg(all(pyo3_disable_reference_pool, pyo3_abort_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."); diff --git a/src/instance.rs b/src/instance.rs index 7835b9c79b6..2085e2fe137 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1832,7 +1832,7 @@ impl Clone for Py { /// 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. +/// is enabled, it will leak the object instead. impl Drop for Py { #[track_caller] fn drop(&mut self) {