From 7ef8010f7e823d6be868b14e8ab54ab7790a7af9 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 15 Oct 2024 12:23:53 -0600 Subject: [PATCH 1/3] Make PyDateTime_IMPORT FFI wrapper thread-safe --- pyo3-ffi/src/datetime.rs | 41 ++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/pyo3-ffi/src/datetime.rs b/pyo3-ffi/src/datetime.rs index ddf200cc473..66862d72328 100644 --- a/pyo3-ffi/src/datetime.rs +++ b/pyo3-ffi/src/datetime.rs @@ -13,6 +13,7 @@ use crate::{PyObject, PyObject_TypeCheck, PyTypeObject, Py_TYPE}; use std::os::raw::c_char; use std::os::raw::c_int; use std::ptr; +use std::sync::Once; use std::{cell::UnsafeCell, ffi::CStr}; #[cfg(not(any(PyPy, GraalPy)))] use {crate::Py_hash_t, std::os::raw::c_uchar}; @@ -602,21 +603,28 @@ pub const PyDateTime_CAPSULE_NAME: &CStr = c_str!("datetime.datetime_CAPI"); /// `PyDateTime_IMPORT` is called #[inline] pub unsafe fn PyDateTimeAPI() -> *mut PyDateTime_CAPI { - *PyDateTimeAPI_impl.0.get() + *PyDateTimeAPI_impl.ptr.get() } /// Populates the `PyDateTimeAPI` object pub unsafe fn PyDateTime_IMPORT() { - // PyPy expects the C-API to be initialized via PyDateTime_Import, so trying to use - // `PyCapsule_Import` will behave unexpectedly in pypy. - #[cfg(PyPy)] - let py_datetime_c_api = PyDateTime_Import(); - - #[cfg(not(PyPy))] - let py_datetime_c_api = - PyCapsule_Import(PyDateTime_CAPSULE_NAME.as_ptr(), 1) as *mut PyDateTime_CAPI; - - *PyDateTimeAPI_impl.0.get() = py_datetime_c_api; + if !PyDateTimeAPI_impl.once.is_completed() { + // PyPy expects the C-API to be initialized via PyDateTime_Import, so trying to use + // `PyCapsule_Import` will behave unexpectedly in pypy. + #[cfg(PyPy)] + let py_datetime_c_api = PyDateTime_Import(); + + #[cfg(not(PyPy))] + let py_datetime_c_api = + PyCapsule_Import(PyDateTime_CAPSULE_NAME.as_ptr(), 1) as *mut PyDateTime_CAPI; + + // Protect against race conditions when the datetime API is concurrently + // initialized in multiple threads. UnsafeCell.get() cannot panic so this + // won't panic either. + PyDateTimeAPI_impl.once.call_once(|| { + *PyDateTimeAPI_impl.ptr.get() = py_datetime_c_api; + }); + } } #[inline] @@ -735,8 +743,13 @@ extern "C" { // Rust specific implementation details -struct PyDateTimeAPISingleton(UnsafeCell<*mut PyDateTime_CAPI>); +struct PyDateTimeAPISingleton { + once: Once, + ptr: UnsafeCell<*mut PyDateTime_CAPI>, +} unsafe impl Sync for PyDateTimeAPISingleton {} -static PyDateTimeAPI_impl: PyDateTimeAPISingleton = - PyDateTimeAPISingleton(UnsafeCell::new(ptr::null_mut())); +static PyDateTimeAPI_impl: PyDateTimeAPISingleton = PyDateTimeAPISingleton { + once: Once::new(), + ptr: UnsafeCell::new(ptr::null_mut()), +}; From 011364ea03dc2d0721363aea055fbc13d942b90e Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 15 Oct 2024 12:29:01 -0600 Subject: [PATCH 2/3] add changelog entry --- newsfragments/4623.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4623.fixed.md diff --git a/newsfragments/4623.fixed.md b/newsfragments/4623.fixed.md new file mode 100644 index 00000000000..18fd8460b44 --- /dev/null +++ b/newsfragments/4623.fixed.md @@ -0,0 +1 @@ +* The FFI wrapper for the PyDateTime_IMPORT macro is now thread-safe. From 8e027866c0e6ea0faedb37c8f46a422ede47304f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Sat, 19 Oct 2024 17:15:04 -0600 Subject: [PATCH 3/3] add error checking for PyCapsule_Import call --- pyo3-ffi/src/datetime.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pyo3-ffi/src/datetime.rs b/pyo3-ffi/src/datetime.rs index 66862d72328..76d12151afc 100644 --- a/pyo3-ffi/src/datetime.rs +++ b/pyo3-ffi/src/datetime.rs @@ -618,6 +618,10 @@ pub unsafe fn PyDateTime_IMPORT() { let py_datetime_c_api = PyCapsule_Import(PyDateTime_CAPSULE_NAME.as_ptr(), 1) as *mut PyDateTime_CAPI; + if py_datetime_c_api.is_null() { + return; + } + // Protect against race conditions when the datetime API is concurrently // initialized in multiple threads. UnsafeCell.get() cannot panic so this // won't panic either.