Skip to content

Commit e3dd721

Browse files
committed
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
1 parent 11e1ac2 commit e3dd721

File tree

2 files changed

+95
-1
lines changed

2 files changed

+95
-1
lines changed

src/python.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ import { cstr, SliceItemRegExp } from "./util.ts";
55

66
const refregistry = new FinalizationRegistry(py.Py_DecRef);
77

8+
// FinalizationRegistry for auto-created callbacks
9+
// Closes the callback when the PyObject holding it is GC'd
10+
const callbackCleanupRegistry = new FinalizationRegistry(
11+
(callback: Callback) => {
12+
callback.destroy();
13+
},
14+
);
15+
816
/**
917
* Symbol used on proxied Python objects to point to the original PyObject object.
1018
* Can be used to implement PythonProxy and create your own proxies.
@@ -548,6 +556,7 @@ export class PyObject {
548556
// Is this still needed (after the change of pinning fields to the callabck) ? might be
549557
const pyObject = new PyObject(fn);
550558
pyObject.#pyMethodDef = methodDef;
559+
// Note: explicit Callback objects are user-managed, not auto-cleaned
551560
return pyObject;
552561
} else if (v instanceof PyObject) {
553562
return v;
@@ -601,6 +610,14 @@ export class PyObject {
601610
if (ProxiedPyObject in v) {
602611
return (v as any)[ProxiedPyObject];
603612
}
613+
614+
if (typeof v === "function") {
615+
const callback = new Callback(v);
616+
const pyObject = PyObject.from(callback);
617+
// Register cleanup to close callback when PyObject is GC'd
618+
callbackCleanupRegistry.register(pyObject, callback);
619+
return pyObject;
620+
}
604621
}
605622

606623
default:

test/test_with_gc.ts

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
1-
import python, { Callback } from "../mod.ts";
1+
import python, { Callback, PyObject } from "../mod.ts";
22
import { assertEquals } from "./asserts.ts";
33

4+
Deno.test("js fns are automaticlly converted to callbacks", () => {
5+
const pyModule = python.runModule(
6+
`
7+
def call_the_callback(cb):
8+
result = cb()
9+
return result + 1
10+
`,
11+
"test_module",
12+
);
13+
14+
assertEquals(pyModule.call_the_callback(() => 4).valueOf(), 5);
15+
16+
// @ts-ignore:requires: --v8-flags=--expose-gc
17+
gc(); // if this is commented out, the test will fail beacuse the callback was not freed
18+
});
19+
420
Deno.test("callbacks are not gc'd while still needed by python", () => {
521
const pyModule = python.runModule(
622
`
@@ -46,3 +62,64 @@ def call_stored_callback():
4662
assertEquals(callCount, 3);
4763
callbackObj.destroy();
4864
});
65+
66+
Deno.test("callbacks are not gc'd while still needed by python (function version)", () => {
67+
const pyModule = python.runModule(
68+
`
69+
stored_callback = None
70+
71+
def store_and_call_callback(cb):
72+
global stored_callback
73+
stored_callback = cb
74+
return stored_callback()
75+
76+
def call_stored_callback():
77+
global stored_callback
78+
if stored_callback is None:
79+
return -1
80+
return stored_callback()
81+
`,
82+
"test_gc_module",
83+
);
84+
85+
let callCount = 0;
86+
const callback = () => {
87+
callCount++;
88+
return callCount * 10;
89+
};
90+
91+
// Store the callback in Python and call it
92+
assertEquals(pyModule.store_and_call_callback(callback).valueOf(), 10);
93+
assertEquals(callCount, 1);
94+
95+
for (let i = 0; i < 10; i++) {
96+
// @ts-ignore:requires: --v8-flags=--expose-gc
97+
gc();
98+
}
99+
100+
// If the callback was incorrectly GC'd, this should segfault
101+
// But it should work because Python holds a reference
102+
assertEquals(pyModule.call_stored_callback().valueOf(), 20);
103+
assertEquals(callCount, 2);
104+
105+
// Call it again to be sure
106+
assertEquals(pyModule.call_stored_callback().valueOf(), 30);
107+
assertEquals(callCount, 3);
108+
});
109+
110+
Deno.test("auto-created callbacks are cleaned up after gc", () => {
111+
// Create callback and explicitly null it out to help GC
112+
// @ts-ignore PyObject can be created from fns its just the types are not exposed
113+
// deno-lint-ignore no-explicit-any
114+
let _f: any = PyObject.from(() => 5);
115+
116+
// Explicitly null the reference
117+
_f = null;
118+
119+
// Now f is null, trigger GC to clean it up
120+
// Run many GC cycles with delays to ensure finalizers execute
121+
for (let i = 0; i < 10; i++) {
122+
// @ts-ignore:requires: --v8-flags=--expose-gc
123+
gc();
124+
}
125+
});

0 commit comments

Comments
 (0)