Skip to content

Commit

Permalink
Fix dealloc implementation to collectly use subtype's tp_free
Browse files Browse the repository at this point in the history
  • Loading branch information
kngwyu committed Jun 22, 2020
1 parent b70ee9a commit f053bc3
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 24 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Fixed
- Fix passing explicit `None` to `Option<T>` argument `#[pyfunction]` with a default value. [#936](https://github.com/PyO3/pyo3/pull/936)
- Fix `PyClass.__new__`'s behavior when inherited by a Python class. [#990](https://github.com/PyO3/pyo3/pull/990)
- Fix `PyClass.__new__`'s not respecting subclasses when inherited by a Python class. [#990](https://github.com/PyO3/pyo3/pull/990)

## [0.10.1] - 2020-05-14
### Fixed
Expand Down
5 changes: 3 additions & 2 deletions examples/rustapi_module/tests/test_subclassing.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@

class SomeSubClass(Subclassable):
def __str__(self):
return "Subclass"
return "SomeSubclass"


def test_subclassing():
if not PYPY:
a = SomeSubClass()
assert str(a) == "Subclass"
assert str(a) == "SomeSubclass"
assert type(a) is SomeSubClass
4 changes: 2 additions & 2 deletions pyo3-derive-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
subcls: *mut pyo3::ffi::PyTypeObject,
subtype: *mut pyo3::ffi::PyTypeObject,
_args: *mut pyo3::ffi::PyObject,
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
Expand All @@ -206,7 +206,7 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {

let _result = pyo3::derive_utils::IntoPyNewResult::into_pynew_result(#body)?;
let initializer = pyo3::PyClassInitializer::from(_result);
let cell = initializer.create_cell_from_subtype(_py, subcls)?;
let cell = initializer.create_cell_from_subtype(_py, subtype)?;
Ok(cell as *mut pyo3::ffi::PyObject)
})
}
Expand Down
14 changes: 7 additions & 7 deletions src/freelist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

//! Free allocation list
use crate::ffi;
use crate::pyclass::{tp_free_fallback, PyClassAlloc};
use crate::type_object::{PyLayout, PyTypeInfo};
use crate::Python;
use crate::{ffi, AsPyPointer, FromPyPointer, PyAny, Python};
use std::mem;
use std::os::raw::c_void;

Expand Down Expand Up @@ -86,14 +85,15 @@ where

unsafe fn dealloc(py: Python, self_: *mut Self::Layout) {
(*self_).py_drop(py);

let obj = self_ as _;
if ffi::PyObject_CallFinalizerFromDealloc(obj) < 0 {
let obj = PyAny::from_borrowed_ptr_or_panic(py, self_ as _);
if Self::is_exact_instance(obj) && ffi::PyObject_CallFinalizerFromDealloc(obj.as_ptr()) < 0
{
// tp_finalize resurrected.
return;
}

if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list().insert(obj) {
match Self::type_object_raw(py).tp_free {
if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list().insert(obj.as_ptr()) {
match (*ffi::Py_TYPE(obj)).tp_free {
Some(free) => free(obj as *mut c_void),
None => tp_free_fallback(obj),
}
Expand Down
2 changes: 1 addition & 1 deletion src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ where
T::BaseLayout: PyBorrowFlagLayout<T::BaseType>,
{
let initializer = value.into();
let obj = unsafe { initializer.create_cell(py)? };
let obj = initializer.create_cell(py)?;
let ob = unsafe { Py::from_owned_ptr(py, obj as _) };
Ok(ob)
}
Expand Down
19 changes: 10 additions & 9 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! `PyClass` trait
use crate::class::methods::{PyClassAttributeDef, PyMethodDefType, PyMethods};
use crate::class::proto_methods::PyProtoMethods;
use crate::conversion::{IntoPyPointer, ToPyObject};
use crate::conversion::{AsPyPointer, FromPyPointer, IntoPyPointer, ToPyObject};
use crate::pyclass_slots::{PyClassDict, PyClassWeakRef};
use crate::type_object::{type_flags, PyLayout};
use crate::types::PyDict;
use crate::types::{PyAny, PyDict};
use crate::{class, ffi, PyCell, PyErr, PyNativeType, PyResult, PyTypeInfo, Python};
use std::ffi::CString;
use std::os::raw::c_void;
Expand Down Expand Up @@ -42,14 +42,16 @@ pub trait PyClassAlloc: PyTypeInfo + Sized {
/// `self_` must be a valid pointer to the Python heap.
unsafe fn dealloc(py: Python, self_: *mut Self::Layout) {
(*self_).py_drop(py);
let obj = self_ as _;
if ffi::PyObject_CallFinalizerFromDealloc(obj) < 0 {
let obj = PyAny::from_borrowed_ptr_or_panic(py, self_ as _);
if Self::is_exact_instance(obj) && ffi::PyObject_CallFinalizerFromDealloc(obj.as_ptr()) < 0
{
// tp_finalize resurrected.
return;
}

match Self::type_object_raw(py).tp_free {
Some(free) => free(obj as *mut c_void),
None => tp_free_fallback(obj),
match (*ffi::Py_TYPE(obj.as_ptr())).tp_free {
Some(free) => free(obj.as_ptr() as *mut c_void),
None => tp_free_fallback(obj.as_ptr()),
}
}
}
Expand All @@ -66,8 +68,7 @@ fn tp_dealloc<T: PyClassAlloc>() -> Option<ffi::destructor> {
Some(dealloc::<T>)
}

#[doc(hidden)]
pub unsafe fn tp_free_fallback(obj: *mut ffi::PyObject) {
pub(crate) unsafe fn tp_free_fallback(obj: *mut ffi::PyObject) {
let ty = ffi::Py_TYPE(obj);
if ffi::PyType_IS_GC(ty) != 0 {
ffi::PyObject_GC_Del(obj as *mut c_void);
Expand Down
7 changes: 5 additions & 2 deletions src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,19 @@ impl<T: PyClass> PyClassInitializer<T> {
}

// Create a new PyCell and initialize it.
pub(crate) unsafe fn create_cell(self, py: Python) -> PyResult<*mut PyCell<T>>
pub(crate) fn create_cell(self, py: Python) -> PyResult<*mut PyCell<T>>
where
T: PyClass,
T::BaseLayout: PyBorrowFlagLayout<T::BaseType>,
{
self.create_cell_from_subtype(py, T::type_object_raw(py) as *const _ as _)
unsafe { self.create_cell_from_subtype(py, T::type_object_raw(py) as *const _ as _) }
}

/// Create a new PyCell and initialize it given a typeobject `subtype`.
/// Called by our `tp_new` generated by the `#[new]` attribute.
///
/// # Safety
/// `cls` must be a subclass of T.
pub unsafe fn create_cell_from_subtype(
self,
py: Python,
Expand Down

0 comments on commit f053bc3

Please sign in to comment.