Skip to content

Commit

Permalink
python: deprecate Python::acquire_gil
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidhewitt
davidhewitt committed Aug 12, 2021
1 parent 4c734ef commit a64ce43
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 127 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Added

- - Add `GILGuard::acquire_gil()` as an `unsafe` replacement for `Python::acquire_gil`. [#1788](https://github.com/PyO3/pyo3/pull/1788)

### Changed

- Change `PyErr::fetch()` to return `Option<PyErr>`. [#1717](https://github.com/PyO3/pyo3/pull/1717)
- `Python::with_gil` will now always acquire the GIL. [#1788](https://github.com/PyO3/pyo3/pull/1788)
- Deprecate `Python::acquire_gil` in favour of `Python::with_gil`. [#1788](https://github.com/PyO3/pyo3/pull/1788)

## [0.14.2] - 2021-08-09

Expand Down
135 changes: 36 additions & 99 deletions src/gil.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,31 @@ pub fn prepare_freethreaded_python() {
});
}

#[cfg(any(not(feature = "auto-initialize"), PyPy))]
pub(crate) fn assert_initialized_once() {
START.call_once_force(|_| unsafe {
// Use call_once_force because if there is a panic because the interpreter is
// not initialized, it's fine for the user to initialize the interpreter and
// retry.
assert_ne!(
ffi::Py_IsInitialized(),
0,
"The Python interpreter is not initalized and the `auto-initialize` \
feature is not enabled.\n\n\
Consider calling `pyo3::prepare_freethreaded_python()` before attempting \
to use Python APIs."
);
assert_ne!(
ffi::PyEval_ThreadsInitialized(),
0,
"Python threading is not initalized and the `auto-initialize` feature is \
not enabled.\n\n\
Consider calling `pyo3::prepare_freethreaded_python()` before attempting \
to use Python APIs."
);
});
}

/// Executes the provided closure with an embedded Python interpreter.
///
/// This function intializes the Python interpreter, executes the provided closure, and then
Expand Down Expand Up @@ -202,60 +227,23 @@ impl GILGuard {
unsafe { Python::assume_gil_acquired() }
}

/// PyO3 internal API for acquiring the GIL. The public API is Python::acquire_gil.
/// Acquires the global interpreter lock, which allows access to the Python runtime.
///
/// If PyO3 does not yet have a `GILPool` for tracking owned PyObject references, then this new
/// `GILGuard` will also contain a `GILPool`.
pub(crate) fn acquire() -> GILGuard {
// Maybe auto-initialize the GIL:
// - If auto-initialize feature set and supported, try to initalize the interpreter.
// - If the auto-initialize feature is set but unsupported, emit hard errors only when the
// extension-module feature is not activated - extension modules don't care about
// auto-initialize so this avoids breaking existing builds.
// - Otherwise, just check the GIL is initialized.
cfg_if::cfg_if! {
if #[cfg(all(feature = "auto-initialize", not(PyPy)))] {
prepare_freethreaded_python();
} else {
START.call_once_force(|_| unsafe {
// Use call_once_force because if there is a panic because the interpreter is
// not initialized, it's fine for the user to initialize the interpreter and
// retry.
assert_ne!(
ffi::Py_IsInitialized(),
0,
"The Python interpreter is not initalized and the `auto-initialize` \
feature is not enabled.\n\n\
Consider calling `pyo3::prepare_freethreaded_python()` before attempting \
to use Python APIs."
);
assert_ne!(
ffi::PyEval_ThreadsInitialized(),
0,
"Python threading is not initalized and the `auto-initialize` feature is \
not enabled.\n\n\
Consider calling `pyo3::prepare_freethreaded_python()` before attempting \
to use Python APIs."
);
});
}
}

Self::acquire_unchecked()
}

/// Acquires the `GILGuard` without performing any state checking.
/// # Safety
/// - The Python interpreter should have already been initialized.
/// - `GILGuard` objects must be dropped in the reverse order they are acquired. See note below.
///
/// This can be called in "unsafe" contexts where the normal interpreter state
/// checking performed by `GILGuard::acquire` may fail. This includes calling
/// as part of multi-phase interpreter initialization.
pub(crate) fn acquire_unchecked() -> GILGuard {
let gstate = unsafe { ffi::PyGILState_Ensure() }; // acquire GIL
/// **Note:** This return type from this function, `GILGuard`, is implemented as a RAII guard
/// around the C-API Python_EnsureGIL. This means that multiple `acquire_gil()` calls are
/// allowed, and will not deadlock. However, `GILGuard`s must be dropped in the reverse order
/// to acquisition. Failing to do this will corrupt the Python GIL state.
pub unsafe fn acquire_gil() -> GILGuard {
let gstate = ffi::PyGILState_Ensure(); // acquire GIL

// If there's already a GILPool, we should not create another or this could lead to
// incorrect dangling references in safe code (see #864).
let pool = if !gil_is_acquired() {
Some(unsafe { GILPool::new() })
Some(GILPool::new())
} else {
// As no GILPool was created, need to update the gil count manually.
increment_gil_count();
Expand All @@ -272,16 +260,6 @@ impl GILGuard {
/// The Drop implementation for `GILGuard` will release the GIL.
impl Drop for GILGuard {
fn drop(&mut self) {
// First up, try to detect if the order of destruction is correct.
let _ = GIL_COUNT.try_with(|c| {
if self.gstate == ffi::PyGILState_STATE::PyGILState_UNLOCKED && c.get() != 1 {
// XXX: this panic commits to leaking all objects in the pool as well as
// potentially meaning the GIL never releases. Perhaps should be an abort?
// Unfortunately abort UX is much worse than panic.
panic!("The first GILGuard acquired must be the last one dropped.");
}
});

// If this GILGuard doesn't own a pool, then need to decrease the count after dropping
// all objects from the pool.
let should_decrement = self.pool.is_none();
Expand Down Expand Up @@ -475,47 +453,6 @@ fn decrement_gil_count() {
});
}

/// Ensures the GIL is held, used in the implementation of `Python::with_gil`.
pub(crate) fn ensure_gil() -> EnsureGIL {
if gil_is_acquired() {
EnsureGIL(None)
} else {
EnsureGIL(Some(GILGuard::acquire()))
}
}

/// Ensures the GIL is held, without interpreter state checking.
///
/// This bypasses interpreter state checking that would normally be performed
/// before acquiring the GIL.
pub(crate) fn ensure_gil_unchecked() -> EnsureGIL {
if gil_is_acquired() {
EnsureGIL(None)
} else {
EnsureGIL(Some(GILGuard::acquire_unchecked()))
}
}

/// Struct used internally which avoids acquiring the GIL where it's not necessary.
#[allow(clippy::upper_case_acronyms)]
pub(crate) struct EnsureGIL(Option<GILGuard>);

impl EnsureGIL {
/// Get the GIL token.
///
/// # Safety
/// If `self.0` is `None`, then this calls [Python::assume_gil_acquired].
/// Thus this method could be used to get access to a GIL token while the GIL is not held.
/// Care should be taken to only use the returned Python in contexts where it is certain the
/// GIL continues to be held.
pub unsafe fn python(&self) -> Python {
match &self.0 {
Some(gil) => gil.python(),
None => Python::assume_gil_acquired(),
}
}
}

#[cfg(test)]
mod tests {
use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL};
Expand Down
88 changes: 60 additions & 28 deletions src/python.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ impl Python<'_> {
/// initialized, this function will initialize it. See
/// [prepare_freethreaded_python()](fn.prepare_freethreaded_python.html) for details.
///
/// This function will always touch a global lock, so will incur a small performance cost when
/// called in tight loops. Note that when implementing `#[pymethods]` or `#[pyfunction]` add a
/// function argument `py: Python` to receive access to the GIL context in which the function is
/// running without the performance overhead.
///
/// # Panics
/// - If the `auto-initialize` feature is not enabled and the Python interpreter is not
/// initialized.
Expand All @@ -158,7 +163,18 @@ impl Python<'_> {
where
F: for<'p> FnOnce(Python<'p>) -> R,
{
f(unsafe { gil::ensure_gil().python() })
// Maybe auto-initialize the GIL:
// - If auto-initialize feature set, try to initialize the interpreter.
// - Otherwise, just check the GIL is initialized.
cfg_if::cfg_if! {
if #[cfg(all(feature = "auto-initialize", not(PyPy)))] {
crate::gil::prepare_freethreaded_python();
} else {
crate::gil::assert_initialized_once();
}
}

unsafe { Python::with_gil_unchecked(f) }
}

/// Like [Python::with_gil] except Python interpreter state checking is skipped.
Expand All @@ -183,37 +199,12 @@ impl Python<'_> {
where
F: for<'p> FnOnce(Python<'p>) -> R,
{
f(gil::ensure_gil_unchecked().python())
let guard = GILGuard::acquire_gil();
f(guard.python())
}
}

impl<'p> Python<'p> {
/// Acquires the global interpreter lock, which allows access to the Python runtime.
///
/// If the `auto-initialize` feature is enabled and the Python runtime is not already
/// initialized, this function will initialize it. See
/// [prepare_freethreaded_python()](fn.prepare_freethreaded_python.html) for details.
///
/// Most users should not need to use this API directly, and should prefer one of two options:
/// 1. When implementing `#[pymethods]` or `#[pyfunction]` add a function argument
/// `py: Python` to receive access to the GIL context in which the function is running.
/// 2. Use [`Python::with_gil`](#method.with_gil) to run a closure with the GIL, acquiring
/// only if needed.
///
/// **Note:** This return type from this function, `GILGuard`, is implemented as a RAII guard
/// around the C-API Python_EnsureGIL. This means that multiple `acquire_gil()` calls are
/// allowed, and will not deadlock. However, `GILGuard`s must be dropped in the reverse order
/// to acquisition. If PyO3 detects this order is not maintained, it may be forced to begin
/// an irrecoverable panic.
///
/// # Panics
/// - If the `auto-initialize` feature is not enabled and the Python interpreter is not
/// initialized.
#[inline]
pub fn acquire_gil() -> GILGuard {
GILGuard::acquire()
}

/// Temporarily releases the `GIL`, thus allowing other Python threads to run.
///
/// # Examples
Expand Down Expand Up @@ -696,6 +687,47 @@ impl<'p> Python<'p> {
pub unsafe fn new_pool(self) -> GILPool {
GILPool::new()
}

/// Acquires the global interpreter lock, which allows access to the Python runtime.
///
/// If the `auto-initialize` feature is enabled and the Python runtime is not already
/// initialized, this function will initialize it. See
/// [prepare_freethreaded_python()](fn.prepare_freethreaded_python.html) for details.
///
/// Most users should not need to use this API directly, and should prefer one of two options:
/// 1. When implementing `#[pymethods]` or `#[pyfunction]` add a function argument
/// `py: Python` to receive access to the GIL context in which the function is running.
/// 2. Use [`Python::with_gil`](#method.with_gil) to run a closure with the GIL, acquiring
/// only if needed.
///
/// **Note:** This return type from this function, `GILGuard`, is implemented as a RAII guard
/// around the C-API Python_EnsureGIL. This means that multiple `acquire_gil()` calls are
/// allowed, and will not deadlock. However, `GILGuard`s must be dropped in the reverse order
/// to acquisition. If PyO3 detects this order is not maintained, it may be forced to begin
/// an irrecoverable panic.
///
/// # Panics
/// - If the `auto-initialize` feature is not enabled and the Python interpreter is not
/// initialized.
#[inline]
#[deprecated(
since = "0.15.0",
note = "prefer Python::with_gil or GILGuard::acquire_gil"
)]
pub fn acquire_gil() -> GILGuard {
// Maybe auto-initialize the GIL:
// - If auto-initialize feature set, try to initialize the interpreter.
// - Otherwise, just check the GIL is initialized.
cfg_if::cfg_if! {
if #[cfg(all(feature = "auto-initialize", not(PyPy)))] {
crate::gil::prepare_freethreaded_python();
} else {
crate::gil::assert_initialized_once();
}
}

unsafe { GILGuard::acquire_gil() }
}
}

#[cfg(test)]
Expand Down

0 comments on commit a64ce43

Please sign in to comment.