Skip to content

Commit

Permalink
Add GILProtected mutex replacement and use it for LazyTypeObjectInner.
Browse files Browse the repository at this point in the history
  • Loading branch information
adamreichold committed Feb 22, 2023
1 parent 12ce8d2 commit f9a21a9
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 21 deletions.
2 changes: 1 addition & 1 deletion guide/src/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

PyO3 provides a struct [`GILOnceCell`] which works equivalently to `OnceCell` but relies solely on the Python GIL for thread safety. This means it can be used in place of `lazy_static` or `once_cell` where you are experiencing the deadlock described above. See the documentation for [`GILOnceCell`] for an example how to use it.

[`GILOnceCell`]: {{#PYO3_DOCS_URL}}/pyo3/once_cell/struct.GILOnceCell.html
[`GILOnceCell`]: {{#PYO3_DOCS_URL}}/pyo3/sync/struct.GILOnceCell.html

## I can't run `cargo test`; or I can't build in a Cargo workspace: I'm having linker issues like "Symbol not found" or "Undefined reference to _PyExc_SystemError"!

Expand Down
1 change: 1 addition & 0 deletions newsfragments/2975.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `GILProtected<T>` to mediate concurrent access to a value using Python's global interpreter lock (GIL).
4 changes: 2 additions & 2 deletions src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ macro_rules! import_exception {

impl $name {
fn type_object_raw(py: $crate::Python<'_>) -> *mut $crate::ffi::PyTypeObject {
use $crate::once_cell::GILOnceCell;
use $crate::sync::GILOnceCell;
use $crate::AsPyPointer;
static TYPE_OBJECT: GILOnceCell<$crate::Py<$crate::types::PyType>> =
GILOnceCell::new();
Expand Down Expand Up @@ -241,7 +241,7 @@ macro_rules! create_exception_type_object {

impl $name {
fn type_object_raw(py: $crate::Python<'_>) -> *mut $crate::ffi::PyTypeObject {
use $crate::once_cell::GILOnceCell;
use $crate::sync::GILOnceCell;
use $crate::AsPyPointer;
static TYPE_OBJECT: GILOnceCell<$crate::Py<$crate::types::PyType>> =
GILOnceCell::new();
Expand Down
2 changes: 1 addition & 1 deletion src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ unsafe fn bpo_35810_workaround(_py: Python<'_>, ty: *mut ffi::PyTypeObject) {
{
// Must check version at runtime for abi3 wheels - they could run against a higher version
// than the build config suggests.
use crate::once_cell::GILOnceCell;
use crate::sync::GILOnceCell;
static IS_PYTHON_3_8: GILOnceCell<bool> = GILOnceCell::new();

if *IS_PYTHON_3_8.get_or_init(_py, || _py.version_info() >= (3, 8)) {
Expand Down
26 changes: 15 additions & 11 deletions src/impl_/pyclass/lazy_type_object.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
use std::{
borrow::Cow,
cell::RefCell,
ffi::CStr,
marker::PhantomData,
thread::{self, ThreadId},
};

use parking_lot::{const_mutex, Mutex};

use crate::{
exceptions::PyRuntimeError, ffi, once_cell::GILOnceCell, pyclass::create_type_object,
types::PyType, AsPyPointer, IntoPyPointer, Py, PyClass, PyErr, PyMethodDefType, PyObject,
PyResult, Python,
exceptions::PyRuntimeError,
ffi,
pyclass::create_type_object,
sync::{GILOnceCell, GILProtected},
types::PyType,
AsPyPointer, IntoPyPointer, Py, PyClass, PyErr, PyMethodDefType, PyObject, PyResult, Python,
};

use super::PyClassItemsIter;
Expand All @@ -24,7 +26,7 @@ struct LazyTypeObjectInner {
value: GILOnceCell<Py<PyType>>,
// Threads which have begun initialization of the `tp_dict`. Used for
// reentrant initialization detection.
initializing_threads: Mutex<Vec<ThreadId>>,
initializing_threads: GILProtected<RefCell<Vec<ThreadId>>>,
tp_dict_filled: GILOnceCell<()>,
}

Expand All @@ -34,7 +36,7 @@ impl<T> LazyTypeObject<T> {
LazyTypeObject(
LazyTypeObjectInner {
value: GILOnceCell::new(),
initializing_threads: const_mutex(Vec::new()),
initializing_threads: GILProtected::new(RefCell::new(Vec::new())),
tp_dict_filled: GILOnceCell::new(),
},
PhantomData,
Expand Down Expand Up @@ -109,7 +111,7 @@ impl LazyTypeObjectInner {

let thread_id = thread::current().id();
{
let mut threads = self.initializing_threads.lock();
let mut threads = self.initializing_threads.with_gil(py).borrow_mut();
if threads.contains(&thread_id) {
// Reentrant call: just return the type object, even if the
// `tp_dict` is not filled yet.
Expand All @@ -119,18 +121,20 @@ impl LazyTypeObjectInner {
}

struct InitializationGuard<'a> {
initializing_threads: &'a Mutex<Vec<ThreadId>>,
initializing_threads: &'a GILProtected<RefCell<Vec<ThreadId>>>,
py: Python<'a>,
thread_id: ThreadId,
}
impl Drop for InitializationGuard<'_> {
fn drop(&mut self) {
let mut threads = self.initializing_threads.lock();
let mut threads = self.initializing_threads.with_gil(self.py).borrow_mut();
threads.retain(|id| *id != self.thread_id);
}
}

let guard = InitializationGuard {
initializing_threads: &self.initializing_threads,
py,
thread_id,
};

Expand Down Expand Up @@ -170,7 +174,7 @@ impl LazyTypeObjectInner {
// Initialization successfully complete, can clear the thread list.
// (No further calls to get_or_init() will try to init, on any thread.)
std::mem::forget(guard);
*self.initializing_threads.lock() = Vec::new();
self.initializing_threads.with_gil(py).replace(Vec::new());
result
});

Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ mod instance;
pub mod marker;
pub mod marshal;
#[macro_use]
pub mod once_cell;
pub mod sync;
pub mod panic;
pub mod prelude;
pub mod pycell;
Expand Down
29 changes: 26 additions & 3 deletions src/once_cell.rs → src/sync.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,30 @@
//! A write-once cell mediated by the Python GIL.
//! Synchronization mechanisms based on the Python GIL.
use crate::{types::PyString, Py, Python};
use std::cell::UnsafeCell;

/// Value with concurrent access protected by the GIL.
///
/// This is a synchronization primitive based on Python's global interpreter lock (GIL).
/// It ensures that only one thread at a time can access the inner value via shared references.
/// It can be combined with interior mutability to obtain mutable references.
pub struct GILProtected<T> {
value: T,
}

impl<T> GILProtected<T> {
/// Place the given value under the protection of the GIL.
pub const fn new(value: T) -> Self {
Self { value }
}

/// Gain access to the inner value by giving proof of having acquired the GIL.
pub fn with_gil<'py>(&'py self, _py: Python<'py>) -> &'py T {
&self.value
}
}

unsafe impl<T> Sync for GILProtected<T> where T: Send {}

/// A write-once cell similar to [`once_cell::OnceCell`](https://docs.rs/once_cell/latest/once_cell/).
///
/// Unlike `once_cell::sync` which blocks threads to achieve thread safety, this implementation
Expand All @@ -25,7 +48,7 @@ use std::cell::UnsafeCell;
/// between threads:
///
/// ```
/// use pyo3::once_cell::GILOnceCell;
/// use pyo3::sync::GILOnceCell;
/// use pyo3::prelude::*;
/// use pyo3::types::PyList;
///
Expand Down Expand Up @@ -170,7 +193,7 @@ impl<T> GILOnceCell<T> {
#[macro_export]
macro_rules! intern {
($py: expr, $text: expr) => {{
static INTERNED: $crate::once_cell::Interned = $crate::once_cell::Interned::new($text);
static INTERNED: $crate::sync::Interned = $crate::sync::Interned::new($text);
INTERNED.get($py)
}};
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/mapping.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2017-present PyO3 Project and Contributors

use crate::err::{PyDowncastError, PyErr, PyResult};
use crate::once_cell::GILOnceCell;
use crate::sync::GILOnceCell;
use crate::type_object::PyTypeInfo;
use crate::types::{PyAny, PyDict, PySequence, PyType};
use crate::{ffi, AsPyPointer, IntoPyPointer, Py, PyNativeType, PyTryFrom, Python, ToPyObject};
Expand Down
2 changes: 1 addition & 1 deletion src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::exceptions::PyTypeError;
#[cfg(feature = "experimental-inspect")]
use crate::inspect::types::TypeInfo;
use crate::internal_tricks::get_ssize_index;
use crate::once_cell::GILOnceCell;
use crate::sync::GILOnceCell;
use crate::type_object::PyTypeInfo;
use crate::types::{PyAny, PyList, PyString, PyTuple, PyType};
use crate::{ffi, PyNativeType, ToPyObject};
Expand Down

0 comments on commit f9a21a9

Please sign in to comment.