runtime: implement SetFinalizer to fix the syscall/js reference leak - #5521
Conversation
Sorry, something went wrong.
|
@b0ch3nski all green and should be passing now. Made the finalizer runner spawn lazily so it adds no size on embedded, and scoped the firing test to wasm. Sorry for the delay, was enjoying my birthday with family and friends! 🙂 |
Sorry, something went wrong.
| } | ||
|
|
||
| // callFinalizer invokes a finalizer func value on the given object pointer. | ||
| func callFinalizer(objPtr unsafe.Pointer, fn interface{}) { |
There was a problem hiding this comment.
This assumes that any func(*T) has the same calling convention as func(unsafe.Pointer). In TinyGo this is currently true (all pointer types are one word), but there is no compile-time validation that fn is actually func(*T) for the correct T.
Sorry, something went wrong.
There was a problem hiding this comment.
Just made SetFinalizer validate its args up front like the stdlib (panics if obj isn't a pointer or finalizer isn't a func).
Reflectlite can't check the exact func(*T) signature, but TinyGo's one-word pointer ABI keeps the reinterpret safe once it's confirmed to be a func.
Also checked the goroutines.go failure and it looks unrelated to my change: it's gated to the conservative/precise GCs, so it isn't even compiled on the boehm host, and the goroutines.go host binary comes out byte-identical with and without my change. Looks like a pre-existing timing flake in that test, so a re-run should be green.
Sorry, something went wrong.
|
Thank you for the fix @felipegenef and to @b0ch3nski for helping review. Now merging. |
Sorry, something went wrong.
Implement runtime.SetFinalizer to fix the syscall/js reference leak
Fixes #1652 (also reported in #1140).
The leak
runtime.SetFinalizer is a no-op on every built-in GC. TinyGo compiles Go's syscall/js unchanged, and makeValue registers a finalizer on every js.Value to call finalizeRef, which frees the value's slot in the wasm_exec.js bridge tables.
Since the finalizer never runs, those tables grow without bound. Every GOOS=js program leaks roughly 2 to 4 slots per DOM event until it OOMs. The same program under standard Go does not leak.
The fix
Implement SetFinalizer in the block GC (gc_blocks.go, used by precise and conservative):
Because the runner is spawned lazily, a program that never calls SetFinalizer links none of this in: the linker drops the whole subsystem, so there is no size or RAM cost. In practice this only affects GOOS=js (wasm), where syscall/js registers these finalizers; TestBinarySize for the baremetal targets is unchanged.
Scope
The common case only: SetFinalizer(ptr, func(ptrType)), run-once, and nil to clear. Ordering, cycles, and AddCleanup are out of scope. wasm_exec.js and the other GCs' stubs are untouched.
Testing
New testdata/finalizer.go golden test covers fire, run-once, clear, and replace. It runs on -target=wasm (GOOS=js), where collecting a specific dropped object is deterministic; on the boehm host and the conservative emulated targets collection is not deterministic, so it is scoped out there. gc.go still passes on host, wasm, wasip1, and wasip2, make fmt is clean, and TestBuild / TestBinarySize are green.
A js.FuncOf harness confirms the bridge table returns to baseline after runtime.GC() instead of growing without bound: