Skip to content

Commit d5b37fc

Browse files
authored
feat: Allow js functions as python callbacks (#84)
1 parent 11e1ac2 commit d5b37fc

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)