From e3dd721b5027ab5f7667e409546ddbf2a76e7f13 Mon Sep 17 00:00:00 2001 From: Nbiba Bedis Date: Wed, 15 Oct 2025 20:56:52 +0100 Subject: [PATCH] Allow js functions as python callbacks Since we cant determine when to clean the callbacks, and exposing this to the user will give an uggly api, the best thing is to rely on FinalizationRegistry to clean up the resource when the GC clears the callback, It seems to work as you can see in the test and Ithink this really improves the api --- src/python.ts | 17 ++++++++++ test/test_with_gc.ts | 79 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/python.ts b/src/python.ts index c8d7332..e0580ec 100644 --- a/src/python.ts +++ b/src/python.ts @@ -5,6 +5,14 @@ import { cstr, SliceItemRegExp } from "./util.ts"; const refregistry = new FinalizationRegistry(py.Py_DecRef); +// FinalizationRegistry for auto-created callbacks +// Closes the callback when the PyObject holding it is GC'd +const callbackCleanupRegistry = new FinalizationRegistry( + (callback: Callback) => { + callback.destroy(); + }, +); + /** * Symbol used on proxied Python objects to point to the original PyObject object. * Can be used to implement PythonProxy and create your own proxies. @@ -548,6 +556,7 @@ export class PyObject { // Is this still needed (after the change of pinning fields to the callabck) ? might be const pyObject = new PyObject(fn); pyObject.#pyMethodDef = methodDef; + // Note: explicit Callback objects are user-managed, not auto-cleaned return pyObject; } else if (v instanceof PyObject) { return v; @@ -601,6 +610,14 @@ export class PyObject { if (ProxiedPyObject in v) { return (v as any)[ProxiedPyObject]; } + + if (typeof v === "function") { + const callback = new Callback(v); + const pyObject = PyObject.from(callback); + // Register cleanup to close callback when PyObject is GC'd + callbackCleanupRegistry.register(pyObject, callback); + return pyObject; + } } default: diff --git a/test/test_with_gc.ts b/test/test_with_gc.ts index d8724b9..ad0455a 100644 --- a/test/test_with_gc.ts +++ b/test/test_with_gc.ts @@ -1,6 +1,22 @@ -import python, { Callback } from "../mod.ts"; +import python, { Callback, PyObject } from "../mod.ts"; import { assertEquals } from "./asserts.ts"; +Deno.test("js fns are automaticlly converted to callbacks", () => { + const pyModule = python.runModule( + ` +def call_the_callback(cb): + result = cb() + return result + 1 + `, + "test_module", + ); + + assertEquals(pyModule.call_the_callback(() => 4).valueOf(), 5); + + // @ts-ignore:requires: --v8-flags=--expose-gc + gc(); // if this is commented out, the test will fail beacuse the callback was not freed +}); + Deno.test("callbacks are not gc'd while still needed by python", () => { const pyModule = python.runModule( ` @@ -46,3 +62,64 @@ def call_stored_callback(): assertEquals(callCount, 3); callbackObj.destroy(); }); + +Deno.test("callbacks are not gc'd while still needed by python (function version)", () => { + const pyModule = python.runModule( + ` +stored_callback = None + +def store_and_call_callback(cb): + global stored_callback + stored_callback = cb + return stored_callback() + +def call_stored_callback(): + global stored_callback + if stored_callback is None: + return -1 + return stored_callback() + `, + "test_gc_module", + ); + + let callCount = 0; + const callback = () => { + callCount++; + return callCount * 10; + }; + + // Store the callback in Python and call it + assertEquals(pyModule.store_and_call_callback(callback).valueOf(), 10); + assertEquals(callCount, 1); + + for (let i = 0; i < 10; i++) { + // @ts-ignore:requires: --v8-flags=--expose-gc + gc(); + } + + // If the callback was incorrectly GC'd, this should segfault + // But it should work because Python holds a reference + assertEquals(pyModule.call_stored_callback().valueOf(), 20); + assertEquals(callCount, 2); + + // Call it again to be sure + assertEquals(pyModule.call_stored_callback().valueOf(), 30); + assertEquals(callCount, 3); +}); + +Deno.test("auto-created callbacks are cleaned up after gc", () => { + // Create callback and explicitly null it out to help GC + // @ts-ignore PyObject can be created from fns its just the types are not exposed + // deno-lint-ignore no-explicit-any + let _f: any = PyObject.from(() => 5); + + // Explicitly null the reference + _f = null; + + // Now f is null, trigger GC to clean it up + // Run many GC cycles with delays to ensure finalizers execute + for (let i = 0; i < 10; i++) { + // @ts-ignore:requires: --v8-flags=--expose-gc + gc(); + } +});