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 21, 2023
1 parent bd07ecc commit 4905332
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 18 deletions.
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).
23 changes: 23 additions & 0 deletions src/gil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,29 @@ impl EnsureGIL {
}
}

/// Value with concurrent access protected by the GIL.
///
/// This is similar to [`Mutex<T>`][std::sync::Mutex] but the lock used is Python's global interpreter lock (GIL).
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> Send for GILProtected<T> where T: Send {}

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

#[cfg(test)]
mod tests {
use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL};
Expand Down
26 changes: 12 additions & 14 deletions src/impl_/pyclass/lazy_type_object.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
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, gil::GILProtected, once_cell::GILOnceCell,
pyclass::create_type_object, types::PyType, AsPyPointer, IntoPyPointer, Py, PyClass, PyErr,
PyMethodDefType, PyObject, PyResult, Python,
};

use super::PyClassItemsIter;
Expand All @@ -24,7 +23,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 +33,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 +108,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 +118,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 +171,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).borrow_mut() = Vec::new();
result
});

Expand Down Expand Up @@ -200,9 +201,6 @@ fn initialize_tp_dict(
Ok(())
}

// This is necessary for making static `LazyTypeObject`s
unsafe impl<T> Sync for LazyTypeObject<T> {}

#[cold]
fn wrap_in_runtime_error(py: Python<'_>, err: PyErr, message: String) -> PyErr {
let runtime_err = PyRuntimeError::new_err(message);
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ pub use crate::conversion::{
pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult};
#[cfg(not(PyPy))]
pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter};
pub use crate::gil::{GILGuard, GILPool};
pub use crate::gil::{GILGuard, GILPool, GILProtected};
pub use crate::instance::{Py, PyNativeType, PyObject};
pub use crate::marker::Python;
pub use crate::pycell::{PyCell, PyRef, PyRefMut};
Expand Down
7 changes: 4 additions & 3 deletions src/pyclass/create_type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,10 @@ impl PyTypeBuilder {
unsafe { self.push_slot(ffi::Py_tp_new, no_constructor_defined as *mut c_void) }
}

if !self.has_dealloc {
panic!("PyTypeBuilder requires you to specify slot ffi::Py_tp_dealloc");
}
assert!(
self.has_dealloc,
"PyTypeBuilder requires you to specify slot ffi::Py_tp_dealloc"
);

if self.has_clear && !self.has_traverse {
return Err(PyTypeError::new_err(format!(
Expand Down

0 comments on commit 4905332

Please sign in to comment.