proposal: syscall/js: remove Func.Release()

[garet90]

I propose we remove the js.Func.Release() method.

Problem

Currently, js.Func's are very prone to bugs. Developers have to manually call Release() on allocated js.Func's after they are sure there are no references. But, releasing funcs too early leads to errors when called again from the js side. On the other hand, never releasing funcs (although safer) results in memory leaks.

Unlike OS files, socket connections, and other manually freed resources, the lifetime of js.Func's are sometimes unclear, making them a good candidate for automatic collection.

Solution

In 2021, JavaScript added a new FinalizationRegistry class where a function may be called when an object is cleaned up by the garbage collector. Using this, we can clean up the func's resources when they are gc'd on the JavaScript side.

Compatibility

syscall/js is not covered by the go compatibility promise.

Alternatively, to not break current apps, Func.Release() can be turned into a no-op or work as usual alongside the new auto gc.

Most modern versions of major browsers support FinalizationRegistry, which is the current precedent for implementing changes to the wasm backend (see comment). In the event of incompatibility, the js.Func can be leaked, still allowing the program to run as normally as possible.

[ianlancetaylor]

CC @golang/js

[Zxilly]

I suppose I could write a patch for this, converting func.Release to a no-op

[gopherbot]

Change https://go.dev/cl/610566 mentions this issue: syscall/js: auto recycle Func

[johanbrandhorst]

I think making func.Release a no-op where this functionality is available would be nice. We could first check if the FinalizationRegistry exists on the window, right? I don't think we want to remove it or break the functionality if we can avoid it, but we could document that it won't be necessary on modern browsers.

[jakebailey]

A major downside of FinalizationRegistry is that it only runs finalizers when the event loop yields, so for code that works synchronously nothing actually ends up getting released, unlike a finalizer/cleanup in Go or even JS's own other Weak* constructs. It'd be good to preserve a way to manually clean up values.

(The TypeScript port to Go has an API on the JS side, and although we're trying to do it out of proc instead of in Wasm, we are still avoiding FinalizationRegistry in favor of explicit resource management via Symbol.dispose partially for this reason.)

[Zxilly]

https://caniuse.com/mdn-javascript_builtins_finalizationregistry
https://caniuse.com/mdn-javascript_builtins_symbol_dispose

It seems that Symbol.dispose support is still pretty bad, maybe we could add FinalizationRegistry as a supplement while still keeping Func.Release? As a no-op if the function being released no longer exists?