From 3a6740a459032d7b5fdacce68520b10f7b6d3c8a Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Sun, 8 Aug 2021 12:08:43 -0700 Subject: [PATCH] gil: add unsafe variation for obtaining GIL without checks 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). --- CHANGELOG.md | 1 + src/gil.rs | 21 +++++++++++++++++++++ src/python.rs | 25 +++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 705e859f473..53168fbaeee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/gil.rs b/src/gil.rs index 8d1832e7ad7..d75da5db3b2 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -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 @@ -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); diff --git a/src/python.rs b/src/python.rs index 03fd9995773..b6757820693 100644 --- a/src/python.rs +++ b/src/python.rs @@ -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: F) -> R + where + F: for<'p> FnOnce(Python<'p>) -> R, + { + f(gil::ensure_gil_unchecked().python()) + } } impl<'p> Python<'p> {