Cheaper and safe caching for py_capsule! and py_capsule_fn!

The `py_capsule!` and `py_capsule_fn!` macros generate `retrieve` functions
that cache their results for subsequent calls.

Prior to this commit, caching is done with a generated unsafe `static mut`
item whose access is made thread-safe by a `std::sync::Once` on the side.
However this synchronization is unnecessary since `retreive` takes a
`Python<'_>` parameter that indicates that the GIL is held.

This changes the cache to use safe `static` item instead, with `GILProtected`
for synchronization-free thread-safety and `OnceCell` for interior mutability.
As a bonus, this removes the need for cache-related `unsafe` code in
generated `retrieve` functions.

This adds a dependency to the `once_cell` crate, which can be removed when
`OnceCell` becomes stable in the standard library:
rust-lang/rust#74465

Alternatively `OnceCell` could be replaced with `UnsafeCell` plus some
`unsafe` code, effectively inlining the core of the logic of `OnceCell`.
`RefCell` might work too.

Fixes #263

Author: SimonSapin

Cargo.toml

@@ -35,6 +35,12 @@ appveyor = { repository = "dgrunwald/rust-cpython" }
 [dependencies]
 libc = "0.2"
 num-traits = "0.2"
+
+# TODO: use `std::lazy::OnceCell` instead and when it’s stable
+# and this crate requires a minimum Rust version that has it stable:
+# https://github.com/rust-lang/rust/issues/74465
+once_cell = "1.8"
+
 paste = "1"
 serde = { version = "1", features = ["derive"], optional = true }
 
@@ -43,7 +49,7 @@ rustversion = "1.0"
 serde_bytes = { version = "0.11" }
 serde_cbor = { version = "0.11" }
 
-# These features are both optional, but you must pick one to 
+# These features are both optional, but you must pick one to
 # indicate which python ffi you are trying to bind to.
 [dependencies.python27-sys]
 optional = true
@@ -83,7 +89,7 @@ py-link-mode-default = [ "python3-sys/link-mode-default" ]
 py-link-mode-unresolved-static = [ "python3-sys/link-mode-unresolved-static" ]
 
 # Optional features to support explicitly specifying python minor version.
-# If you don't care which minor version, just specify python3-sys as a 
+# If you don't care which minor version, just specify python3-sys as a
 # feature.
 python-3-9 = ["python3-sys/python-3-9"]
 python-3-8 = ["python3-sys/python-3-8"]

src/lib.rs

@@ -235,6 +235,7 @@ pub mod _detail {
         handle_callback, py_fn_impl, AbortOnDrop, PyObjectCallbackConverter,
         PythonObjectCallbackConverter,
     };
+    pub use once_cell::unsync::OnceCell;
     pub use paste;
 }
 

src/objects/capsule.rs

@@ -347,28 +347,27 @@ macro_rules! py_capsule {
     (from $($capsmod:ident).+ import $capsname:ident as $rustmod:ident for $ruststruct: ident ) => (
         mod $rustmod {
             use super::*;
-            use std::sync::Once;
+            use $crate::_detail::OnceCell;
+            use $crate::GILProtected;
             use $crate::PyClone;
 
-            static mut CAPS_DATA: Option<$crate::PyResult<&$ruststruct>> = None;
-
-            static INIT: Once = Once::new();
+            static CAPS_DATA: GILProtected<OnceCell<$crate::PyResult<&$ruststruct>>> =
+                GILProtected::new(OnceCell::new());
 
             pub type RawPyObject = $crate::_detail::ffi::PyObject;
 
             pub unsafe fn retrieve<'a>(py: $crate::Python) -> $crate::PyResult<&'a $ruststruct> {
-                INIT.call_once(|| {
+                let ref_to_result = CAPS_DATA.get(py).get_or_init(|| {
                     let caps_name =
                         std::ffi::CStr::from_bytes_with_nul_unchecked(
                             concat!($( stringify!($capsmod), "."),*,
                                     stringify!($capsname),
                                     "\0").as_bytes());
-                    CAPS_DATA = Some($crate::PyCapsule::import_data(py, caps_name));
+                    $crate::PyCapsule::import_data(py, caps_name)
                 });
-                match CAPS_DATA {
-                    Some(Ok(d)) => Ok(d),
-                    Some(Err(ref e)) => Err(e.clone_ref(py)),
-                    _ => panic!("Uninitialized"), // can't happen
+                match ref_to_result {
+                    &Ok(ref x) => Ok(x),
+                    &Err(ref e) => Err(e.clone_ref(py))
                 }
             }
         }
@@ -397,7 +396,7 @@ macro_rules! py_capsule {
 /// ```ignore
 /// mod $rustmod {
 ///     pub type CapsuleFn = unsafe extern "C" (args) -> ret_type ;
-///     pub unsafe fn retrieve<'a>(py: Python) -> PyResult<CapsuleFn) { ... }
+///     pub unsafe fn retrieve<'a>(py: Python) -> PyResult<CapsuleFn> { ... }
 /// }
 /// ```
 /// - a `RawPyObject` type suitable for signatures that involve Python C objects;
@@ -482,15 +481,15 @@ macro_rules! py_capsule_fn {
     (from $($capsmod:ident).+ import $capsname:ident as $rustmod:ident signature $( $sig: tt)* ) => (
         mod $rustmod {
             use super::*;
-            use std::sync::Once;
+            use $crate::_detail::OnceCell;
+            use $crate::GILProtected;
             use $crate::PyClone;
 
             pub type CapsuleFn = unsafe extern "C" fn $( $sig )*;
             pub type RawPyObject = $crate::_detail::ffi::PyObject;
 
-            static mut CAPS_FN: Option<$crate::PyResult<CapsuleFn>> = None;
-
-            static INIT: Once = Once::new();
+            static CAPS_FN: GILProtected<OnceCell<$crate::PyResult<CapsuleFn>>> =
+                GILProtected::new(OnceCell::new());
 
             fn import(py: $crate::Python) -> $crate::PyResult<CapsuleFn> {
                 unsafe {
@@ -504,12 +503,9 @@ macro_rules! py_capsule_fn {
             }
 
             pub fn retrieve(py: $crate::Python) -> $crate::PyResult<CapsuleFn> {
-                unsafe {
-                    INIT.call_once(|| { CAPS_FN = Some(import(py)) });
-                    match CAPS_FN.as_ref().unwrap() {
-                        &Ok(f) => Ok(f),
-                        &Err(ref e) => Err(e.clone_ref(py)),
-                    }
+                match CAPS_FN.get(py).get_or_init(|| import(py)) {
+                    &Ok(f) => Ok(f),
+                    &Err(ref e) => Err(e.clone_ref(py)),
                 }
             }
         }