Skip to content

Commit

Permalink
feature gate deprecated APIs for GILPool (#4181)
Browse files Browse the repository at this point in the history
  • Loading branch information
alex authored May 17, 2024
1 parent fff053b commit fe79f54
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 35 deletions.
77 changes: 45 additions & 32 deletions src/gil.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Interaction with Python's global interpreter lock
#[cfg(feature = "gil-refs")]
use crate::impl_::not_send::{NotSend, NOT_SEND};
#[cfg(pyo3_disable_reference_pool)]
use crate::impl_::panic::PanicTrap;
Expand Down Expand Up @@ -127,19 +128,16 @@ where

ffi::Py_InitializeEx(0);

// Safety: the GIL is already held because of the Py_IntializeEx call.
#[allow(deprecated)] // TODO: remove this with the GIL Refs feature in 0.22
let pool = GILPool::new();

// Import the threading module - this ensures that it will associate this thread as the "main"
// thread, which is important to avoid an `AssertionError` at finalization.
pool.python().import_bound("threading").unwrap();
let result = {
let guard = GILGuard::assume();
let py = guard.python();
// Import the threading module - this ensures that it will associate this thread as the "main"
// thread, which is important to avoid an `AssertionError` at finalization.
py.import_bound("threading").unwrap();

// Execute the closure.
let result = f(pool.python());

// Drop the pool before finalizing.
drop(pool);
// Execute the closure.
f(py)
};

// Finalize the Python interpreter.
ffi::Py_Finalize();
Expand All @@ -154,7 +152,8 @@ pub(crate) enum GILGuard {
/// Indicates that we actually acquired the GIL when this GILGuard was acquired
Ensured {
gstate: ffi::PyGILState_STATE,
#[allow(deprecated)] // TODO: remove this with the gil-refs feature in 0.22
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
pool: mem::ManuallyDrop<GILPool>,
},
}
Expand Down Expand Up @@ -221,12 +220,23 @@ impl GILGuard {
}

let gstate = unsafe { ffi::PyGILState_Ensure() }; // acquire GIL
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };
{
let pool = unsafe { mem::ManuallyDrop::new(GILPool::new()) };
increment_gil_count();
GILGuard::Ensured { gstate, pool }
}

increment_gil_count();
#[cfg(not(feature = "gil-refs"))]
{
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(unsafe { Python::assume_gil_acquired() });

GILGuard::Ensured { gstate, pool }
GILGuard::Ensured { gstate }
}
}
/// Acquires the `GILGuard` while assuming that the GIL is already held.
pub(crate) unsafe fn assume() -> Self {
Expand All @@ -246,12 +256,17 @@ impl Drop for GILGuard {
fn drop(&mut self) {
match self {
GILGuard::Assumed => {}
#[cfg(feature = "gil-refs")]
GILGuard::Ensured { gstate, pool } => unsafe {
// Drop the objects in the pool before attempting to release the thread state
mem::ManuallyDrop::drop(pool);

ffi::PyGILState_Release(*gstate);
},
#[cfg(not(feature = "gil-refs"))]
GILGuard::Ensured { gstate } => unsafe {
ffi::PyGILState_Release(*gstate);
},
}
decrement_gil_count();
}
Expand Down Expand Up @@ -368,12 +383,10 @@ impl Drop for LockGIL {
///
/// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "`GILPool` has no function if PyO3's deprecated GIL Refs API is not used"
)
#[cfg(feature = "gil-refs")]
#[deprecated(
since = "0.21.0",
note = "`GILPool` has no function if PyO3's deprecated GIL Refs API is not used"
)]
pub struct GILPool {
/// Initial length of owned objects and anys.
Expand All @@ -382,6 +395,7 @@ pub struct GILPool {
_not_send: NotSend,
}

#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
impl GILPool {
/// Creates a new [`GILPool`]. This function should only ever be called with the GIL held.
Expand Down Expand Up @@ -419,6 +433,7 @@ impl GILPool {
}
}

#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
impl Drop for GILPool {
fn drop(&mut self) {
Expand Down Expand Up @@ -534,19 +549,15 @@ fn decrement_gil_count() {

#[cfg(test)]
mod tests {
#[allow(deprecated)]
use super::GILPool;
#[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 super::{gil_is_acquired, POOL};
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
use super::{GILPool, GIL_COUNT, OWNED_OBJECTS};
use crate::types::any::PyAnyMethods;
use crate::{PyObject, Python};
#[cfg(feature = "gil-refs")]
use {super::OWNED_OBJECTS, crate::gil};

#[cfg(not(pyo3_disable_reference_pool))]
use crate::{ffi, gil};
use crate::{PyObject, Python};
use std::ptr::NonNull;

fn get_object(py: Python<'_>) -> PyObject {
Expand Down Expand Up @@ -691,6 +702,7 @@ mod tests {
}

#[test]
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
fn test_gil_counts() {
// Check with_gil and GILPool both increase counts correctly
Expand Down Expand Up @@ -782,6 +794,7 @@ mod tests {

#[test]
#[cfg(not(pyo3_disable_reference_pool))]
#[cfg(feature = "gil-refs")]
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.
Expand Down
1 change: 1 addition & 0 deletions src/impl_/not_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ use crate::Python;
/// Workaround for lack of !Send on stable (<https://github.com/rust-lang/rust/issues/68318>).
pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);

#[cfg(feature = "gil-refs")]
pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ pub use crate::conversion::{FromPyPointer, PyTryFrom, PyTryInto};
#[cfg(feature = "gil-refs")]
pub use crate::err::PyDowncastError;
pub use crate::err::{DowncastError, DowncastIntoError, PyErr, PyErrArguments, PyResult, ToPyErr};
#[cfg(feature = "gil-refs")]
#[allow(deprecated)]
pub use crate::gil::GILPool;
#[cfg(not(any(PyPy, GraalPy)))]
Expand Down
4 changes: 1 addition & 3 deletions src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,10 @@ use crate::types::{
PyAny, PyDict, PyEllipsis, PyModule, PyNone, PyNotImplemented, PyString, PyType,
};
use crate::version::PythonVersionInfo;
#[cfg(feature = "gil-refs")]
use crate::PyNativeType;
use crate::{ffi, Bound, IntoPy, Py, PyObject, PyTypeInfo};
#[allow(deprecated)]
#[cfg(feature = "gil-refs")]
use crate::{gil::GILPool, FromPyPointer};
use crate::{gil::GILPool, FromPyPointer, PyNativeType};
use std::ffi::{CStr, CString};
use std::marker::PhantomData;
use std::os::raw::c_int;
Expand Down

0 comments on commit fe79f54

Please sign in to comment.