mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 10:44:36 +08:00
Own generated native closures and retain their method definitions (#8962)
* Own generated native closures and retain their method definitions Assisted-by: Codex:gpt-6 * Keep native callable GC traversal off the bind path Revert method_descriptor and classmethod_descriptor to untracked payloads. Their only object field, _method_def_owner, points to an untracked HeapMethodDef. Restore the derived Traverse for builtin_function_or_method without a clear implementation. default_dealloc calls the clear slot on every deallocation, so the custom clear allocated a Vec for each bound builtin. Use the derived Traverse for builtin_method, skipping the static class. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Jeong YunWon <jeong@youknowone.org>
This commit is contained in:
1 parent
fdd38b8193
commit
a61f37a1b7
4 files changed
+113
-21
No files matched your search
@@ -78,7 +78,7 @@ impl PyNativeFunction {
|
||||
self.zelf.as_deref().or(self.module_object.as_deref())
|
||||
}
|
||||
|
||||
pub const fn as_func(&self) -> &'static dyn PyNativeFn {
|
||||
pub const fn as_func(&self) -> &dyn PyNativeFn {
|
||||
self.value.func
|
||||
}
|
||||
}
|
||||
@@ -243,10 +243,12 @@ impl PyNativeFunction {
|
||||
name = "builtin_method",
|
||||
module = false,
|
||||
base = PyNativeFunction,
|
||||
ctx = "builtin_method_type"
|
||||
ctx = "builtin_method_type",
|
||||
traverse
|
||||
)]
|
||||
pub struct PyNativeMethod {
|
||||
pub(crate) func: PyNativeFunction,
|
||||
#[pytraverse(skip)]
|
||||
pub(crate) class: &'static Py<PyType>,
|
||||
}
|
||||
|
||||
|
||||
@@ -39,7 +39,7 @@ pub struct PyMethodDescriptor {
|
||||
#[pymember(name = "__objclass__", path = "typ")]
|
||||
#[pymember(name = "__name__", path = "name")]
|
||||
pub common: PyDescriptor,
|
||||
pub method: &'static PyMethodDef,
|
||||
pub(crate) method: &'static PyMethodDef,
|
||||
// vectorcall: vector_call_func,
|
||||
/// Prevent HeapMethodDef from being freed while this descriptor references it
|
||||
pub(crate) _method_def_owner: Option<PyObjectRef>,
|
||||
@@ -126,7 +126,8 @@ impl Callable for PyMethodDescriptor {
|
||||
|
||||
impl PyMethodDescriptor {
|
||||
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeFunction> {
|
||||
self.method.build_bound_function(ctx, obj)
|
||||
self.method
|
||||
.build_bound_function_with_owner(ctx, obj, self._method_def_owner.clone())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,7 +182,7 @@ pub struct PyClassMethodDescriptor {
|
||||
#[pymember(name = "__objclass__", path = "typ")]
|
||||
#[pymember(name = "__name__", path = "name")]
|
||||
pub common: PyDescriptor,
|
||||
pub method: &'static PyMethodDef,
|
||||
pub(crate) method: &'static PyMethodDef,
|
||||
pub(crate) _method_def_owner: Option<PyObjectRef>,
|
||||
}
|
||||
|
||||
@@ -199,7 +200,8 @@ impl PyClassMethodDescriptor {
|
||||
}
|
||||
|
||||
pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef<PyNativeFunction> {
|
||||
self.method.build_bound_function(ctx, obj)
|
||||
self.method
|
||||
.build_bound_function_with_owner(ctx, obj, self._method_def_owner.clone())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -232,13 +232,22 @@ impl PyMethodDef {
|
||||
&'static self,
|
||||
ctx: &Context,
|
||||
obj: PyObjectRef,
|
||||
) -> PyRef<PyNativeFunction> {
|
||||
self.build_bound_function_with_owner(ctx, obj, None)
|
||||
}
|
||||
|
||||
pub(crate) fn build_bound_function_with_owner(
|
||||
&'static self,
|
||||
ctx: &Context,
|
||||
obj: PyObjectRef,
|
||||
owner: Option<PyObjectRef>,
|
||||
) -> PyRef<PyNativeFunction> {
|
||||
let function = PyNativeFunction {
|
||||
zelf: Some(obj),
|
||||
value: self,
|
||||
module_object: None,
|
||||
module: crate::object::PyAtomicRef::new_empty(),
|
||||
_method_def_owner: None,
|
||||
_method_def_owner: owner,
|
||||
};
|
||||
PyRef::new_ref(
|
||||
function,
|
||||
@@ -383,15 +392,51 @@ impl core::fmt::Debug for PyMethodDef {
|
||||
// This is not a part of CPython API.
|
||||
// But useful to support dynamically generated methods
|
||||
#[pyclass(name, module = false, ctx = "method_def")]
|
||||
#[derive(Debug)]
|
||||
pub struct HeapMethodDef {
|
||||
method: PyMethodDef,
|
||||
_function_owner: Option<Box<dyn PyNativeFn>>,
|
||||
}
|
||||
|
||||
impl core::fmt::Debug for HeapMethodDef {
|
||||
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
|
||||
self.method.fmt(f)
|
||||
}
|
||||
}
|
||||
|
||||
impl HeapMethodDef {
|
||||
#[must_use]
|
||||
pub const fn new(method: PyMethodDef) -> Self {
|
||||
Self { method }
|
||||
Self {
|
||||
method,
|
||||
_function_owner: None,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_owned_function(
|
||||
name: &'static str,
|
||||
function: Box<dyn PyNativeFn>,
|
||||
flags: PyMethodFlags,
|
||||
doc: super::ItemDoc,
|
||||
) -> Self {
|
||||
// SAFETY: the box has a stable address and is retained by this payload.
|
||||
// The definition is private; all functions/descriptors referring to it
|
||||
// retain this HeapMethodDef and expose no static reference to callers.
|
||||
let func = unsafe { &*core::ptr::from_ref(&*function) };
|
||||
Self {
|
||||
method: PyMethodDef {
|
||||
name,
|
||||
func,
|
||||
flags,
|
||||
#[cfg(feature = "doc")]
|
||||
doc_off: doc.offset,
|
||||
#[cfg(feature = "doc")]
|
||||
doc_len: doc.len,
|
||||
#[cfg(feature = "doc")]
|
||||
doc_body_pending: false,
|
||||
doc: doc.text,
|
||||
},
|
||||
_function_owner: Some(function),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -439,3 +484,53 @@ pub(crate) fn init(ctx: &'static Context) {
|
||||
|
||||
let _ = ctx.intern_str(HeapMethodDef::NAME);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::{AsObject, function::FuncArgs};
|
||||
|
||||
#[test]
|
||||
fn generated_functions_own_and_release_their_native_captures() {
|
||||
use alloc::sync::Arc;
|
||||
let captured = Arc::new(17);
|
||||
crate::Interpreter::without_stdlib(Default::default()).enter(|vm| {
|
||||
let capture = captured.clone();
|
||||
let definition = vm.ctx.new_method_def(
|
||||
"captured",
|
||||
move |_args: FuncArgs| *capture,
|
||||
PyMethodFlags::empty(),
|
||||
super::super::ItemDoc::NONE,
|
||||
);
|
||||
let function = definition.build_function(vm, None);
|
||||
let descriptor = definition.build_method(vm.ctx.types.object_type, vm);
|
||||
drop(definition);
|
||||
assert_eq!(Arc::strong_count(&captured), 2);
|
||||
assert_eq!(
|
||||
function
|
||||
.as_object()
|
||||
.call((), vm)
|
||||
.unwrap()
|
||||
.try_into_value::<i32>(vm)
|
||||
.unwrap(),
|
||||
17
|
||||
);
|
||||
let method = descriptor.bind(vm.ctx.none(), &vm.ctx);
|
||||
assert!(method._method_def_owner.is_some());
|
||||
drop(function);
|
||||
drop(descriptor);
|
||||
assert_eq!(Arc::strong_count(&captured), 2);
|
||||
assert_eq!(
|
||||
method
|
||||
.as_object()
|
||||
.call((), vm)
|
||||
.unwrap()
|
||||
.try_into_value::<i32>(vm)
|
||||
.unwrap(),
|
||||
17
|
||||
);
|
||||
drop(method);
|
||||
assert_eq!(Arc::strong_count(&captured), 1);
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -659,19 +659,12 @@ impl Context {
|
||||
where
|
||||
F: IntoPyNativeFn<FKind>,
|
||||
{
|
||||
let def = PyMethodDef {
|
||||
let payload = HeapMethodDef::with_owned_function(
|
||||
name,
|
||||
func: Box::leak(Box::new(f.into_func())),
|
||||
flags: flags.with_call_convention(F::ARGS),
|
||||
#[cfg(feature = "doc")]
|
||||
doc_off: doc.offset,
|
||||
#[cfg(feature = "doc")]
|
||||
doc_len: doc.len,
|
||||
#[cfg(feature = "doc")]
|
||||
doc_body_pending: false,
|
||||
doc: doc.text,
|
||||
};
|
||||
let payload = HeapMethodDef::new(def);
|
||||
Box::new(f.into_func()),
|
||||
flags.with_call_convention(F::ARGS),
|
||||
doc,
|
||||
);
|
||||
PyRef::new_ref(payload, self.types.method_def.to_owned(), None)
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user