luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1365 from davidhewitt/fix-tp-dealloc 

pyclass: fix reference count issue in subclass new
This commit is contained in:
David Hewitt 2021-01-08 06:43:03 +00:00 committed by GitHub
parent 423b7bf79f b92441cd7c
commit 364b7c2214
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 94 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Remove `#[deny(warnings)]` attribute (and instead refuse warnings only in CI). [#1340](https://github.com/PyO3/pyo3/pull/1340)
- Fix deprecation warning for missing `__module__` with `#[pyclass]`. [#1343](https://github.com/PyO3/pyo3/pull/1343)
- Correct return type of `PyFrozenSet::empty` to `&PyFrozenSet` (was incorrectly `&PySet`). [#1351](https://github.com/PyO3/pyo3/pull/1351)
- Fix missing `Py_INCREF` on heap type objects on Python versions before 3.8. [#1365](https://github.com/PyO3/pyo3/pull/1365)
## [0.13.0] - 2020-12-22
### Packaging

16
src/freelist.rs
View File

@ -76,6 +76,8 @@ where
if subtype == Self::type_object_raw(py) {
if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list(py).pop() {
ffi::PyObject_Init(obj, subtype);
#[cfg(not(Py_3_8))]
crate::pyclass::bpo_35810_workaround(py, subtype);
return obj as _;
}
}
@ -87,13 +89,13 @@ where
let obj = PyAny::from_borrowed_ptr_or_panic(py, self_ as _);
if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list(py).insert(obj.as_ptr()) {
match get_type_free(ffi::Py_TYPE(obj)) {
Some(free) => {
let ty = ffi::Py_TYPE(obj);
free(obj as *mut c_void);
ffi::Py_DECREF(ty as *mut ffi::PyObject);
}
None => tp_free_fallback(obj),
let ty = ffi::Py_TYPE(obj);
let free = get_type_free(ty).unwrap_or_else(|| tp_free_fallback(ty));
free(obj as *mut c_void);
#[cfg(Py_3_8)]
if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
ffi::Py_DECREF(ty as *mut ffi::PyObject);
}
}
}

58
src/pyclass.rs
View File

@ -1,11 +1,9 @@
//! `PyClass` and related traits.
use crate::class::methods::{PyClassAttributeDef, PyMethodDefType, PyMethods};
use crate::class::proto_methods::PyProtoMethods;
use crate::conversion::{AsPyPointer, FromPyPointer};
use crate::derive_utils::PyBaseTypeUtils;
use crate::pyclass_slots::{PyClassDict, PyClassWeakRef};
use crate::type_object::{type_flags, PyLayout};
use crate::types::PyAny;
use crate::{ffi, PyCell, PyErr, PyNativeType, PyResult, PyTypeInfo, Python};
use std::convert::TryInto;
use std::ffi::CString;
@ -23,6 +21,26 @@ pub(crate) unsafe fn get_type_free(tp: *mut ffi::PyTypeObject) -> Option<ffi::fr
mem::transmute(ffi::PyType_GetSlot(tp, ffi::Py_tp_free))
}
/// Workaround for Python issue 35810; no longer necessary in Python 3.8
#[inline]
#[cfg(not(Py_3_8))]
pub(crate) unsafe fn bpo_35810_workaround(_py: Python, ty: *mut ffi::PyTypeObject) {
#[cfg(Py_LIMITED_API)]
{
// Must check version at runtime for abi3 wheels - they could run against a higher version
// than the build config suggests.
use crate::once_cell::GILOnceCell;
static IS_PYTHON_3_8: GILOnceCell<bool> = GILOnceCell::new();
if *IS_PYTHON_3_8.get_or_init(_py, || _py.version_info() >= (3, 8)) {
// No fix needed - the wheel is running on a sufficiently new interpreter.
return;
}
}
ffi::Py_INCREF(ty as *mut ffi::PyObject);
}
#[inline]
pub(crate) unsafe fn default_new<T: PyTypeInfo>(
py: Python,
@ -44,8 +62,13 @@ pub(crate) unsafe fn default_new<T: PyTypeInfo>(
unreachable!("Subclassing native types isn't support in limited API mode");
}
}
let alloc = get_type_alloc(subtype).unwrap_or(ffi::PyType_GenericAlloc);
alloc(subtype, 0) as _
#[cfg(not(Py_3_8))]
bpo_35810_workaround(py, subtype);
alloc(subtype, 0)
}
/// This trait enables custom `tp_new`/`tp_dealloc` implementations for `T: PyClass`.
@ -64,15 +87,15 @@ 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 = PyAny::from_borrowed_ptr_or_panic(py, self_ as _);
let obj = self_ as *mut ffi::PyObject;
match get_type_free(ffi::Py_TYPE(obj.as_ptr())) {
Some(free) => {
let ty = ffi::Py_TYPE(obj.as_ptr());
free(obj.as_ptr() as *mut c_void);
ffi::Py_DECREF(ty as *mut ffi::PyObject);
}
None => tp_free_fallback(obj.as_ptr()),
let ty = ffi::Py_TYPE(obj);
let free = get_type_free(ty).unwrap_or_else(|| tp_free_fallback(ty));
free(obj as *mut c_void);
#[cfg(Py_3_8)]
if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
ffi::Py_DECREF(ty as *mut ffi::PyObject);
}
}
}
@ -89,18 +112,11 @@ fn tp_dealloc<T: PyClassAlloc>() -> Option<ffi::destructor> {
Some(dealloc::<T>)
}
pub(crate) unsafe fn tp_free_fallback(obj: *mut ffi::PyObject) {
let ty = ffi::Py_TYPE(obj);
pub(crate) unsafe fn tp_free_fallback(ty: *mut ffi::PyTypeObject) -> ffi::freefunc {
if ffi::PyType_IS_GC(ty) != 0 {
ffi::PyObject_GC_Del(obj as *mut c_void);
ffi::PyObject_GC_Del
} else {
ffi::PyObject_Free(obj as *mut c_void);
}
// For heap types, PyType_GenericAlloc calls INCREF on the type objects,
// so we need to call DECREF here:
if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
ffi::Py_DECREF(ty as *mut ffi::PyObject);
ffi::PyObject_Free
}
}

47
tests/test_inheritance.rs
View File

@ -209,3 +209,50 @@ mod inheriting_native_type {
);
}
}
#[pyclass(subclass)]
struct SimpleClass {}
#[pymethods]
impl SimpleClass {
#[new]
fn new() -> Self {
Self {}
}
}
#[test]
fn test_subclass_ref_counts() {
// regression test for issue #1363
use pyo3::type_object::PyTypeObject;
Python::with_gil(|py| {
#[allow(non_snake_case)]
let SimpleClass = SimpleClass::type_object(py);
py_run!(
py,
SimpleClass,
r#"
import gc
import sys
class SubClass(SimpleClass):
pass
gc.collect()
count = sys.getrefcount(SubClass)
for i in range(1000):
c = SubClass()
del c
gc.collect()
after = sys.getrefcount(SubClass)
# depending on Python's GC the count may be either identical or exactly 1000 higher,
# both are expected values that are not representative of the issue.
#
# (With issue #1363 the count will be decreased.)
assert after == count or (after == count + 1000), f"{after} vs {count}"
"#
);
})
}