Memory leak when terminating worker with ongoing call

[snosenzo]

From what I can tell there seems to be a memory leak causing otherwise unreferenced and garbage-disposable objects from being garbage collected because of comlink's usage of the FinalizationRegistry. Specifically when a wraped worker function is called and then terminated before returning, the class that references the worker is not disposed even after the instantiated class has no other references linking to it.

I created a fork with an example demonstrating the issue here

[Excerpt from example]

  class WorkerProcessor {
      constructor() {
        this.worker = new Worker("./Processor.worker.js");
        this.remote = Comlink.wrap(this.worker);
      }

      async process(array) {
        console.log("process started");
        if (!this.remote) {
          throw new Error("No remote");
        }
        return await this.remote.process(array);
      }

      terminate() {
        this.shutdown = true;
        // Uncommenting either line below fixes the memory leak
        // this.remote = undefined;
        // this.remote[Comlink.releaseProxy]();
        this.worker.terminate();
      }
    }

The class is used as follows:

    document
      .querySelector("#runAndTerminateBtn")
      .addEventListener("click", () => {
        const processor = new WorkerProcessor();
        processor.process(new Uint8Array(10));
        processor.terminate();
      });

Once the click handler has been called and is completed, I would expect that processor which is contained only to this function, would be able to be garbage collected. However it remains in memory even after manual GC, as seen in the heap snapshot below.

Referenced in the heap snapshot is the FinalizationRegistry holding onto a reference of processor. To test this, I set proxyFinalizers = undefined in comlink.ts, I found that the instantiated class is garbage collected, which causes me to believe that there is either an issue with the way that comlink uses the FinalizationRegistry or there is a deeper issue.

I tested calling [Comlink.releaseProxy]() on the proxy and it does perform the expected result and allow the instantiated class it's referenced-by to be garbage collected. However, the docs state that if the browser supports FinalizationRegistry this should be unnecessary.

Other testing I did to resolve the issue is set the reference to the wrapped comlink proxy to undefined in the instantiated before terminating the worker (as seen commented out in the example shared below). This allowed the instantiated class and promise to be garbage collected.

I do want to note that terminating the worker immediately after calling the async comlink proxy does cause the Promise from process() to never resolve. However, a promise not resolving shouldn't keep it from being garbage collected if nothing else is referencing it.

Another finding is that this happens with a .then on the promise and without .then on the promise (as depicted in the example).

I've tested this in Google Chrome and Firefox and both exhibit this same issue. The process for reproducing can be found in the index.html described in the example.

Here's an example chrome heap snapshot for the instantiated object that is never collected:

[Edited for clarity]

[snosenzo]

So I think I figured out that the issue isn't with comlink, but rather with the await used in process in my example, which causes the promise to keep a reference to this, causing neither to be disposed until the remote is unreferenced from this or manually released. It's a bit of a confusing issue though and I'm not sure all of my questions are answered w.r.t. the deletion of the FinalizationRegistry also solving my problem. Maybe I'll come back and do a bigger writeup if people find it helpful.

[benjamind]

Thanks for following up @snosenzo