Skip to content

Commit

Permalink
If the global reference pool is disabled, leak by default and abort o…
Browse files Browse the repository at this point in the history
…nly if pyo3_abort_on_drop_without_reference_pool is set additionally
  • Loading branch information
adamreichold committed May 11, 2024
1 parent 4e74a30 commit 44276ee
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 5 deletions.
2 changes: 1 addition & 1 deletion guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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 `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.
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 leak the object if called without the GIL being held instead.
</details>

## from 0.20.* to 0.21
Expand Down
2 changes: 1 addition & 1 deletion guide/src/performance.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
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 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<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

Expand Down
1 change: 1 addition & 0 deletions pyo3-build-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/gil.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -455,7 +455,7 @@ pub unsafe fn register_decref(obj: NonNull<ffi::PyObject>) {
} 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.");
Expand Down
2 changes: 1 addition & 1 deletion src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1832,7 +1832,7 @@ impl<T> Clone for Py<T> {
/// 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<T> Drop for Py<T> {
#[track_caller]
fn drop(&mut self) {
Expand Down

0 comments on commit 44276ee

Please # to comment.