proxy causes test errors in Deno

[vicary]

Update 2023-02-07: A port for Deno is available at https://deno.land/x/comlink, when the npm: protocol is mature enough I will make it compatible with both Node and Deno (and Bun). My intention to merge still holds, let me know, @surma.

Update: After some deep dives, I think I can overhaul the proxy mechanism for a clean Deno support. If you want to see this happen, a coffee would do. I'll merge everything back when the author is active again.

---

Given this simple worker script,

// worker.ts
import { expose } from "https://cdn.skypack.dev/comlink?dts";

expose(async function (callback: (...args: unknown[]) => unknown) {
  return await callback();
});

It runs normally without leaking unclosed MessagePort at runtime via deno run:

// app.ts
import { proxy, wrap } from "https://cdn.skypack.dev/comlink?dts";

const worker = new Worker(new URL("./worker.ts").href, { type: "module" });
const linked = comlink.wrap(worker);
const callback = proxy(() => {
  console.log("called.");
});

await linked(callback);

worker.terminate();

But when I wrap the code above into a Deno test case it("should work", async () => { ... });, the test throws the an error with message AssertionError: Test case is leaking async ops.

Full error log (Click to expand)

error: AssertionError: Test case is leaking async ops.

 - 1 async operation to receive a message from a MessagePort was started in this test, but never completed. This is often caused by not awaiting the result of not closing a `MessagePort`. The operation was started here:
    at Object.opAsync (deno:core/01_core.js:176:42)
    at deno:ext/web/13_message_port.js:143:31
    at MessagePort.start (deno:ext/web/13_message_port.js:172:9)
    at expose (https://cdn.skypack.dev/-/comlink@v4.3.1-ebLSsXPUzhGrZgtPT5jX/dist=es2019,mode=imports/optimized/comlink.js:111:8)
    at Object.serialize (https://cdn.skypack.dev/-/comlink@v4.3.1-ebLSsXPUzhGrZgtPT5jX/dist=es2019,mode=imports/optimized/comlink.js:10:5)
    at toWireValue (https://cdn.skypack.dev/-/comlink@v4.3.1-ebLSsXPUzhGrZgtPT5jX/dist=es2019,mode=imports/optimized/comlink.js:222:56)
    at Array.map (<anonymous>)
    at processArguments (https://cdn.skypack.dev/-/comlink@v4.3.1-ebLSsXPUzhGrZgtPT5jX/dist=es2019,mode=imports/optimized/comlink.js:201:34)
    at Object.apply (https://cdn.skypack.dev/-/comlink@v4.3.1-ebLSsXPUzhGrZgtPT5jX/dist=es2019,mode=imports/optimized/comlink.js:178:45)
    at Object.<anonymous> (file:///Users/vicary/Documents/Projects/vicary/deno-workerpool/Workerpool.test.ts:146:11)
 - 1 async operation to op_host_recv_ctrl was started in this test, but never completed. The operation was started here:
    at Object.opAsync (deno:core/01_core.js:176:42)
    at hostRecvCtrl (deno:runtime/js/11_workers.js:54:17)
    at Worker.#pollControl (deno:runtime/js/11_workers.js:140:36)
    at new Worker (deno:runtime/js/11_workers.js:113:24)
    at Object.<anonymous> (file:///Users/vicary/Documents/Projects/vicary/deno-workerpool/Workerpool.test.ts:137:20)
    at Function.runTest (https://deno.land/std@0.155.0/testing/_test_suite.ts:358:16)
    at Function.runTest (https://deno.land/std@0.155.0/testing/_test_suite.ts:346:33)
    at fn (https://deno.land/std@0.155.0/testing/_test_suite.ts:316:37)
    at testStepSanitizer (deno:runtime/js/40_testing.js:445:13)
    at asyncOpSanitizer (deno:runtime/js/40_testing.js:144:15)
 - 1 async operation to op_host_recv_message was started in this test, but never completed. The operation was started here:
    at Object.opAsync (deno:core/01_core.js:176:42)
    at hostRecvMessage (deno:runtime/js/11_workers.js:58:17)
    at Worker.#pollMessages (deno:runtime/js/11_workers.js:171:28)

    at assert (deno:ext/web/00_infra.js:295:13)
    at asyncOpSanitizer (deno:runtime/js/40_testing.js:226:13)
    at async resourceSanitizer (deno:runtime/js/40_testing.js:371:7)
    at async Object.exitSanitizer [as fn] (deno:runtime/js/40_testing.js:428:9)
    at async runTest (deno:runtime/js/40_testing.js:834:7)
    at async runTests (deno:runtime/js/40_testing.js:1089:22)

I suspect this is caused by a delayed disposal of an opened MessagePort via proxy and Deno test doesn't like it, could anyone confirm this?

[vicary]

After forking and some deep dives, I found that the proxy method is not adhering to the recommendations from the HTML Spec 9.4.5 Ports and garbage collection (The 2nd Note), quoted below.

Authors are strongly encouraged to explicitly close MessagePort objects to disentangle them, so that their resources can be recollected. Creating many MessagePort objects and discarding them without closing them can lead to high transient memory usage since garbage collection is not necessarily performed promptly, especially for MessagePorts where garbage collection can involve cross-process coordination.

Specifically, proxyTransferHandler#serialize simply opens up a new MessageChannel every single time a proxy function is to be transferred. These MessagePort pairs are never closed explicitly and relies on GC.

Unfortunately Deno keep track of MessagePorts created during each test and asserts their disposal at the end of each test.

I think besides best practices, comlink technically did nothing wrong. But it'd be much better if users at least have a way to close the throwaway proxy channels.

[exside]

Enjoy the coffe 😉

[vicary]

@exside Here you go in my fork.

There I created a test for Deno, with comments describing different types of op leaks and how to deal with them.

If everything's good I'll release as 4.3.2 in deno.land next week, give it a try!