runtime: implement SetFinalizer to fix the syscall/js reference leak#5521
Conversation
|
@felipegenef mind the CI errors |
735e164 to
9796326
Compare
|
@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! 🙂 |
| } | ||
|
|
||
| // 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.
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.
9796326 to
354d4ab
Compare
|
Thank you for the fix @felipegenef and to @b0ch3nski for helping review. Now merging. |
Implement runtime.SetFinalizer to fix the syscall/js reference leak
Fixes #1652 (also reported in #1140).
The leak
runtime.SetFinalizeris a no-op on every built-in GC. TinyGo compiles Go'ssyscall/jsunchanged, andmakeValueregisters a finalizer on everyjs.Valueto callfinalizeRef, which frees the value's slot in thewasm_exec.jsbridge tables.Since the finalizer never runs, those tables grow without bound. Every
GOOS=jsprogram leaks roughly 2 to 4 slots per DOM event until it OOMs. The same program under standard Go does not leak.The fix
Implement
SetFinalizerin the block GC (gc_blocks.go, used bypreciseandconservative):(object, finalizer). The object address is stored encoded so the conservative scanner can't pin it; the finalizer value is kept live by the table.SetFinalizercall and woken aftergcLockis released, so a finalizer can allocate safely. Underscheduler=nonethey drain inline instead.reflect.Value.Callis unavailable, so finalizers use the same closure-ABI call the runtime already uses for timer callbacks.Because the runner is spawned lazily, a program that never calls
SetFinalizerlinks none of this in: the linker drops the whole subsystem, so there is no size or RAM cost. In practice this only affectsGOOS=js(wasm), wheresyscall/jsregisters these finalizers;TestBinarySizefor the baremetal targets is unchanged.Scope
The common case only:
SetFinalizer(ptr, func(ptrType)), run-once, andnilto clear. Ordering, cycles, andAddCleanupare out of scope.wasm_exec.jsand the other GCs' stubs are untouched.Testing
New
testdata/finalizer.gogolden 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.gostill passes on host, wasm, wasip1, and wasip2,make fmtis clean, andTestBuild/TestBinarySizeare green.A
js.FuncOfharness confirms the bridge table returns to baseline afterruntime.GC()instead of growing without bound:_valueshigh-water