Skip to content

Commit

Permalink
avoid calling PyType_GetSlot on static types before 3.10
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Oct 4, 2024
1 parent 4ca6582 commit 677f68d
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 47 deletions.
49 changes: 22 additions & 27 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::impl_::pycell::{PyClassObject, PyClassObjectLayout};
use crate::pycell::impl_::PyClassBorrowChecker as _;
use crate::pycell::{PyBorrowError, PyBorrowMutError};
use crate::pyclass::boolean_struct::False;
use crate::type_object::get_slot_static_type_fallback;
use crate::types::any::PyAnyMethods;
use crate::{
ffi, Bound, DowncastError, Py, PyAny, PyClass, PyClassInitializer, PyErr, PyObject, PyRef,
Expand Down Expand Up @@ -695,39 +696,33 @@ pub unsafe fn tp_new_impl<T: PyClass>(
}

unsafe fn get_tp_traverse(ty: *mut ffi::PyTypeObject) -> Option<ffi::traverseproc> {
#[cfg(not(Py_LIMITED_API))]
{
(*ty).tp_traverse
}

#[cfg(Py_LIMITED_API)]
{
std::mem::transmute(ffi::PyType_GetSlot(ty, ffi::Py_tp_traverse))
}
let ptr = get_slot_static_type_fallback(
Python::assume_gil_acquired(),
ty,
ffi::Py_tp_traverse,
|ty| unsafe { std::mem::transmute((*ty).tp_traverse) },
);
std::mem::transmute(ptr)
}

unsafe fn get_tp_clear(ty: *mut ffi::PyTypeObject) -> Option<ffi::inquiry> {
#[cfg(not(Py_LIMITED_API))]
{
(*ty).tp_clear
}

#[cfg(Py_LIMITED_API)]
{
std::mem::transmute(ffi::PyType_GetSlot(ty, ffi::Py_tp_clear))
}
let ptr = get_slot_static_type_fallback(
Python::assume_gil_acquired(),
ty,
ffi::Py_tp_clear,
|ty| unsafe { std::mem::transmute((*ty).tp_clear) },
);
std::mem::transmute(ptr)
}

unsafe fn get_tp_base(ty: *mut ffi::PyTypeObject) -> *mut ffi::PyTypeObject {
#[cfg(not(Py_LIMITED_API))]
{
(*ty).tp_base
}

#[cfg(Py_LIMITED_API)]
{
ffi::PyType_GetSlot(ty, ffi::Py_tp_base).cast()
}
get_slot_static_type_fallback(
Python::assume_gil_acquired(),
ty,
ffi::Py_tp_clear,
|ty| unsafe { (*ty).tp_base.cast() },
)
.cast()
}

#[cfg(test)]
Expand Down
4 changes: 2 additions & 2 deletions src/pycell/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ where
let type_obj = T::type_object_raw(py);
// For `#[pyclass]` types which inherit from PyAny, we can just call tp_free
if type_obj == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) {
return get_tp_free(ffi::Py_TYPE(slf))(slf.cast());
return get_tp_free(py, ffi::Py_TYPE(slf))(slf.cast());
}

