Skip to content

Commit

Permalink
gil: add unsafe variation for obtaining GIL without checks
Browse files Browse the repository at this point in the history
GILGuard::acquire() cannot be called during multi-phase Python
interpreter initialization because it calls Py_IsInitialized(),
which doesn't report the interpreter as initialized until all
phases of initialization have completed.

PyOxidizer uses the multi-phase initialization API and needs to
interact with pyo3's high-level APIs (not the FFI bindings) after
partial interpreter initialization, before the interpreter is fully
initialized. Attempts to use GILGuard::acquire() result in a panic
due to the aforementioned Py_IsInitialized() check failing.

This commit refactors the GILGuard logic into a function that
obtains the actual GILGuard and another function to perform
checks before calling the aforementioned functions.

A new unsafe `Python::with_gil_unchecked()` has been defined
to acquire the GIL via the unchecked code path so we may obtain
a `Python` during multi-phase initialization (and possibly other
scenarios).
  • Loading branch information
indygreg committed Aug 8, 2021
1 parent 3de5591 commit 3a6740a
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Add `indexmap` feature to add `ToPyObject`, `IntoPy` and `FromPyObject` implementations for `indexmap::IndexMap`. [#1728](https://github.com/PyO3/pyo3/pull/1728)
- Add `pyo3_build_config::add_extension_module_link_args()` to use in build scripts to set linker arguments (for macOS). [#1755](https://github.com/PyO3/pyo3/pull/1755)
- Add `Python::with_gil_unchecked()` unsafe variation of `Python::with_gil()` to allow obtaining a `Python` in scenarios where `Python::with_gil()` would fail.

### Changed

Expand Down
21 changes: 21 additions & 0 deletions src/gil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ impl GILGuard {
}
}

Self::acquire_unchecked()
}

/// Acquires the `GILGuard` without performing any state checking.
///
/// 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

// If there's already a GILPool, we should not create another or this could lead to
Expand Down Expand Up @@ -475,6 +484,18 @@ pub(crate) fn ensure_gil() -> EnsureGIL {
}
}

/// 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>);
Expand Down
25 changes: 25 additions & 0 deletions src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,31 @@ impl Python<'_> {
{
f(unsafe { gil::ensure_gil().python() })
}

/// Like [Python::with_gil] except Python interpreter state checking is skipped.
///
/// Normally when the GIL is acquired, we check that the Python interpreter is an
/// appropriate state (e.g. it is fully initialized). This function skips those
/// checks.
///
/// # Safety
///
/// If [Python::with_gil] would succeed, it is safe to call this function.
///
/// In most cases, you should use [Python::with_gil].
///
/// A justified scenario for calling this function is during multi-phase interpreter
/// initialization when [Python::with_gil] would fail before `_Py_InitializeMain()`
/// is called because the interpreter is only partially initialized.
///
/// Behavior in other scenarios is not documented.
#[inline]
pub unsafe fn with_gil_unchecked<F, R>(f: F) -> R
where
F: for<'p> FnOnce(Python<'p>) -> R,
{
f(gil::ensure_gil_unchecked().python())
}
}

impl<'p> Python<'p> {
Expand Down

0 comments on commit 3a6740a

Please sign in to comment.