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(); + } +});