Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down
79 changes: 78 additions & 1 deletion test/test_with_gc.ts
Original file line number Diff line number Diff line change
@@ -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(
`
Expand Down Expand Up @@ -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();
}
});