From 053edde054da5810983ff5d10a2278dcec7e8f2d Mon Sep 17 00:00:00 2001 From: 1ndahous3 <9205230+1ndahous3@users.noreply.github.com> Date: Sun, 4 Oct 2026 17:40:16 +0300 Subject: [PATCH 1/2] Own generated native closures and retain their method definitions Assisted-by: Codex:gpt-6 --- crates/vm/src/builtins/builtin_func.rs | 40 +++++++++- crates/vm/src/builtins/descriptor.rs | 27 +++++-- crates/vm/src/function/method.rs | 101 ++++++++++++++++++++++++- crates/vm/src/vm/context.rs | 17 ++--- 4 files changed, 160 insertions(+), 25 deletions(-) diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index 61e93ad031b..1d1b2b69c2c 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -5,15 +5,19 @@ use crate::{ common::wtf8::Wtf8, convert::TryFromObject, function::{Callee, FuncArgs, PyComparisonValue, PyMethodDef, PyMethodFlags, PyNativeFn}, + object::{Traverse, TraverseFn}, types::{Callable, Comparable, PyComparisonOp, Representable}, }; use alloc::fmt; // PyCFunctionObject in CPython #[repr(C)] -#[pyclass(name = "builtin_function_or_method", module = false, traverse)] +#[pyclass( + name = "builtin_function_or_method", + module = false, + traverse = "manual" +)] pub struct PyNativeFunction { - #[pytraverse(skip)] pub(crate) value: &'static PyMethodDef, pub(crate) zelf: Option, // Module that owns this function. Not passed as a call argument. @@ -24,6 +28,23 @@ pub struct PyNativeFunction { pub(crate) _method_def_owner: Option, } +unsafe impl Traverse for PyNativeFunction { + fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { + self.zelf.traverse(tracer_fn); + self.module_object.traverse(tracer_fn); + self.module.traverse(tracer_fn); + self._method_def_owner.traverse(tracer_fn); + } + + fn clear(&mut self, out: &mut Vec) { + out.extend(self.zelf.take()); + out.extend(self.module_object.take()); + // GC has exclusive access while clearing this unreachable object. + out.extend(unsafe { self.module.swap(None) }); + // Keep the definition owner until deallocation: `value` borrows it. + } +} + impl PyPayload for PyNativeFunction { fn class(ctx: &Context) -> &'static Py { ctx.types.builtin_function_or_method_type @@ -78,7 +99,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,13 +264,24 @@ impl PyNativeFunction { name = "builtin_method", module = false, base = PyNativeFunction, - ctx = "builtin_method_type" + ctx = "builtin_method_type", + traverse = "manual" )] pub struct PyNativeMethod { pub(crate) func: PyNativeFunction, pub(crate) class: &'static Py, } +unsafe impl Traverse for PyNativeMethod { + fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { + self.func.traverse(tracer_fn); + } + + fn clear(&mut self, out: &mut Vec) { + self.func.clear(out); + } +} + #[pyclass(flags(HAS_WEAKREF, DISALLOW_INSTANTIATION))] impl PyNativeMethod {} diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index ae8f0f6a1af..866a3a057e9 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -8,6 +8,7 @@ use crate::{ function::{ Callee, FuncArgs, ItemDoc, PyMethodDef, PyMethodFlags, PySetterValue, PySsize, plain_doc, }, + object::{Traverse, TraverseFn}, protocol::{PyNumberBinaryFunc, PyNumberTernaryFunc, PyNumberUnaryFunc}, types::{ Callable, Comparable, DelFunc, DescrGetFunc, DescrSetFunc, GenericMethod, GetDescriptor, @@ -34,17 +35,23 @@ pub struct PyDescriptorOwned { pub qualname: PyRwLock>, } -#[pyclass(name = "method_descriptor", module = false)] +#[pyclass(name = "method_descriptor", module = false, traverse = "manual")] 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, } +unsafe impl Traverse for PyMethodDescriptor { + fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { + self._method_def_owner.traverse(tracer_fn); + } +} + impl PyMethodDescriptor { pub fn new(method: &'static PyMethodDef, typ: &'static Py, ctx: &Context) -> Self { Self { @@ -126,7 +133,8 @@ impl Callable for PyMethodDescriptor { impl PyMethodDescriptor { pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { - self.method.build_bound_function(ctx, obj) + self.method + .build_bound_function_with_owner(ctx, obj, self._method_def_owner.clone()) } } @@ -176,15 +184,21 @@ impl Representable for PyMethodDescriptor { } // METH_CLASS descriptors. Same layout as method_descriptor; a distinct type. -#[pyclass(name = "classmethod_descriptor", module = false)] +#[pyclass(name = "classmethod_descriptor", module = false, traverse = "manual")] 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, } +unsafe impl Traverse for PyClassMethodDescriptor { + fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { + self._method_def_owner.traverse(tracer_fn); + } +} + impl PyClassMethodDescriptor { pub fn new(method: &'static PyMethodDef, typ: &'static Py, ctx: &Context) -> Self { Self { @@ -199,7 +213,8 @@ impl PyClassMethodDescriptor { } pub fn bind(&self, obj: PyObjectRef, ctx: &Context) -> PyRef { - self.method.build_bound_function(ctx, obj) + self.method + .build_bound_function_with_owner(ctx, obj, self._method_def_owner.clone()) } } diff --git a/crates/vm/src/function/method.rs b/crates/vm/src/function/method.rs index b33da287710..d586c099ba7 100644 --- a/crates/vm/src/function/method.rs +++ b/crates/vm/src/function/method.rs @@ -232,13 +232,22 @@ impl PyMethodDef { &'static self, ctx: &Context, obj: PyObjectRef, + ) -> PyRef { + 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, ) -> PyRef { 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>, +} + +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, + 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::(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::(vm) + .unwrap(), + 17 + ); + drop(method); + assert_eq!(Arc::strong_count(&captured), 1); + }); + } +} diff --git a/crates/vm/src/vm/context.rs b/crates/vm/src/vm/context.rs index 42da0551cae..56fa65feb10 100644 --- a/crates/vm/src/vm/context.rs +++ b/crates/vm/src/vm/context.rs @@ -659,19 +659,12 @@ impl Context { where F: IntoPyNativeFn, { - 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) } From 30064ddd2467bff961ad5639482966af251bcf6b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 15 Oct 2026 21:48:25 +0900 Subject: [PATCH 2/2] 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 --- crates/vm/src/builtins/builtin_func.rs | 38 +++----------------------- crates/vm/src/builtins/descriptor.rs | 17 ++---------- 2 files changed, 6 insertions(+), 49 deletions(-) diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index 1d1b2b69c2c..917b7b24fcc 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -5,19 +5,15 @@ use crate::{ common::wtf8::Wtf8, convert::TryFromObject, function::{Callee, FuncArgs, PyComparisonValue, PyMethodDef, PyMethodFlags, PyNativeFn}, - object::{Traverse, TraverseFn}, types::{Callable, Comparable, PyComparisonOp, Representable}, }; use alloc::fmt; // PyCFunctionObject in CPython #[repr(C)] -#[pyclass( - name = "builtin_function_or_method", - module = false, - traverse = "manual" -)] +#[pyclass(name = "builtin_function_or_method", module = false, traverse)] pub struct PyNativeFunction { + #[pytraverse(skip)] pub(crate) value: &'static PyMethodDef, pub(crate) zelf: Option, // Module that owns this function. Not passed as a call argument. @@ -28,23 +24,6 @@ pub struct PyNativeFunction { pub(crate) _method_def_owner: Option, } -unsafe impl Traverse for PyNativeFunction { - fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self.zelf.traverse(tracer_fn); - self.module_object.traverse(tracer_fn); - self.module.traverse(tracer_fn); - self._method_def_owner.traverse(tracer_fn); - } - - fn clear(&mut self, out: &mut Vec) { - out.extend(self.zelf.take()); - out.extend(self.module_object.take()); - // GC has exclusive access while clearing this unreachable object. - out.extend(unsafe { self.module.swap(None) }); - // Keep the definition owner until deallocation: `value` borrows it. - } -} - impl PyPayload for PyNativeFunction { fn class(ctx: &Context) -> &'static Py { ctx.types.builtin_function_or_method_type @@ -265,23 +244,14 @@ impl PyNativeFunction { module = false, base = PyNativeFunction, ctx = "builtin_method_type", - traverse = "manual" + traverse )] pub struct PyNativeMethod { pub(crate) func: PyNativeFunction, + #[pytraverse(skip)] pub(crate) class: &'static Py, } -unsafe impl Traverse for PyNativeMethod { - fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self.func.traverse(tracer_fn); - } - - fn clear(&mut self, out: &mut Vec) { - self.func.clear(out); - } -} - #[pyclass(flags(HAS_WEAKREF, DISALLOW_INSTANTIATION))] impl PyNativeMethod {} diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index 866a3a057e9..4c7c1804d4c 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -8,7 +8,6 @@ use crate::{ function::{ Callee, FuncArgs, ItemDoc, PyMethodDef, PyMethodFlags, PySetterValue, PySsize, plain_doc, }, - object::{Traverse, TraverseFn}, protocol::{PyNumberBinaryFunc, PyNumberTernaryFunc, PyNumberUnaryFunc}, types::{ Callable, Comparable, DelFunc, DescrGetFunc, DescrSetFunc, GenericMethod, GetDescriptor, @@ -35,7 +34,7 @@ pub struct PyDescriptorOwned { pub qualname: PyRwLock>, } -#[pyclass(name = "method_descriptor", module = false, traverse = "manual")] +#[pyclass(name = "method_descriptor", module = false)] pub struct PyMethodDescriptor { #[pymember(name = "__objclass__", path = "typ")] #[pymember(name = "__name__", path = "name")] @@ -46,12 +45,6 @@ pub struct PyMethodDescriptor { pub(crate) _method_def_owner: Option, } -unsafe impl Traverse for PyMethodDescriptor { - fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self._method_def_owner.traverse(tracer_fn); - } -} - impl PyMethodDescriptor { pub fn new(method: &'static PyMethodDef, typ: &'static Py, ctx: &Context) -> Self { Self { @@ -184,7 +177,7 @@ impl Representable for PyMethodDescriptor { } // METH_CLASS descriptors. Same layout as method_descriptor; a distinct type. -#[pyclass(name = "classmethod_descriptor", module = false, traverse = "manual")] +#[pyclass(name = "classmethod_descriptor", module = false)] pub struct PyClassMethodDescriptor { #[pymember(name = "__objclass__", path = "typ")] #[pymember(name = "__name__", path = "name")] @@ -193,12 +186,6 @@ pub struct PyClassMethodDescriptor { pub(crate) _method_def_owner: Option, } -unsafe impl Traverse for PyClassMethodDescriptor { - fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self._method_def_owner.traverse(tracer_fn); - } -} - impl PyClassMethodDescriptor { pub fn new(method: &'static PyMethodDef, typ: &'static Py, ctx: &Context) -> Self { Self {