// More complex native types (e.g. `extends=PyDict`) require calling the base's dealloc.
Expand All @@ -221,7 +221,7 @@ where
}
dealloc(slf);
} else {
get_tp_free(ffi::Py_TYPE(slf))(slf.cast());
get_tp_free(py, ffi::Py_TYPE(slf))(slf.cast());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
// HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments
let is_base_object = type_object == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type);
if is_base_object {
let alloc = get_tp_alloc(subtype).unwrap_or(ffi::PyType_GenericAlloc);
let alloc = get_tp_alloc(py, subtype).unwrap_or(ffi::PyType_GenericAlloc);
let obj = alloc(subtype, 0);
return if obj.is_null() {
Err(PyErr::fetch(py))
Expand Down
65 changes: 50 additions & 15 deletions src/type_object.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Python type object information
use std::os::raw::c_int;

use crate::ffi_ptr_ext::FfiPtrExt;
use crate::types::any::PyAnyMethods;
use crate::types::{PyAny, PyType};
Expand Down Expand Up @@ -120,30 +122,63 @@ where
}

#[inline]
pub(crate) unsafe fn get_tp_alloc(tp: *mut ffi::PyTypeObject) -> Option<ffi::allocfunc> {
#[cfg(not(Py_LIMITED_API))]
{
(*tp).tp_alloc
}
pub(crate) unsafe fn get_tp_alloc(
py: Python<'_>,
tp: *mut ffi::PyTypeObject,
) -> Option<ffi::allocfunc> {
let ptr = get_slot_static_type_fallback(py, tp, ffi::Py_tp_alloc, |tp| unsafe {
std::mem::transmute((*tp).tp_alloc)
});
std::mem::transmute(ptr)
}

#[cfg(Py_LIMITED_API)]
{
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_alloc);
std::mem::transmute(ptr)
}
#[inline]
pub(crate) unsafe fn get_tp_free(py: Python<'_>, tp: *mut ffi::PyTypeObject) -> ffi::freefunc {
let ptr = get_slot_static_type_fallback(py, tp, ffi::Py_tp_free, |tp| unsafe {
std::mem::transmute((*tp).tp_free)
});
debug_assert_ne!(ptr, std::ptr::null_mut());
std::mem::transmute(ptr)
}

/// Before Python 3.10, `PyType_GetSlot` was not legal to use on static types.
///
/// `fallback` will resort to reading from the type object, which is sort of okay
/// for abi3 on 3.7 -> 3.9, because those type objects are definitely not changing
/// layout (between those versions).
///
/// `fallback` is also used as an optimized path on non-abi3.
#[inline]
pub(crate) unsafe fn get_tp_free(tp: *mut ffi::PyTypeObject) -> ffi::freefunc {
pub(crate) unsafe fn get_slot_static_type_fallback(
py: Python<'_>,
tp: *mut ffi::PyTypeObject,
slot: c_int,
fallback: unsafe fn(*mut ffi::PyTypeObject) -> *mut std::os::raw::c_void,
) -> *mut std::os::raw::c_void {
#[cfg(not(Py_LIMITED_API))]
{
(*tp).tp_free.unwrap()
let _ = py; // unused
let _ = slot; // unused
fallback(tp)
}

#[cfg(Py_LIMITED_API)]
{
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_free);
debug_assert_ne!(ptr, std::ptr::null_mut());
std::mem::transmute(ptr)
#[cfg(not(Py_3_10))]
{
// Must check version at runtime for abi3 wheels - they could run against a higher version
// than the build config suggests, and we don't know that the type object layout will
// always be stable.
use crate::sync::GILOnceCell;
static IS_PYTHON_3_10: GILOnceCell<bool> = GILOnceCell::new();

if !*IS_PYTHON_3_10.get_or_init(py, || py.version_info() >= (3, 10)) {
if ffi::PyType_HasFeature(attr.get_type_ptr(), ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
return fallback(tp);
}
}
}

unsafe { ffi::PyType_GetSlot(tp, slot) }
}
}
9 changes: 7 additions & 2 deletions src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::ffi_ptr_ext::FfiPtrExt;
use crate::instance::Bound;
use crate::internal_tricks::ptr_from_ref;
use crate::py_result_ext::PyResultExt;
use crate::type_object::{PyTypeCheck, PyTypeInfo};
use crate::type_object::{get_slot_static_type_fallback, PyTypeCheck, PyTypeInfo};
#[cfg(not(any(PyPy, GraalPy)))]
use crate::types::PySuper;
use crate::types::{PyDict, PyIterator, PyList, PyString, PyTuple, PyType};
Expand Down Expand Up @@ -1585,7 +1585,12 @@ impl<'py> Bound<'py, PyAny> {
// This is the preferred faster path, but does not work on static types (generally,
// types defined in extension modules) before Python 3.10.
unsafe {
let descr_get_ptr = ffi::PyType_GetSlot(attr.get_type_ptr(), ffi::Py_tp_descr_get);
let descr_get_ptr = get_slot_static_type_fallback(
py,
attr.get_type_ptr(),
ffi::Py_tp_descr_get,
|tp| std::mem::transmute((*tp).tp_descr_get),
);
if descr_get_ptr.is_null() {
return Ok(Some(attr));
}
Expand Down

0 comments on commit 677f68d

Please sign in to comment.