From 851d47e0136b05422510e2347cd115bf6757bd88 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Wed, 22 Jul 2026 23:00:27 -0300 Subject: [PATCH 01/16] runtime: run syscall/js finalizers on wasm without a manual GC --- main_test.go | 7 +- src/internal/task/task_asyncify.go | 41 ++++++++ src/internal/task/task_finishing_tasks.go | 10 ++ src/runtime/gc_finalizer.go | 50 +++++++++- src/runtime/gc_finalizer_sched.go | 15 ++- src/runtime/gc_finalizer_sched_other.go | 8 ++ src/runtime/scheduler_cooperative.go | 27 ++++++ testdata/finalizeridle.go | 108 ++++++++++++++++++++++ testdata/finalizeridle.txt | 1 + 9 files changed, 258 insertions(+), 9 deletions(-) create mode 100644 src/internal/task/task_finishing_tasks.go create mode 100644 src/runtime/gc_finalizer_sched_other.go create mode 100644 testdata/finalizeridle.go create mode 100644 testdata/finalizeridle.txt diff --git a/main_test.go b/main_test.go index e686cc8de7..220ae82074 100644 --- a/main_test.go +++ b/main_test.go @@ -60,6 +60,7 @@ func TestBuild(t *testing.T) { "channel.go", "embed/", "finalizer.go", + "finalizeridle.go", "float.go", "gc.go", "generics.go", @@ -359,9 +360,9 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { continue } } - if name == "finalizer.go" && options.Target != "wasm" { - // runtime.SetFinalizer is implemented for the block GC, but the - // test asserts deterministic collection of a dropped object, which + if (name == "finalizer.go" || name == "finalizeridle.go") && options.Target != "wasm" { + // runtime.SetFinalizer is implemented for the block GC, but these + // tests assert deterministic collection of a dropped object, which // only holds on the GOOS=js wasm target. The host default GC is // boehm (SetFinalizer is a no-op there); conservative stack scanning // on the emulated targets can pin the object; and the wasip2 diff --git a/src/internal/task/task_asyncify.go b/src/internal/task/task_asyncify.go index 0f74370678..97b4635fe9 100644 --- a/src/internal/task/task_asyncify.go +++ b/src/internal/task/task_asyncify.go @@ -45,6 +45,11 @@ type stackState struct { // overwritten. It can be checked from time to time to see whether a stack // overflow happened in the past. canaryPtr *uintptr + + // top is the first address past the end of the stack allocation (the + // initial C stack pointer). Kept so the whole stack buffer can be located + // again after the goroutine finishes. + top unsafe.Pointer } // start creates and starts a new goroutine with the given function and arguments. @@ -81,6 +86,35 @@ func (s *state) initialize(fn uintptr, args unsafe.Pointer, stackSize uintptr) { // Calculate stack base addresses. s.asyncifysp = unsafe.Add(stack, unsafe.Sizeof(uintptr(0))) s.csp = unsafe.Add(stack, stackSize) + s.top = unsafe.Add(stack, stackSize) +} + +//go:linkname memzero runtime.memzero +func memzero(ptr unsafe.Pointer, size uintptr) + +// finishing is set by the runtime immediately before a goroutine that has run +// to completion pauses for the last time. Resume observes it and clears the +// goroutine's stack. It is written and read on the same (cooperative) scheduler +// thread with no suspension point in between, so a plain global is safe. +var finishing bool + +// MarkFinishing records that the current goroutine has finished and will not be +// resumed, so Resume may reclaim its stack once control returns to the scheduler. +func MarkFinishing() { + finishing = true +} + +// clearStack zeroes a finished goroutine's entire stack buffer. The buffer is a +// plain heap allocation scanned conservatively by the GC (it can hold arbitrary +// pointers), so any stale pointer left in it by the goroutine's now-returned +// call frames would keep unrelated objects reachable (and, transitively, other +// finished stacks reachable through them) until a later collection happens to +// break the chain. Zeroing the buffer the moment the goroutine finishes drops +// those stale references immediately, so the objects they pointed at (and the +// stack itself) become collectable at the next cycle. +func (t *Task) clearStack() { + base := unsafe.Pointer(t.state.canaryPtr) + memzero(base, uintptr(t.state.top)-uintptr(base)) } // currentTask is the current running task, or nil if currently in the scheduler. @@ -126,6 +160,13 @@ func (t *Task) Resume() { if uintptr(t.state.asyncifysp) > uintptr(t.state.csp) { runtimePanic("stack overflow") } + if finishing { + // The goroutine just ran to completion and paused for the last time. It + // will never be resumed, so its stack can be cleared now to drop any + // pointers its returned frames left behind (see clearStack). + finishing = false + t.clearStack() + } } //go:linkname saveStackPointer runtime.saveStackPointer diff --git a/src/internal/task/task_finishing_tasks.go b/src/internal/task/task_finishing_tasks.go new file mode 100644 index 0000000000..5ba351a58d --- /dev/null +++ b/src/internal/task/task_finishing_tasks.go @@ -0,0 +1,10 @@ +//go:build scheduler.tasks + +package task + +// MarkFinishing is a no-op for the stack-based scheduler. Zeroing a finished +// goroutine's stack to drop the stale pointers its returned frames leave behind +// is only implemented for the asyncify scheduler, whose goroutine stacks are +// heap buffers scanned conservatively (see the asyncify MarkFinishing and +// Resume). deadlock and goexit in the cooperative scheduler call this for both. +func MarkFinishing() {} diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index 13dd6f2c7e..279c5eb2a9 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -33,10 +33,24 @@ type finalizerEntry struct { fn interface{} } +// finalizerGCThreshold bounds how many finalizers may be registered since the +// last collection before the scheduler proactively runs one at its idle point. +// A registered finalizer almost always guards an external resource, most +// importantly a syscall/js bridge-table slot (js.Value or js.Func), that costs +// only a few bytes of Go heap but pins a whole JS object and its slot. Without +// this, a long-lived instance with a large resident heap defers GC (and thus +// finalizer draining) until the Go heap itself fills, which for a bursty, +// mostly-idle workload may be never, so the external resources accumulate +// without bound. Coupling a GC to finalizer-registration pressure caps that +// accumulation at roughly this many entries regardless of heap size. Zero +// disables the trigger. +const finalizerGCThreshold = 32 + var ( finalizers *finalizerEntry // registered finalizers; a GC root that keeps fn values alive finalizerPending *finalizerEntry // finalizers whose object died, waiting to run numFinalizers uintptr // number of registered finalizers; fast-path gate for scanFinalizers + finalizersSinceGC uintptr // finalizers registered since the last GC; drives the scheduler idle-point pressure trigger finalizersQueued bool // set when scanFinalizers queued at least one finalizer to run finalizerFutex task.Futex // wakes the finalizerRunner goroutine after a GC queues work finalizerDraining bool // guards against re-entrant inline draining (scheduler.none) @@ -106,6 +120,7 @@ func registerFinalizer(addr uintptr, fn interface{}) { entry.next = finalizers finalizers = entry numFinalizers++ + finalizersSinceGC++ // pressure signal for the proactive GC trigger at the scheduler's idle point // A finalizer is registered, so make sure the runner exists. The flag is // serialized by gcLock; the spawn itself allocates, so it must run after the // lock is released. @@ -121,6 +136,11 @@ func registerFinalizer(addr uintptr, fn interface{}) { // current GC cycle and queues their finalizers. It must be called under gcLock, // after marking is complete and before sweep frees anything. func scanFinalizers() { + // A collection is running now, so reset the registration-pressure counter + // that drives the proactive idle-point trigger, regardless of whether any + // finalizer is registered or fires this cycle. + finalizersSinceGC = 0 + // Nothing registered and nothing waiting to run: fast path. if numFinalizers == 0 && finalizerPending == nil { return @@ -155,7 +175,7 @@ func scanFinalizers() { // found above and any queued by an earlier cycle that the runner has not // drained yet. Otherwise the next GC would not mark them (their only // reference is the encoded, scanner-invisible pending entry) and sweep would - // free them out from under a finalizer that hasn't run — a use-after-free. + // free them out from under a finalizer that hasn't run, a use-after-free. // Walking the pending list is safe: scanFinalizers and dequeueFinalizer are // both serialized under gcLock. var resurrected bool @@ -225,6 +245,34 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { return n, objPtr } +// finalizerPressureGC collects when at least finalizerGCThreshold finalizers +// have been registered since the last GC, then hands any freshly-queued +// finalizers to the runner. It reports whether it collected. A registered +// finalizer almost always guards an external resource whose Go-heap cost is tiny +// (a few bytes) relative to what it pins, so the registration count is a proxy +// for external memory pressure that the heap-size GC trigger cannot see. +// +// It is installed as the cooperative scheduler's idle hook by the first +// SetFinalizer (see spawnFinalizerRunner) and called only from the scheduler's +// drained-runqueue point, where no goroutine is running on its own stack. That +// reclaims a completed run of goroutines' now-dead values in a single pass, +// rather than forcing a collection synchronously inside alloc while an +// operation's values are still live, which would scale GC frequency with +// allocation churn and waste most collections on still-live values. +func finalizerPressureGC() bool { + if finalizerGCThreshold == 0 || finalizersSinceGC < finalizerGCThreshold { + return false + } + gcLock.Lock() + runGC() + gcLock.Unlock() + if finalizersQueued { + finalizersQueued = false + wakeFinalizer() + } + return true +} + // wakeFinalizer is called after a GC (with gcLock already released) that queued // finalizers. On schedulers with goroutines it wakes the finalizerRunner; on // scheduler.none it drains inline. diff --git a/src/runtime/gc_finalizer_sched.go b/src/runtime/gc_finalizer_sched.go index d983decc7c..7c2dcb4554 100644 --- a/src/runtime/gc_finalizer_sched.go +++ b/src/runtime/gc_finalizer_sched.go @@ -1,8 +1,13 @@ -//go:build (gc.conservative || gc.precise) && !scheduler.none +//go:build (gc.conservative || gc.precise) && (scheduler.tasks || scheduler.asyncify) package runtime -// The go statement lives in this scheduler-gated file, not inline in -// registerFinalizer, so scheduler.none builds never reference internal/task.start -// and the runner is DCE'd when SetFinalizer is unused. -func spawnFinalizerRunner() { go finalizerRunner() } +// The go statement and the idle-hook install live in this scheduler-gated file, +// not inline in registerFinalizer, so a build that never calls SetFinalizer +// keeps internal/task.start and the whole finalizer collection path DCE'd. The +// cooperative scheduler additionally collects on finalizer-registration pressure +// at its idle point (see finalizerIdleGC in scheduler_cooperative.go). +func spawnFinalizerRunner() { + finalizerIdleGC = finalizerPressureGC + go finalizerRunner() +} diff --git a/src/runtime/gc_finalizer_sched_other.go b/src/runtime/gc_finalizer_sched_other.go new file mode 100644 index 0000000000..e087b37ec3 --- /dev/null +++ b/src/runtime/gc_finalizer_sched_other.go @@ -0,0 +1,8 @@ +//go:build (gc.conservative || gc.precise) && !scheduler.none && !scheduler.tasks && !scheduler.asyncify + +package runtime + +// Non-cooperative schedulers (cores, threads) spawn the finalizer runner but +// have no cooperative idle point, so they do not install the idle-pressure +// collector; the runner drains finalizers as GCs queue them. +func spawnFinalizerRunner() { go finalizerRunner() } diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index 5970dae389..6c3d1f75a6 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -41,6 +41,13 @@ var ( sleepQueueBaseTime timeUnit ) +// finalizerIdleGC, when non-nil, is called at the scheduler's idle point to +// collect on finalizer-registration pressure (returning whether it did). It is +// installed lazily by the first SetFinalizer, so a program that never registers +// a finalizer never assigns it and the linker drops the whole collection path. +// It is nil under GCs without a finalizer table. +var finalizerIdleGC func() bool + // deadlock is called when a goroutine cannot proceed any more, but is in theory // not exited (so deferred calls won't run). This can happen for example in code // like this, that blocks forever: @@ -49,12 +56,18 @@ var ( // //go:noinline func deadlock() { + // A goroutine reaches deadlock when it can make no further progress. The + // common case by far is a goroutine that ran to completion: the compiler + // emits a deadlock call at the end of every goroutine wrapper. Flag it so + // the scheduler can reclaim the finished goroutine's stack. + task.MarkFinishing() // call yield without requesting a wakeup task.Pause() panic("unreachable") } func goexit() { + task.MarkFinishing() task.Exit() } @@ -183,6 +196,20 @@ func scheduler(returnAtDeadlock bool) { t := runqueue.Pop() if t == nil { + // Idle point: the run queue is drained, so no goroutine is running on + // its own stack. This is the safe place to reclaim external resources + // whose finalizers have piled up since the last collection. Running it + // here, once per drained run queue and only at the top level + // (task.Current() == nil, so a re-entrant call from a suspended + // goroutine does not collect while that goroutine is mid-operation), + // reclaims a completed run of goroutines' now-dead values in one pass. + // Forcing the collection inside alloc instead would scale GC frequency + // with allocation churn: a goroutine that allocates hundreds of + // short-lived finalized objects would trigger dozens of collections + // mid-run, most of them wasted on values that are still live. + if task.Current() == nil && finalizerIdleGC != nil && finalizerIdleGC() { + continue + } if sleepQueue == nil && timerQueue == nil { if returnAtDeadlock { return diff --git a/testdata/finalizeridle.go b/testdata/finalizeridle.go new file mode 100644 index 0000000000..cb232db483 --- /dev/null +++ b/testdata/finalizeridle.go @@ -0,0 +1,108 @@ +package main + +// Tests that the cooperative scheduler reclaims finalizer-guarded objects on its +// own, without an explicit runtime.GC(), once enough finalizers have been +// registered since the last collection. A registered finalizer usually guards an +// external resource whose Go-heap cost is tiny relative to what it pins, so the +// registration count drives a proactive collection at the scheduler's idle +// point. The second case additionally checks that a finished goroutine's stack +// no longer pins the objects its frames held. +// +// Like finalizer.go, this is only run on the precise wasm target (see the tests +// slice and the skip in main_test.go): there a dropped object is deterministically +// collected, so the finalizers fire predictably. It never calls runtime.GC(): the +// point is that the idle-point trigger collects on its own. + +import ( + "runtime" + "time" +) + +// batch must exceed the runtime's finalizer-registration threshold so the idle +// collection is guaranteed to trigger. +const batch = 64 + +var ( + ranDropped int + ranOnStack int + sink int +) + +// scrubStack overwrites the stack region used by an alloc-and-drop helper with +// non-pointer words. It is called at the same depth as that helper so this +// recursion reuses (and clears) the frame that just held the dropped pointers; +// otherwise a stale copy keeps an object marked and it is never collected. The +// returned value derived from buf keeps the writes live. +// +//go:noinline +func scrubStack(depth int) int { + if depth <= 0 { + return sink + } + var buf [64]int + for i := range buf { + buf[i] = depth + i + } + sink += buf[depth&63] + return scrubStack(depth-1) + buf[0] +} + +// registerAndDrop registers `batch` finalizers and returns without leaking any +// reference to the objects, so they become unreachable. The finalizer must not +// capture its object (that would pin it forever): it takes the pointer as its +// argument and touches only a package global. +// +//go:noinline +func registerAndDrop() { + for i := 0; i < batch; i++ { + p := new([2]int) + runtime.SetFinalizer(p, func(*[2]int) { ranDropped++ }) + } +} + +// testIdleCollect checks that registering many finalizers and then only parking +// the goroutine (time.Sleep, never runtime.GC()) is enough for the objects to be +// collected and their finalizers to run. +func testIdleCollect() { + registerAndDrop() + for i := 0; i < 500 && ranDropped < batch; i++ { + sink += scrubStack(40) + time.Sleep(time.Millisecond) + } + if ranDropped != batch { + panic("idle collection did not run every finalizer") + } +} + +// testFinishedGoroutineStacks checks that a goroutine which registers a finalizer +// on a stack-local object and then returns no longer pins that object: once the +// goroutine has finished, the idle collection reclaims the object. Without +// zeroing a finished goroutine's conservatively scanned stack, the stale pointer +// would keep the object alive. +func testFinishedGoroutineStacks() { + done := make(chan struct{}) + for i := 0; i < batch; i++ { + go func() { + p := new([2]int) + runtime.SetFinalizer(p, func(*[2]int) { ranOnStack++ }) + // p stays on this goroutine's stack until it returns just below. + done <- struct{}{} + }() + } + for i := 0; i < batch; i++ { + <-done + } + for i := 0; i < 500 && ranOnStack < batch; i++ { + sink += scrubStack(40) + time.Sleep(time.Millisecond) + } + if ranOnStack != batch { + panic("finished goroutine stack still pinned finalized objects") + } +} + +func main() { + testIdleCollect() + testFinishedGoroutineStacks() + println("ok") +} diff --git a/testdata/finalizeridle.txt b/testdata/finalizeridle.txt new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/testdata/finalizeridle.txt @@ -0,0 +1 @@ +ok From 58898cd2839c01482a5c24c6cbaef89d9d7373ee Mon Sep 17 00:00:00 2001 From: felipegenef Date: Thu, 23 Jul 2026 12:36:07 -0300 Subject: [PATCH 02/16] runtime: address review feedback on finalizer idle GC --- compileopts/finalizer_coverage_test.go | 70 +++++++++++++++++++++++++ src/internal/task/task_asyncify.go | 20 +++---- src/runtime/gc_finalizer.go | 13 ++++- src/runtime/gc_finalizer_sched_other.go | 13 +++-- 4 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 compileopts/finalizer_coverage_test.go diff --git a/compileopts/finalizer_coverage_test.go b/compileopts/finalizer_coverage_test.go new file mode 100644 index 0000000000..604d8b6de7 --- /dev/null +++ b/compileopts/finalizer_coverage_test.go @@ -0,0 +1,70 @@ +package compileopts + +import ( + "go/build/constraint" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestFinalizerRunnerSchedulerCoverage checks that the build constraints on the +// gc_finalizer_sched*.go files define spawnFinalizerRunner for exactly one file +// per scheduler. The three constraints must partition the scheduler space: every +// scheduler matches exactly one file, so none can be left with the symbol +// undefined or defined twice. It iterates validSchedulerOptions as the source of +// truth, so a newly added scheduler is covered by this check automatically. +func TestFinalizerRunnerSchedulerCoverage(t *testing.T) { + files := []string{ + "gc_finalizer_sched.go", + "gc_finalizer_sched_none.go", + "gc_finalizer_sched_other.go", + } + exprs := make([]constraint.Expr, len(files)) + for i, name := range files { + exprs[i] = readBuildConstraint(t, filepath.Join("..", "src", "runtime", name)) + } + + for _, sched := range validSchedulerOptions { + // The finalizer table exists under the block GCs; gc.conservative + // satisfies the "gc.conservative || gc.precise" half of every constraint. + tags := map[string]bool{ + "gc.conservative": true, + "scheduler." + sched: true, + } + var matched []string + for i, expr := range exprs { + if expr.Eval(func(tag string) bool { return tags[tag] }) { + matched = append(matched, files[i]) + } + } + if len(matched) != 1 { + t.Errorf("scheduler.%s: spawnFinalizerRunner defined in %d files %v, want exactly 1", + sched, len(matched), matched) + } + } +} + +// readBuildConstraint returns the parsed //go:build expression of a Go file. +func readBuildConstraint(t *testing.T, path string) constraint.Expr { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, line := range strings.Split(string(data), "\n") { + line = strings.TrimSpace(line) + if constraint.IsGoBuild(line) { + expr, err := constraint.Parse(line) + if err != nil { + t.Fatalf("%s: %v", path, err) + } + return expr + } + if line != "" && !strings.HasPrefix(line, "//") { + break // reached code before any //go:build line + } + } + t.Fatalf("%s: no //go:build line found", path) + return nil +} diff --git a/src/internal/task/task_asyncify.go b/src/internal/task/task_asyncify.go index 97b4635fe9..f9453ebc04 100644 --- a/src/internal/task/task_asyncify.go +++ b/src/internal/task/task_asyncify.go @@ -28,6 +28,12 @@ type state struct { stackState launched bool + + // finishing is set immediately before this goroutine, having run to + // completion, pauses for the last time. Resume observes it and clears the + // goroutine's stack. It lives on the task so each finishing goroutine owns + // its own flag, independent of scheduler timing. + finishing bool } // stackState is the saved state of a stack while unwound. @@ -92,16 +98,12 @@ func (s *state) initialize(fn uintptr, args unsafe.Pointer, stackSize uintptr) { //go:linkname memzero runtime.memzero func memzero(ptr unsafe.Pointer, size uintptr) -// finishing is set by the runtime immediately before a goroutine that has run -// to completion pauses for the last time. Resume observes it and clears the -// goroutine's stack. It is written and read on the same (cooperative) scheduler -// thread with no suspension point in between, so a plain global is safe. -var finishing bool - // MarkFinishing records that the current goroutine has finished and will not be // resumed, so Resume may reclaim its stack once control returns to the scheduler. +// The flag lives on the task itself, so each finishing goroutine owns its own and +// the handoff to Resume does not depend on scheduler timing. func MarkFinishing() { - finishing = true + currentTask.state.finishing = true } // clearStack zeroes a finished goroutine's entire stack buffer. The buffer is a @@ -160,11 +162,11 @@ func (t *Task) Resume() { if uintptr(t.state.asyncifysp) > uintptr(t.state.csp) { runtimePanic("stack overflow") } - if finishing { + if t.state.finishing { // The goroutine just ran to completion and paused for the last time. It // will never be resumed, so its stack can be cleared now to drop any // pointers its returned frames left behind (see clearStack). - finishing = false + t.state.finishing = false t.clearStack() } } diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index 279c5eb2a9..ee1afa6b69 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -42,8 +42,17 @@ type finalizerEntry struct { // finalizer draining) until the Go heap itself fills, which for a bursty, // mostly-idle workload may be never, so the external resources accumulate // without bound. Coupling a GC to finalizer-registration pressure caps that -// accumulation at roughly this many entries regardless of heap size. Zero -// disables the trigger. +// accumulation at roughly this many entries regardless of heap size. +// +// This is a compile-time policy constant, in the spirit of Go's forcegcperiod. +// The trigger only fires at the scheduler's idle point (a drained run queue) and +// each firing resets the count (see scanFinalizers), so it is throttled to that +// point rather than firing once per this-many registrations: a setup phase that +// registers many long-lived finalizers pays at most one extra collection at the +// first idle point after it, not one per threshold, and that collection just +// marks still-live data during otherwise-idle time without freeing anything +// early. Keeping it a const also lets the compiler constant-fold the check and, +// with zero, drop the pressure path entirely. Zero disables the trigger. const finalizerGCThreshold = 32 var ( diff --git a/src/runtime/gc_finalizer_sched_other.go b/src/runtime/gc_finalizer_sched_other.go index e087b37ec3..dbade9d323 100644 --- a/src/runtime/gc_finalizer_sched_other.go +++ b/src/runtime/gc_finalizer_sched_other.go @@ -2,7 +2,14 @@ package runtime -// Non-cooperative schedulers (cores, threads) spawn the finalizer runner but -// have no cooperative idle point, so they do not install the idle-pressure -// collector; the runner drains finalizers as GCs queue them. +// spawnFinalizerRunner is defined once per scheduler class, and the three build +// constraints partition the scheduler space exactly (exactly one scheduler.* tag +// is ever set): scheduler.none in gc_finalizer_sched_none.go, scheduler.tasks and +// scheduler.asyncify in gc_finalizer_sched.go, and every other variant here. This +// is the catch-all, so a new scheduler variant lands here and stays defined +// rather than falling through to an undefined reference. +// +// Non-cooperative schedulers (cores, threads) spawn the finalizer runner but have +// no cooperative idle point, so they do not install the idle-pressure collector; +// the runner drains finalizers as GCs queue them. func spawnFinalizerRunner() { go finalizerRunner() } From ee59cd99d974b4855171ada1679e75be9a4efd69 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Fri, 24 Jul 2026 19:06:17 -0300 Subject: [PATCH 03/16] runtime: clear a finished task's args pointer so its arguments are collectable --- src/internal/task/task_asyncify.go | 5 +++- testdata/finalizeridle.go | 40 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/internal/task/task_asyncify.go b/src/internal/task/task_asyncify.go index f9453ebc04..713bf1d898 100644 --- a/src/internal/task/task_asyncify.go +++ b/src/internal/task/task_asyncify.go @@ -165,9 +165,12 @@ func (t *Task) Resume() { if t.state.finishing { // The goroutine just ran to completion and paused for the last time. It // will never be resumed, so its stack can be cleared now to drop any - // pointers its returned frames left behind (see clearStack). + // pointers its returned frames left behind (see clearStack). The args + // bundle is likewise no longer needed, so drop that reference too, else + // any pointers in the arguments would keep their objects reachable. t.state.finishing = false t.clearStack() + t.state.args = nil } } diff --git a/testdata/finalizeridle.go b/testdata/finalizeridle.go index cb232db483..172b82bd60 100644 --- a/testdata/finalizeridle.go +++ b/testdata/finalizeridle.go @@ -25,6 +25,7 @@ const batch = 64 var ( ranDropped int ranOnStack int + ranInArgs int sink int ) @@ -101,8 +102,47 @@ func testFinishedGoroutineStacks() { } } +// launchArgGoroutine allocates a finalized object and launches a goroutine that +// receives it as an argument, then returns without leaving any reference behind. +// The object reaches the goroutine only through its argument bundle, and the +// only transient copies (of the pointer and the bundle) live in this frame, which +// returns immediately so the later scrubStack recursion reuses and clears it. +// +//go:noinline +func launchArgGoroutine(done chan struct{}) { + p := new([2]int) + runtime.SetFinalizer(p, func(*[2]int) { ranInArgs++ }) + go func(q *[2]int) { + sink += q[0] + done <- struct{}{} + }(p) +} + +// testFinishedGoroutineArgs checks that a goroutine which receives a finalized +// object as an argument no longer pins it once finished: the argument bundle the +// goroutine was launched with is dropped when it completes, so the idle collection +// reclaims the object. Without clearing a finished goroutine's args pointer the +// bundle would keep the object alive even after its stack has been zeroed. +func testFinishedGoroutineArgs() { + done := make(chan struct{}) + for i := 0; i < batch; i++ { + launchArgGoroutine(done) + } + for i := 0; i < batch; i++ { + <-done + } + for i := 0; i < 500 && ranInArgs < batch; i++ { + sink += scrubStack(40) + time.Sleep(time.Millisecond) + } + if ranInArgs != batch { + panic("finished goroutine args still pinned finalized objects") + } +} + func main() { testIdleCollect() testFinishedGoroutineStacks() + testFinishedGoroutineArgs() println("ok") } From d619b24d4707829b34a1f01ac20f51675892584e Mon Sep 17 00:00:00 2001 From: felipegenef Date: Sun, 26 Jul 2026 00:28:20 -0300 Subject: [PATCH 04/16] runtime: skip the finalizer scan with a per-block registration bit --- builder/sizes_test.go | 2 +- main_test.go | 25 +++-- src/runtime/gc_finalizer.go | 119 +++++++++++++++++++- testdata/finalizerbits.go | 212 ++++++++++++++++++++++++++++++++++++ testdata/finalizerbits.txt | 1 + 5 files changed, 345 insertions(+), 14 deletions(-) create mode 100644 testdata/finalizerbits.go create mode 100644 testdata/finalizerbits.txt diff --git a/builder/sizes_test.go b/builder/sizes_test.go index f6bb112fa0..5c5bc1e0be 100644 --- a/builder/sizes_test.go +++ b/builder/sizes_test.go @@ -44,7 +44,7 @@ func TestBinarySize(t *testing.T) { // microcontrollers {"hifive1b", "examples/echo", 4277, 307, 0, 2260}, {"microbit", "examples/serial", 2836, 368, 8, 2256}, - {"wioterminal", "examples/pininterrupt", 8013, 1663, 132, 7488}, + {"wioterminal", "examples/pininterrupt", 8013, 1667, 132, 7488}, // TODO: also check wasm. Right now this is difficult, because // wasm binaries are run through wasm-opt and therefore the diff --git a/main_test.go b/main_test.go index 220ae82074..1dc9fb675c 100644 --- a/main_test.go +++ b/main_test.go @@ -60,6 +60,7 @@ func TestBuild(t *testing.T) { "channel.go", "embed/", "finalizer.go", + "finalizerbits.go", "finalizeridle.go", "float.go", "gc.go", @@ -360,16 +361,20 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { continue } } - if (name == "finalizer.go" || name == "finalizeridle.go") && options.Target != "wasm" { - // runtime.SetFinalizer is implemented for the block GC, but these - // tests assert deterministic collection of a dropped object, which - // only holds on the GOOS=js wasm target. The host default GC is - // boehm (SetFinalizer is a no-op there); conservative stack scanning - // on the emulated targets can pin the object; and the wasip2 - // component entry lays out the stack differently, so collection is - // not deterministic on those. The feature still works on all of - // them, it just can't be golden-tested for firing. - continue + if options.Target != "wasm" { + switch name { + case "finalizer.go", "finalizerbits.go", "finalizeridle.go": + // runtime.SetFinalizer is implemented for the block GC, but the + // finalizer tests assert deterministic collection of a dropped + // object, which only holds on the GOOS=js wasm target. The host + // default GC is boehm (SetFinalizer is a no-op there); + // conservative stack scanning on the emulated targets can pin the + // object; and the wasip2 component entry lays out the stack + // differently, so collection is not deterministic on those. The + // feature still works on all of them, it just can't be + // golden-tested for firing. + continue + } } name := name // redefine to avoid race condition diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index ee1afa6b69..c0a0d2f0c6 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -78,6 +78,98 @@ var ( // finalizable object forever and the object could never be detected as dead. // Under the precise GC a plain uintptr field is not scanned anyway, so the // encoding is harmless there and required for the conservative build. +// finalizerGCDivisor scales the registration trigger with the size of the +// table: the next collection is due after roughly numFinalizers/this many new +// registrations, never fewer than finalizerGCThreshold. +const finalizerGCDivisor = 2 + +// finalizerGCTrigger returns how many registrations since the last collection +// are needed to run the next one. Each collection scans the whole table, which +// costs O(numFinalizers), so a trigger that stays constant while the table grows +// makes N registrations cost O(N^2) in scanning alone. Scaling the trigger with +// the table keeps the amortized scan cost per registration constant, the same +// reasoning behind Go's proportional GOGC pacing: collect when the tracked set +// has grown by a fraction of itself, not by a fixed count. +// +// The floor keeps the original behaviour for small tables, where a proportional +// trigger would fire too rarely to be useful. +func finalizerGCTrigger() uintptr { + if finalizerGCThreshold == 0 { + return 0 + } + if proportional := numFinalizers / finalizerGCDivisor; proportional > finalizerGCThreshold { + return proportional + } + return finalizerGCThreshold +} + +// finalizerBits records, one bit per heap block, whether the object starting at +// that block already has a registered finalizer. It answers the "is this object +// already registered?" question that SetFinalizer's replace semantics require +// without walking the table, so the common case (a fresh object, which is every +// syscall/js value) never scans anything. +// +// This mirrors what upstream Go gets from its per-span specials plus the +// arena-level "span has specials" bitmap: a constant-time way to skip objects +// that have nothing registered. +// +// The bitmap is allocated on the first registration and grown with the heap, so +// a program that never registers a finalizer keeps the whole feature dead. +var finalizerBits []byte + +// finalizerBitsNeeded is the bitmap length that covers the current heap. +func finalizerBitsNeeded() uintptr { return (uintptr(endBlock) + 7) / 8 } + +// growFinalizerBits allocates a wider bitmap if the heap outgrew the current +// one. It must run with gcLock released, because allocating takes gcLock. +func growFinalizerBits() []byte { + need := finalizerBitsNeeded() + if uintptr(len(finalizerBits)) >= need { + return nil + } + return make([]byte, need) +} + +// adoptFinalizerBits installs a wider bitmap under gcLock, carrying the old bits +// over. A nil or already-obsolete buffer is ignored. +func adoptFinalizerBits(buf []byte) { + if len(buf) <= len(finalizerBits) { + return + } + copy(buf, finalizerBits) + finalizerBits = buf +} + +func finalizerBitIndex(addr uintptr) uintptr { return uintptr(blockFromAddr(addr)) } + +func finalizerBitGet(addr uintptr) bool { + i := finalizerBitIndex(addr) + if i/8 >= uintptr(len(finalizerBits)) { + // The bitmap does not describe this address yet (the heap grew since it + // was sized). Answer conservatively: a spurious "maybe" only costs one + // scan, while a wrong "no" would let a second entry be registered for an + // object that already has one, and its finalizer would run twice. + return true + } + return finalizerBits[i/8]&(1<<(i%8)) != 0 +} + +func finalizerBitSet(addr uintptr) { + i := finalizerBitIndex(addr) + if i/8 >= uintptr(len(finalizerBits)) { + return + } + finalizerBits[i/8] |= 1 << (i % 8) +} + +func finalizerBitClear(addr uintptr) { + i := finalizerBitIndex(addr) + if i/8 >= uintptr(len(finalizerBits)) { + return + } + finalizerBits[i/8] &^= 1 << (i % 8) +} + func encodeFinalizerPtr(addr uintptr) uintptr { return ^addr } func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } @@ -89,9 +181,18 @@ func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } func registerFinalizer(addr uintptr, fn interface{}) { enc := encodeFinalizerPtr(addr) + tracked := isOnHeap(addr) + if fn == nil { - // Clear: remove every registration for this object. + // Clear: remove every registration for this object. The bit proves in + // one test that there is nothing to remove. + if tracked && !finalizerBitGet(addr) { + return + } gcLock.Lock() + if tracked { + finalizerBitClear(addr) + } prev := &finalizers for n := *prev; n != nil; n = *prev { if n.obj == enc { @@ -108,8 +209,13 @@ func registerFinalizer(addr uintptr, fn interface{}) { // Register or replace. The allocation happens before gcLock is taken, // because alloc acquires gcLock itself. entry := &finalizerEntry{obj: enc, fn: fn} + wider := growFinalizerBits() gcLock.Lock() - for n := finalizers; n != nil; n = n.next { + adoptFinalizerBits(wider) + // Only an object whose bit is set can already be in the table, so a fresh + // object skips the scan entirely. An address the bitmap cannot describe + // (not on the heap) always scans, as before. + for n := finalizers; (!tracked || finalizerBitGet(addr)) && n != nil; n = n.next { if n.obj == enc { // Replace the finalizer for an already-registered object, so it // still runs only once (Go SetFinalizer replace semantics). @@ -128,6 +234,9 @@ func registerFinalizer(addr uintptr, fn interface{}) { } entry.next = finalizers finalizers = entry + if tracked { + finalizerBitSet(addr) + } numFinalizers++ finalizersSinceGC++ // pressure signal for the proactive GC trigger at the scheduler's idle point // A finalizer is registered, so make sure the runner exists. The flag is @@ -175,6 +284,9 @@ func scanFinalizers() { // and into the pending queue (alloc-free), so its finalizer runs once. *prev = n.next numFinalizers-- + // The object is gone; clear its bit so a later object reusing the + // address starts clean. + finalizerBitClear(addr) n.next = finalizerPending finalizerPending = n finalizersQueued = true @@ -269,7 +381,8 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { // operation's values are still live, which would scale GC frequency with // allocation churn and waste most collections on still-live values. func finalizerPressureGC() bool { - if finalizerGCThreshold == 0 || finalizersSinceGC < finalizerGCThreshold { + trigger := finalizerGCTrigger() + if trigger == 0 || finalizersSinceGC < trigger { return false } gcLock.Lock() diff --git a/testdata/finalizerbits.go b/testdata/finalizerbits.go new file mode 100644 index 0000000000..5ab10de32c --- /dev/null +++ b/testdata/finalizerbits.go @@ -0,0 +1,212 @@ +package main + +// Tests the registration bookkeeping behind runtime.SetFinalizer on the block +// GC: the per-block bit that records whether an object already has a finalizer. +// The bit is what lets a fresh object skip the registered-finalizer scan, so +// these cases pin the invariants that skipping must never break: +// +// - an object whose finalizer was cleared and then registered again still runs +// it exactly once, so clearing resets the bookkeeping; +// - registering twice replaces, it never leaves two registrations behind +// (which would run the finalizer twice); +// - churning register/clear on one object leaves no residue; +// - memory reused by a later object registers correctly, so a dead object's +// bookkeeping does not leak onto whatever lands at its address next; +// - a batch where only some objects keep a finalizer runs exactly those. +// +// Like finalizer.go, this is only run on the precise wasm target (see the tests +// slice and the skip in main_test.go): there a dropped object is deterministically +// collected, so the finalizers fire predictably. +// +// Each test calls its alloc helper and scrubStack at the same call depth, so the +// recursion reuses and clears the frame that just held the dropped pointers. + +import "runtime" + +type box struct{ x int } + +const batch = 32 + +var ( + reregisteredRan int + replacedOldRan int + replacedNewRan int + churnRan int + reuseFirstRan int + reuseSecondRan int + keptRan int + droppedRan int + sink int +) + +// scrubStack overwrites the stack region used by an alloc-and-drop helper with +// non-pointer words. It must be called at the same call depth as that helper so +// this recursion reuses (and clears) the frame that just held the dropped +// pointer; otherwise a stale copy keeps the object marked and it is never +// collected. The returned value derived from buf keeps the writes live. +// +//go:noinline +func scrubStack(depth int) int { + if depth <= 0 { + return sink + } + var buf [64]int + for i := range buf { + buf[i] = depth + i + } + sink += buf[depth&63] + return scrubStack(depth-1) + buf[0] +} + +//go:noinline +func allocClearThenRegister() { + p := &box{x: 1} + runtime.SetFinalizer(p, func(*box) { panic("cleared finalizer ran") }) + runtime.SetFinalizer(p, nil) + runtime.SetFinalizer(p, func(*box) { reregisteredRan++ }) +} + +// testClearThenRegister checks that clearing a finalizer and registering a new +// one leaves exactly the new one: clearing has to reset the bookkeeping, not +// just unlink the entry. +func testClearThenRegister() { + allocClearThenRegister() + for i := 0; i < 200 && reregisteredRan == 0; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if reregisteredRan != 1 { + panic("finalizerbits: re-registered finalizer did not run exactly once") + } +} + +//go:noinline +func allocRegisterTwice() { + for i := 0; i < batch; i++ { + p := &box{x: i} + runtime.SetFinalizer(p, func(*box) { replacedOldRan++ }) + runtime.SetFinalizer(p, func(*box) { replacedNewRan++ }) + } +} + +// testRegisterTwiceLeavesOne checks the replace path over a whole batch: the +// second registration must find the first one and take its place. A missed +// lookup would leave two registrations for the same object, and its finalizer +// would run twice. +func testRegisterTwiceLeavesOne() { + allocRegisterTwice() + for i := 0; i < 200 && replacedNewRan < batch; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if replacedOldRan != 0 { + panic("finalizerbits: replaced finalizer still ran") + } + if replacedNewRan != batch { + panic("finalizerbits: replacement did not run exactly once per object") + } +} + +//go:noinline +func allocChurn() { + p := &box{x: 3} + for i := 0; i < 64; i++ { + runtime.SetFinalizer(p, func(*box) { churnRan++ }) + runtime.SetFinalizer(p, nil) + } +} + +// testChurnLeavesNothing checks that many register/clear rounds on one object +// leave nothing behind: the object dies with no finalizer, so nothing runs. +func testChurnLeavesNothing() { + allocChurn() + for i := 0; i < 200; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if churnRan != 0 { + panic("finalizerbits: churned register/clear left a live registration") + } +} + +//go:noinline +func allocFirstRound() { + for i := 0; i < batch; i++ { + p := &box{x: i} + runtime.SetFinalizer(p, func(*box) { reuseFirstRan++ }) + } +} + +//go:noinline +func allocSecondRound() { + for i := 0; i < batch; i++ { + p := &box{x: i} + runtime.SetFinalizer(p, func(*box) { reuseSecondRan++ }) + } +} + +// testAddressReuse checks that objects allocated into memory freed by a previous +// finalized batch register correctly themselves. A dead object's bookkeeping must +// not survive onto whatever lands at its address next. +func testAddressReuse() { + allocFirstRound() + for i := 0; i < 200 && reuseFirstRan < batch; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if reuseFirstRan != batch { + panic("finalizerbits: first round did not run every finalizer") + } + allocSecondRound() + for i := 0; i < 200 && reuseSecondRan < batch; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if reuseSecondRan != batch { + panic("finalizerbits: second round into reused memory lost finalizers") + } +} + +//go:noinline +func allocMixedBatch() { + for i := 0; i < batch; i++ { + p := &box{x: i} + if i%2 == 0 { + runtime.SetFinalizer(p, func(*box) { droppedRan++ }) + runtime.SetFinalizer(p, nil) + } else { + runtime.SetFinalizer(p, func(*box) { keptRan++ }) + } + } +} + +// testMixedBatch checks that clearing some registrations inside a batch affects +// only those objects: the ones still registered run, the cleared ones do not. +func testMixedBatch() { + allocMixedBatch() + for i := 0; i < 200 && keptRan < batch/2; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if droppedRan != 0 { + panic("finalizerbits: a cleared finalizer inside the batch ran") + } + if keptRan != batch/2 { + panic("finalizerbits: kept finalizers did not all run exactly once") + } +} + +func main() { + testClearThenRegister() + testRegisterTwiceLeavesOne() + testChurnLeavesNothing() + testAddressReuse() + testMixedBatch() + println("ok") +} diff --git a/testdata/finalizerbits.txt b/testdata/finalizerbits.txt new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/testdata/finalizerbits.txt @@ -0,0 +1 @@ +ok From 45ce61c8bb79c357d3a7229533e0a5c9f11918d3 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Sun, 2 Aug 2026 17:57:27 -0300 Subject: [PATCH 05/16] runtime: guard the finalizer registration bitmap with gcLock --- src/runtime/gc_finalizer.go | 70 +++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index c0a0d2f0c6..2374cce87d 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -73,11 +73,6 @@ var ( finalizerRunnerStarted bool ) -// The object address is stored bitwise-NOT so it never looks like a live heap -// pointer to the conservative scanner. Otherwise the entry would pin every -// finalizable object forever and the object could never be detected as dead. -// Under the precise GC a plain uintptr field is not scanned anyway, so the -// encoding is harmless there and required for the conservative build. // finalizerGCDivisor scales the registration trigger with the size of the // table: the next collection is due after roughly numFinalizers/this many new // registrations, never fewer than finalizerGCThreshold. @@ -115,23 +110,33 @@ func finalizerGCTrigger() uintptr { // // The bitmap is allocated on the first registration and grown with the heap, so // a program that never registers a finalizer keeps the whole feature dead. +// +// Every access goes through gcLock, including the reads. The slice header itself +// is replaced when the heap grows, so an unlocked reader on a parallel scheduler +// (cores, threads) could observe a stale bit, or tear the header and index the +// old, shorter buffer with the new length. var finalizerBits []byte -// finalizerBitsNeeded is the bitmap length that covers the current heap. -func finalizerBitsNeeded() uintptr { return (uintptr(endBlock) + 7) / 8 } - -// growFinalizerBits allocates a wider bitmap if the heap outgrew the current -// one. It must run with gcLock released, because allocating takes gcLock. -func growFinalizerBits() []byte { - need := finalizerBitsNeeded() +// finalizerBitsShortfall returns the bitmap length needed to cover the current +// heap, or zero if the current bitmap already covers it. It must be called under +// gcLock: that is what makes reading finalizerBits and endBlock safe against a +// concurrent adoptFinalizerBits on another core. The caller then allocates with +// the lock released (allocating takes gcLock) and installs the result with +// adoptFinalizerBits. +func finalizerBitsShortfall() uintptr { + need := (uintptr(endBlock) + 7) / 8 if uintptr(len(finalizerBits)) >= need { - return nil + return 0 } - return make([]byte, need) + return need } // adoptFinalizerBits installs a wider bitmap under gcLock, carrying the old bits -// over. A nil or already-obsolete buffer is ignored. +// over. A nil or already-obsolete buffer is ignored, which is what makes it safe +// for the heap to have grown again (or another core to have installed its own +// wider bitmap) while the caller was allocating with the lock released. A buffer +// that covers less than the current heap is still an improvement: addresses past +// its end just keep answering conservatively in finalizerBitGet. func adoptFinalizerBits(buf []byte) { if len(buf) <= len(finalizerBits) { return @@ -170,6 +175,11 @@ func finalizerBitClear(addr uintptr) { finalizerBits[i/8] &^= 1 << (i % 8) } +// The object address is stored bitwise-NOT so it never looks like a live heap +// pointer to the conservative scanner. Otherwise the entry would pin every +// finalizable object forever and the object could never be detected as dead. +// Under the precise GC a plain uintptr field is not scanned anyway, so the +// encoding is harmless there and required for the conservative build. func encodeFinalizerPtr(addr uintptr) uintptr { return ^addr } func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } @@ -181,15 +191,20 @@ func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } func registerFinalizer(addr uintptr, fn interface{}) { enc := encodeFinalizerPtr(addr) - tracked := isOnHeap(addr) - if fn == nil { // Clear: remove every registration for this object. The bit proves in - // one test that there is nothing to remove. + // one test that there is nothing to remove, but only while gcLock is + // held: a registration on another core may be setting that same bit (and + // replacing the bitmap) right now, and a stale read of zero would skip + // the removal and leave the finalizer registered on a live object. + // Holding the lock for the check costs nothing extra, because the removal + // below needs it anyway; what the bit saves is the O(numFinalizers) walk. + gcLock.Lock() + tracked := isOnHeap(addr) if tracked && !finalizerBitGet(addr) { + gcLock.Unlock() return } - gcLock.Lock() if tracked { finalizerBitClear(addr) } @@ -206,12 +221,21 @@ func registerFinalizer(addr uintptr, fn interface{}) { return } - // Register or replace. The allocation happens before gcLock is taken, - // because alloc acquires gcLock itself. + // Register or replace. Allocating acquires gcLock, so the entry is allocated + // before the lock is taken and a wider bitmap is allocated by dropping the + // lock for just that call. Only a heap that outgrew the bitmap pays that + // round trip; the common case holds the lock once, and adoptFinalizerBits + // tolerates the heap having grown again (or another core having installed a + // wider bitmap) while this one was allocating. entry := &finalizerEntry{obj: enc, fn: fn} - wider := growFinalizerBits() gcLock.Lock() - adoptFinalizerBits(wider) + if shortfall := finalizerBitsShortfall(); shortfall != 0 { + gcLock.Unlock() + wider := make([]byte, shortfall) + gcLock.Lock() + adoptFinalizerBits(wider) + } + tracked := isOnHeap(addr) // Only an object whose bit is set can already be in the table, so a fresh // object skips the scan entirely. An address the bitmap cannot describe // (not on the heap) always scans, as before. From 91d43ecca3309b3002a6aa90ea0548c3f46889a0 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Sun, 2 Aug 2026 19:30:00 -0300 Subject: [PATCH 06/16] testdata: cover finalizer invariants on every scheduler --- main_test.go | 47 +++++++++-- testdata/finalizerinvariants.go | 139 +++++++++++++++++++++++++++++++ testdata/finalizerinvariants.txt | 1 + 3 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 testdata/finalizerinvariants.go create mode 100644 testdata/finalizerinvariants.txt diff --git a/main_test.go b/main_test.go index 34851258b3..e3059dd636 100644 --- a/main_test.go +++ b/main_test.go @@ -62,6 +62,7 @@ func TestBuild(t *testing.T) { "finalizer.go", "finalizerbits.go", "finalizeridle.go", + "finalizerinvariants.go", "float.go", "gc.go", "generics.go", @@ -369,15 +370,26 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { if options.Target != "wasm" { switch name { case "finalizer.go", "finalizerbits.go", "finalizeridle.go": - // runtime.SetFinalizer is implemented for the block GC, but the - // finalizer tests assert deterministic collection of a dropped - // object, which only holds on the GOOS=js wasm target. The host - // default GC is boehm (SetFinalizer is a no-op there); - // conservative stack scanning on the emulated targets can pin the - // object; and the wasip2 component entry lays out the stack - // differently, so collection is not deterministic on those. The - // feature still works on all of them, it just can't be - // golden-tested for firing. + // runtime.SetFinalizer is implemented for the block GC, but these + // tests assert deterministic collection of a dropped object, + // which only holds on the GOOS=js wasm target. The host default + // GC is boehm (SetFinalizer is a no-op there); conservative stack + // scanning on the emulated targets can pin the object; and the + // wasip2 component entry lays out the stack differently, so + // collection is not deterministic on those. The feature still + // works on all of them, it just can't be golden-tested for + // firing, which is what finalizerinvariants.go covers instead. + continue + } + } + if options.Target == "simavr" { + switch name { + case "finalizerinvariants.go": + // Finalizers are detected by the GC, and gc.go is already skipped + // on AVR for its high mark false positive rate (see the simavr + // switch above). Registering and clearing a finalizer works + // there, but a single runtime.GC() call does not return, so this + // test inherits that limitation rather than adding a new one. continue } } @@ -402,6 +414,23 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { runTest("alias.go", options, t, nil, nil) }) } + if options.Target == "" { + // The host default GC is boehm, where SetFinalizer is unimplemented, so + // the plain host run of finalizerinvariants.go passes without exercising + // anything. Re-run it on the block GC to cover the two schedulers no + // other target in this suite reaches: threads (the host default) and + // none. Together with cortex-m-qemu (tasks), riscv-qemu (cores) and the + // wasm targets (asyncify), that covers every scheduler variant. + for _, scheduler := range []string{"threads", "none"} { + t.Run("finalizerinvariants.go-gc-conservative-scheduler-"+scheduler, func(t *testing.T) { + t.Parallel() + options := compileopts.Options(options) + options.GC = "conservative" + options.Scheduler = scheduler + runTest("finalizerinvariants.go", options, t, nil, nil) + }) + } + } if options.Target == "" || isWASI { t.Run("filesystem.go", func(t *testing.T) { t.Parallel() diff --git a/testdata/finalizerinvariants.go b/testdata/finalizerinvariants.go new file mode 100644 index 0000000000..afd0874a90 --- /dev/null +++ b/testdata/finalizerinvariants.go @@ -0,0 +1,139 @@ +package main + +// Invariants of runtime.SetFinalizer that hold on every target the block GC +// supports, not only the ones where a dropped object is deterministically +// collected. +// +// finalizer.go, finalizerbits.go and finalizeridle.go all assert that a +// finalizer fired, which needs the dropped object to actually be collected, so +// they only run on wasm (see the skip in main_test.go). Conservative stack +// scanning elsewhere can keep a dropped object alive and the finalizer then +// correctly does not run. +// +// The opposite direction is portable: a conservative collector only ever +// over-retains, never under-retains, so "this finalizer must never run" holds +// on every target. Those are exactly the invariants the per-block registration +// bitmap can break, because a wrong bit skips the table walk that the clear and +// replace semantics of SetFinalizer depend on. A stale or torn bit therefore +// shows up here as a finalizer that runs when it must not. + +import "runtime" + +type obj struct{ x int } + +const batch = 8 + +var ( + clearedRan int + replacedRan int + reachedRan int + ranTwice int + seen [batch]int + reachable []*obj + sink int +) + +// scrubStack overwrites the stack region used by an alloc-and-drop helper with +// non-pointer words, so a stale frame does not keep the dropped object marked. +// It must be called at the same call depth as those helpers. +// +//go:noinline +func scrubStack(depth int) int { + if depth <= 0 { + return sink + } + var buf [16]int + for i := range buf { + buf[i] = depth + i + } + sink += buf[depth&15] + return scrubStack(depth-1) + buf[0] +} + +// dropCleared registers a finalizer, clears it, then drops the object. Clearing +// must remove the registration, so this finalizer may never run. +// +//go:noinline +func dropCleared() { + p := &obj{} + runtime.SetFinalizer(p, func(*obj) { clearedRan++ }) + runtime.SetFinalizer(p, nil) +} + +// dropReplaced registers a finalizer and then replaces it. Registering twice +// must replace rather than accumulate, so the first func may never run and the +// second may run at most once. +// +//go:noinline +func dropReplaced(id int) { + p := &obj{} + runtime.SetFinalizer(p, func(*obj) { replacedRan++ }) + runtime.SetFinalizer(p, func(*obj) { + seen[id]++ + if seen[id] > 1 { + ranTwice++ + } + }) +} + +// keepReachable registers a finalizer on an object held by a global. A +// reachable object must never be finalized. +// +//go:noinline +func keepReachable(id int) { + p := &obj{x: id} + runtime.SetFinalizer(p, func(*obj) { reachedRan++ }) + reachable = append(reachable, p) +} + +func main() { + for i := 0; i < batch; i++ { + dropCleared() + } + scrubStack(12) + + for i := 0; i < batch; i++ { + dropReplaced(i) + } + scrubStack(12) + + for i := 0; i < batch; i++ { + keepReachable(i) + } + + // Collect repeatedly, yielding so the finalizer runner goroutine gets to + // drain anything that was queued. + for i := 0; i < 4; i++ { + runtime.GC() + runtime.Gosched() + } + scrubStack(12) + for i := 0; i < 4; i++ { + runtime.GC() + runtime.Gosched() + } + + // Touch the reachable set after the collections so it stays a live root + // across all of them. + total := 0 + for _, p := range reachable { + total += p.x + } + if total != batch*(batch-1)/2 { + println("FAIL: reachable set corrupted:", total) + return + } + + switch { + case clearedRan != 0: + println("FAIL: cleared finalizer ran:", clearedRan) + case replacedRan != 0: + println("FAIL: replaced finalizer ran:", replacedRan) + case reachedRan != 0: + println("FAIL: reachable object was finalized:", reachedRan) + case ranTwice != 0: + println("FAIL: finalizer ran more than once:", ranTwice) + default: + println("ok") + } +} diff --git a/testdata/finalizerinvariants.txt b/testdata/finalizerinvariants.txt new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/testdata/finalizerinvariants.txt @@ -0,0 +1 @@ +ok From 363b90190c5a6080fcb14490bf6871d94615799d Mon Sep 17 00:00:00 2001 From: felipegenef Date: Sun, 2 Aug 2026 20:01:13 -0300 Subject: [PATCH 07/16] main_test: limit the finalizer scheduler variants to linux and darwin --- main_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/main_test.go b/main_test.go index e3059dd636..5ccf3fd058 100644 --- a/main_test.go +++ b/main_test.go @@ -414,13 +414,23 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { runTest("alias.go", options, t, nil, nil) }) } - if options.Target == "" { + buildGOOS := options.GOOS + if buildGOOS == "" { + buildGOOS = runtime.GOOS + } + if options.Target == "" && (buildGOOS == "linux" || buildGOOS == "darwin") { // The host default GC is boehm, where SetFinalizer is unimplemented, so // the plain host run of finalizerinvariants.go passes without exercising // anything. Re-run it on the block GC to cover the two schedulers no // other target in this suite reaches: threads (the host default) and // none. Together with cortex-m-qemu (tasks), riscv-qemu (cores) and the // wasm targets (asyncify), that covers every scheduler variant. + // + // Restricted to linux and darwin: internal/task only defines threadID + // for those two, so scheduler.threads does not build anywhere else, and + // scheduler.none does not link on Windows either. Both predate this test + // (they reproduce with any testdata file), so this skips rather than + // works around them. Same reasoning as TestTimerStopResetRace above. for _, scheduler := range []string{"threads", "none"} { t.Run("finalizerinvariants.go-gc-conservative-scheduler-"+scheduler, func(t *testing.T) { t.Parallel() From 69c945bed2eb376069b739f7a83f5aba8b09d6c2 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Sun, 2 Aug 2026 21:12:17 -0300 Subject: [PATCH 08/16] testdata: wait for the finalizer queue to drain before asserting --- main_test.go | 19 +++++-- testdata/finalizerinvariants.go | 92 +++++++++++++++++++++++++++++---- 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/main_test.go b/main_test.go index 5ccf3fd058..9dddc06912 100644 --- a/main_test.go +++ b/main_test.go @@ -382,6 +382,15 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { continue } } + if options.Target == "" && options.GC == "" { + switch name { + case "finalizerinvariants.go": + // The default GC on these is boehm, where SetFinalizer is + // unimplemented, so there is nothing to assert. The explicit + // -gc=conservative variants below cover the host instead. + continue + } + } if options.Target == "simavr" { switch name { case "finalizerinvariants.go": @@ -420,11 +429,11 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { } if options.Target == "" && (buildGOOS == "linux" || buildGOOS == "darwin") { // The host default GC is boehm, where SetFinalizer is unimplemented, so - // the plain host run of finalizerinvariants.go passes without exercising - // anything. Re-run it on the block GC to cover the two schedulers no - // other target in this suite reaches: threads (the host default) and - // none. Together with cortex-m-qemu (tasks), riscv-qemu (cores) and the - // wasm targets (asyncify), that covers every scheduler variant. + // the plain host run of finalizerinvariants.go is skipped above. Run it + // on the block GC instead, which also covers the two schedulers no other + // target in this suite reaches: threads (the host default) and none. + // Together with cortex-m-qemu (tasks), riscv-qemu (cores) and the wasm + // targets (asyncify), that covers every scheduler variant. // // Restricted to linux and darwin: internal/task only defines threadID // for those two, so scheduler.threads does not build anywhere else, and diff --git a/testdata/finalizerinvariants.go b/testdata/finalizerinvariants.go index afd0874a90..dcc330fddb 100644 --- a/testdata/finalizerinvariants.go +++ b/testdata/finalizerinvariants.go @@ -17,7 +17,10 @@ package main // replace semantics of SetFinalizer depend on. A stale or torn bit therefore // shows up here as a finalizer that runs when it must not. -import "runtime" +import ( + "runtime" + "time" +) type obj struct{ x int } @@ -86,6 +89,67 @@ func keepReachable(id int) { reachable = append(reachable, p) } +// finalizerRuns is the total number of finalizer invocations observed so far, +// across every counter. Individual counters are asserted on at the end; this +// sum exists only to tell "the runner is still working" from "the queue is +// empty". +func finalizerRuns() int { + n := clearedRan + replacedRan + reachedRan + ranTwice + for _, s := range seen { + n += s + } + return n +} + +// Bounds for drainFinalizers. quietRounds is how many consecutive rounds must +// observe no new invocation before the queue counts as drained; maxRounds caps +// a target that never runs a finalizer at all, which the vacuity check in main +// then reports. +// +// Measured, every target here drains in 4 rounds and then 3, including +// scheduler.threads, so maxRounds is headroom for a loaded machine rather than +// an expected cost: the loop exits on quiescence long before reaching it. +const ( + quietRounds = 3 + maxRounds = 50 +) + +// drainFinalizers collects until the finalizer queue is drained, and returns +// only once it is. +// +// It waits on the observable result rather than on a fixed delay. Gosched does +// not synchronize with the runner under scheduler.threads, where it is a no-op +// (every goroutine is its own thread, so there is nothing to yield to) and the +// runner is a separate thread blocked on a futex. Sleeping a fixed amount would +// only make the race less likely; polling until invocations stop arriving is +// what actually establishes that the queue is empty. The sleep below is the +// poll interval, not the wait. +// +// Quiescence alone is not enough to start with, because a runner that has not +// been scheduled yet looks identical to a drained queue. So the quiet rounds +// only count once at least one finalizer has run. +func drainFinalizers() { + quiet := 0 + for i := 0; i < maxRounds; i++ { + before := finalizerRuns() + runtime.GC() + runtime.Gosched() + time.Sleep(time.Millisecond) + switch { + case finalizerRuns() != before: + quiet = 0 + case before == 0: + // Nothing has run yet: cannot tell a drained queue from a runner + // that has not started, so keep polling. + default: + quiet++ + if quiet == quietRounds { + return + } + } + } +} + func main() { for i := 0; i < batch; i++ { dropCleared() @@ -101,17 +165,9 @@ func main() { keepReachable(i) } - // Collect repeatedly, yielding so the finalizer runner goroutine gets to - // drain anything that was queued. - for i := 0; i < 4; i++ { - runtime.GC() - runtime.Gosched() - } + drainFinalizers() scrubStack(12) - for i := 0; i < 4; i++ { - runtime.GC() - runtime.Gosched() - } + drainFinalizers() // Touch the reachable set after the collections so it stays a live root // across all of them. @@ -124,7 +180,21 @@ func main() { return } + // Count the replacement finalizers that ran. The assertions below are all of + // the "must never run" kind, so they are only meaningful if something was + // collected and drained at all: with nothing collected they hold trivially + // and the test reports success while checking nothing. Requiring at least + // one firing turns that silent pass into a failure. It stays at "at least + // one" rather than "all", because a conservative stack scan is allowed to + // pin any individual object. + replacementsRan := 0 + for _, n := range seen { + replacementsRan += n + } + switch { + case replacementsRan == 0: + println("FAIL: no finalizer ran at all, the assertions below prove nothing") case clearedRan != 0: println("FAIL: cleared finalizer ran:", clearedRan) case replacedRan != 0: From 37e70970735c1e64e5233368e1ebaf70ac9aa3ed Mon Sep 17 00:00:00 2001 From: felipegenef Date: Mon, 3 Aug 2026 11:04:53 -0300 Subject: [PATCH 09/16] testdata: make the finalizer counters atomic and wait for a known drain count --- testdata/finalizerinvariants.go | 140 +++++++++++++------------------- 1 file changed, 58 insertions(+), 82 deletions(-) diff --git a/testdata/finalizerinvariants.go b/testdata/finalizerinvariants.go index dcc330fddb..fd31e2579f 100644 --- a/testdata/finalizerinvariants.go +++ b/testdata/finalizerinvariants.go @@ -19,6 +19,7 @@ package main import ( "runtime" + "sync/atomic" "time" ) @@ -26,12 +27,16 @@ type obj struct{ x int } const batch = 8 +// The counters are written by finalizers and read by main. Under +// scheduler.threads and scheduler.cores those are different threads running at +// the same time, so every access goes through sync/atomic rather than a plain +// int. var ( - clearedRan int - replacedRan int - reachedRan int - ranTwice int - seen [batch]int + clearedRan atomic.Int32 + replacedRan atomic.Int32 + reachedRan atomic.Int32 + ranTwice atomic.Int32 + seen [batch]atomic.Int32 reachable []*obj sink int ) @@ -59,7 +64,7 @@ func scrubStack(depth int) int { //go:noinline func dropCleared() { p := &obj{} - runtime.SetFinalizer(p, func(*obj) { clearedRan++ }) + runtime.SetFinalizer(p, func(*obj) { clearedRan.Add(1) }) runtime.SetFinalizer(p, nil) } @@ -70,11 +75,10 @@ func dropCleared() { //go:noinline func dropReplaced(id int) { p := &obj{} - runtime.SetFinalizer(p, func(*obj) { replacedRan++ }) + runtime.SetFinalizer(p, func(*obj) { replacedRan.Add(1) }) runtime.SetFinalizer(p, func(*obj) { - seen[id]++ - if seen[id] > 1 { - ranTwice++ + if seen[id].Add(1) > 1 { + ranTwice.Add(1) } }) } @@ -85,69 +89,55 @@ func dropReplaced(id int) { //go:noinline func keepReachable(id int) { p := &obj{x: id} - runtime.SetFinalizer(p, func(*obj) { reachedRan++ }) + runtime.SetFinalizer(p, func(*obj) { reachedRan.Add(1) }) reachable = append(reachable, p) } -// finalizerRuns is the total number of finalizer invocations observed so far, -// across every counter. Individual counters are asserted on at the end; this -// sum exists only to tell "the runner is still working" from "the queue is -// empty". -func finalizerRuns() int { - n := clearedRan + replacedRan + reachedRan + ranTwice - for _, s := range seen { - n += s +// replacementsRan is how many of the batch replacement finalizers have run. +func replacementsRan() int { + n := 0 + for i := range seen { + n += int(seen[i].Load()) } return n } -// Bounds for drainFinalizers. quietRounds is how many consecutive rounds must -// observe no new invocation before the queue counts as drained; maxRounds caps -// a target that never runs a finalizer at all, which the vacuity check in main -// then reports. -// -// Measured, every target here drains in 4 rounds and then 3, including -// scheduler.threads, so maxRounds is headroom for a loaded machine rather than -// an expected cost: the loop exits on quiescence long before reaching it. -const ( - quietRounds = 3 - maxRounds = 50 -) +// maxRounds bounds waitForDrain. Measured, every target here reaches the full +// count within a handful of rounds, so this is headroom for a loaded machine +// rather than an expected cost. +const maxRounds = 500 -// drainFinalizers collects until the finalizer queue is drained, and returns -// only once it is. +// waitForDrain collects until every replacement finalizer has run, and reports +// whether it got there. // -// It waits on the observable result rather than on a fixed delay. Gosched does -// not synchronize with the runner under scheduler.threads, where it is a no-op -// (every goroutine is its own thread, so there is nothing to yield to) and the -// runner is a separate thread blocked on a futex. Sleeping a fixed amount would -// only make the race less likely; polling until invocations stop arriving is -// what actually establishes that the queue is empty. The sleep below is the -// poll interval, not the wait. +// It waits for a known count rather than for the queue to look idle. Idleness +// cannot be observed from here: runtime exposes no way to ask whether the +// finalizer queue is empty, and under scheduler.threads the runner is a +// separate thread, so a stretch with no new invocation is indistinguishable +// from a runner that has simply not been scheduled yet. Waiting for a specific +// number of invocations has no such ambiguity, and not reaching it is a test +// failure rather than a silently short wait. // -// Quiescence alone is not enough to start with, because a runner that has not -// been scheduled yet looks identical to a drained queue. So the quiet rounds -// only count once at least one finalizer has run. -func drainFinalizers() { - quiet := 0 +// batch is the right target because these objects are allocated and dropped +// inside a //go:noinline helper whose frame scrubStack then overwrites, so +// nothing is left pointing at them for a conservative scan to find. +func waitForDrain() bool { for i := 0; i < maxRounds; i++ { - before := finalizerRuns() runtime.GC() runtime.Gosched() time.Sleep(time.Millisecond) - switch { - case finalizerRuns() != before: - quiet = 0 - case before == 0: - // Nothing has run yet: cannot tell a drained queue from a runner - // that has not started, so keep polling. - default: - quiet++ - if quiet == quietRounds { - return - } + if replacementsRan() == batch { + // The runner has worked through the queue these objects were in. Do + // one more pass so that a finalizer which must NOT run, but which a + // wrong bitmap bit left registered, is queued and drained here + // instead of after the counters are read. + runtime.GC() + runtime.Gosched() + time.Sleep(time.Millisecond) + return true } } + return false } func main() { @@ -165,9 +155,7 @@ func main() { keepReachable(i) } - drainFinalizers() - scrubStack(12) - drainFinalizers() + drained := waitForDrain() // Touch the reachable set after the collections so it stays a live root // across all of them. @@ -180,29 +168,17 @@ func main() { return } - // Count the replacement finalizers that ran. The assertions below are all of - // the "must never run" kind, so they are only meaningful if something was - // collected and drained at all: with nothing collected they hold trivially - // and the test reports success while checking nothing. Requiring at least - // one firing turns that silent pass into a failure. It stays at "at least - // one" rather than "all", because a conservative stack scan is allowed to - // pin any individual object. - replacementsRan := 0 - for _, n := range seen { - replacementsRan += n - } - switch { - case replacementsRan == 0: - println("FAIL: no finalizer ran at all, the assertions below prove nothing") - case clearedRan != 0: - println("FAIL: cleared finalizer ran:", clearedRan) - case replacedRan != 0: - println("FAIL: replaced finalizer ran:", replacedRan) - case reachedRan != 0: - println("FAIL: reachable object was finalized:", reachedRan) - case ranTwice != 0: - println("FAIL: finalizer ran more than once:", ranTwice) + case !drained: + println("FAIL: only", replacementsRan(), "of", batch, "replacement finalizers ran, the assertions below prove nothing") + case clearedRan.Load() != 0: + println("FAIL: cleared finalizer ran:", clearedRan.Load()) + case replacedRan.Load() != 0: + println("FAIL: replaced finalizer ran:", replacedRan.Load()) + case reachedRan.Load() != 0: + println("FAIL: reachable object was finalized:", reachedRan.Load()) + case ranTwice.Load() != 0: + println("FAIL: finalizer ran more than once:", ranTwice.Load()) default: println("ok") } From a6e58daf266803a80e1c97239bd69eeb13dab2c2 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Tue, 4 Aug 2026 17:31:23 -0300 Subject: [PATCH 10/16] runtime: add finalizer bookkeeping asserts under runtime_asserts --- src/runtime/gc_finalizer.go | 80 ++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index 2374cce87d..103b9208bb 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -183,6 +183,38 @@ func finalizerBitClear(addr uintptr) { func encodeFinalizerPtr(addr uintptr) uintptr { return ^addr } func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } +// finalizerRegistered reports whether the table holds an entry for enc. This is +// the linear answer the registration bitmap exists to avoid, so it is only used +// under gcAsserts, to check the bitmap against the table it summarizes. +func finalizerRegistered(enc uintptr) bool { + for n := finalizers; n != nil; n = n.next { + if n.obj == enc { + return true + } + } + return false +} + +// assertFinalizerTable verifies the bookkeeping that the table, the counter and +// the registration bitmap have to agree on. A registered entry without its bit +// is the dangerous direction: the clear and replace paths trust a clear bit to +// mean "nothing registered" and skip the table walk, so a missing bit turns +// SetFinalizer(obj, nil) into a silent no-op and lets a re-registration add a +// second entry, which runs the finalizer twice. Only called under gcAsserts. +func assertFinalizerTable() { + var count uintptr + for n := finalizers; n != nil; n = n.next { + count++ + addr := decodeFinalizerPtr(n.obj) + if isOnHeap(addr) && !finalizerBitGet(addr) { + runtimeFatal("gc: registered finalizer without its bitmap bit") + } + } + if count != numFinalizers { + runtimeFatal("gc: numFinalizers does not match the finalizer table") + } +} + // registerFinalizer records fn as the finalizer for the object at addr. A nil fn // removes any registration for the object. Growing the table (allocating a node) // is the only allocation and it happens here, on the caller, never during GC. @@ -202,6 +234,11 @@ func registerFinalizer(addr uintptr, fn interface{}) { gcLock.Lock() tracked := isOnHeap(addr) if tracked && !finalizerBitGet(addr) { + // Taking this shortcut on a stale bit would silently skip the + // removal, so check the answer against the table it stands in for. + if gcAsserts && finalizerRegistered(enc) { + runtimeFatal("gc: finalizer bit clear but the object is registered") + } gcLock.Unlock() return } @@ -236,6 +273,11 @@ func registerFinalizer(addr uintptr, fn interface{}) { adoptFinalizerBits(wider) } tracked := isOnHeap(addr) + // Skipping the scan on a stale bit would add a second entry for an object + // that already has one, and its finalizer would then run twice. + if gcAsserts && tracked && !finalizerBitGet(addr) && finalizerRegistered(enc) { + runtimeFatal("gc: finalizer bit clear but the object is registered") + } // Only an object whose bit is set can already be in the table, so a fresh // object skips the scan entirely. An address the bitmap cannot describe // (not on the heap) always scans, as before. @@ -325,13 +367,33 @@ func scanFinalizers() { // both serialized under gcLock. var resurrected bool for n := finalizerPending; n != nil; n = n.next { - markRoot(0, decodeFinalizerPtr(n.obj)) + addr := decodeFinalizerPtr(n.obj) + if gcAsserts && !isOnHeap(addr) { + runtimeFatal("gc: pending finalizer for an object off the heap") + } + markRoot(0, addr) resurrected = true } if resurrected { // Re-scan so objects reachable only from resurrected objects also // survive this sweep. finishMark() + if gcAsserts { + // Every pending object must have survived the resurrection above. + // One that did not is about to be swept while its finalizer is + // still queued, which is a use-after-free in callFinalizer. + for n := finalizerPending; n != nil; n = n.next { + // Inside the collection, so the resurrected object is expected + // to carry the mark state rather than plain head. + if blockFromAddr(decodeFinalizerPtr(n.obj)).state() != blockStateMark { + runtimeFatal("gc: pending finalizer object was not resurrected") + } + } + } + } + + if gcAsserts { + assertFinalizerTable() } } @@ -384,7 +446,21 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { var objPtr unsafe.Pointer if n != nil { finalizerPending = n.next - objPtr = unsafe.Pointer(decodeFinalizerPtr(n.obj)) + addr := decodeFinalizerPtr(n.obj) + if gcAsserts { + if !isOnHeap(addr) { + runtimeFatal("gc: dequeued finalizer for an object off the heap") + } + // scanFinalizers resurrects everything still pending, so sweep must + // have left the object allocated. A freed block here means + // callFinalizer is about to run on memory that is back in the free + // list. The mark bit is not the thing to check: this runs outside a + // collection, where unmark has already turned mark back into head. + if blockFromAddr(addr).state() == blockStateFree { + runtimeFatal("gc: dequeued finalizer for a freed object") + } + } + objPtr = unsafe.Pointer(addr) } gcLock.Unlock() return n, objPtr From fc0fe94e7a6b02b70eca633050edf1953a9ef47d Mon Sep 17 00:00:00 2001 From: felipegenef Date: Thu, 20 Aug 2026 16:59:48 -0300 Subject: [PATCH 11/16] runtime: address finalizer GC review feedback --- compiler/goroutine.go | 12 +- compiler/testdata/goroutine-wasm-asyncify.ll | 12 +- compiler/testdata/large.ll | 4 +- main_test.go | 55 ++++---- src/internal/task/task_finishing_tasks.go | 2 +- src/runtime/gc_finalizer.go | 30 +++-- src/runtime/scheduler_cooperative.go | 25 +++- testdata/finalizeridle.go | 81 ++++++++++-- testdata/finalizerinvariants.go | 129 +++---------------- 9 files changed, 169 insertions(+), 181 deletions(-) diff --git a/compiler/goroutine.go b/compiler/goroutine.go index 26d489a3a4..497c3a25bf 100644 --- a/compiler/goroutine.go +++ b/compiler/goroutine.go @@ -309,10 +309,10 @@ func (c *compilerContext) createGoroutineStartWrapper(fnType llvm.Type, fn llvm. } defer b.Dispose() - var deadlock llvm.Value - var deadlockType llvm.Type + var exitGoroutine llvm.Value + var exitGoroutineType llvm.Type if c.Scheduler == "asyncify" { - deadlockType, deadlock = c.getFunction(c.program.ImportedPackage("runtime").Members["deadlock"].(*ssa.Function)) + exitGoroutineType, exitGoroutine = c.getFunction(c.program.ImportedPackage("runtime").Members["exitGoroutine"].(*ssa.Function)) } if !fn.IsAFunction().IsNil() { @@ -377,7 +377,7 @@ func (c *compilerContext) createGoroutineStartWrapper(fnType llvm.Type, fn llvm. b.CreateCall(fnType, fn, params, "") if c.Scheduler == "asyncify" { - b.CreateCall(deadlockType, deadlock, []llvm.Value{ + b.CreateCall(exitGoroutineType, exitGoroutine, []llvm.Value{ llvm.Undef(c.dataPtrType), }, "") } @@ -528,14 +528,14 @@ func (c *compilerContext) createGoroutineStartWrapper(fnType llvm.Type, fn llvm. b.CreateCall(fnType, fnPtr, params, "") if c.Scheduler == "asyncify" { - b.CreateCall(deadlockType, deadlock, []llvm.Value{ + b.CreateCall(exitGoroutineType, exitGoroutine, []llvm.Value{ llvm.Undef(c.dataPtrType), }, "") } } if c.Scheduler == "asyncify" { - // The goroutine was terminated via deadlock. + // The goroutine was terminated via exitGoroutine. b.CreateUnreachable() } else { // Finish the function. Every basic block must end in a terminator, and diff --git a/compiler/testdata/goroutine-wasm-asyncify.ll b/compiler/testdata/goroutine-wasm-asyncify.ll index 0c3f2f7073..062ec1a423 100644 --- a/compiler/testdata/goroutine-wasm-asyncify.ll +++ b/compiler/testdata/goroutine-wasm-asyncify.ll @@ -22,14 +22,14 @@ entry: declare void @main.regularFunction(i32, ptr) #0 -declare void @runtime.deadlock(ptr) #0 +declare void @runtime.exitGoroutine(ptr) #0 ; Function Attrs: nounwind define linkonce_odr void @"main.regularFunction$gowrapper"(ptr %0) unnamed_addr #2 { entry: %unpack.int = ptrtoint ptr %0 to i32 call void @main.regularFunction(i32 %unpack.int, ptr undef) #11 - call void @runtime.deadlock(ptr undef) #11 + call void @runtime.exitGoroutine(ptr undef) #11 unreachable } @@ -53,7 +53,7 @@ define linkonce_odr void @"main.inlineFunctionGoroutine$1$gowrapper"(ptr %0) unn entry: %unpack.int = ptrtoint ptr %0 to i32 call void @"main.inlineFunctionGoroutine$1"(i32 %unpack.int, ptr undef) - call void @runtime.deadlock(ptr undef) #11 + call void @runtime.exitGoroutine(ptr undef) #11 unreachable } @@ -96,7 +96,7 @@ entry: %2 = getelementptr inbounds nuw i8, ptr %0, i32 4 %3 = load ptr, ptr %2, align 4 call void @"main.closureFunctionGoroutine$1"(i32 %1, ptr %3) - call void @runtime.deadlock(ptr undef) #11 + call void @runtime.exitGoroutine(ptr undef) #11 unreachable } @@ -130,7 +130,7 @@ entry: %4 = getelementptr inbounds nuw i8, ptr %0, i32 8 %5 = load ptr, ptr %4, align 4 call void %5(i32 %1, ptr %3) #11 - call void @runtime.deadlock(ptr undef) #11 + call void @runtime.exitGoroutine(ptr undef) #11 unreachable } @@ -193,7 +193,7 @@ entry: %6 = getelementptr inbounds nuw i8, ptr %0, i32 12 %7 = load ptr, ptr %6, align 4 call void @"interface:{Print:func:{basic:string}{}}.Print$invoke"(ptr %1, ptr %3, i32 %5, ptr %7, ptr undef) #11 - call void @runtime.deadlock(ptr undef) #11 + call void @runtime.exitGoroutine(ptr undef) #11 unreachable } diff --git a/compiler/testdata/large.ll b/compiler/testdata/large.ll index 27da846060..d0a2085478 100644 --- a/compiler/testdata/large.ll +++ b/compiler/testdata/large.ll @@ -203,13 +203,13 @@ entry: ret void } -declare void @runtime.deadlock(ptr) #0 +declare void @runtime.exitGoroutine(ptr) #0 ; Function Attrs: nounwind define linkonce_odr void @"main.readLargeValue$gowrapper"(ptr %0) unnamed_addr #6 { entry: %1 = call i8 @main.readLargeValue(ptr %0, ptr undef) - call void @runtime.deadlock(ptr undef) #9 + call void @runtime.exitGoroutine(ptr undef) #9 unreachable } diff --git a/main_test.go b/main_test.go index 9dddc06912..98dfca12cc 100644 --- a/main_test.go +++ b/main_test.go @@ -119,7 +119,24 @@ func TestBuild(t *testing.T) { t.Run("Host", func(t *testing.T) { t.Parallel() - runPlatTests(optionsFromTarget("", sema), tests, t) + hostOptions := optionsFromTarget("", sema) + runPlatTests(hostOptions, tests, t) + + // Exercise the host-only schedulers with a GC that implements finalizers. + switch runtime.GOOS { + case "darwin", "linux": + for _, scheduler := range []string{"threads", "none"} { + scheduler := scheduler + t.Run("finalizerinvariants.go-gc-conservative-scheduler-"+scheduler, func(t *testing.T) { + t.Parallel() + options := compileopts.Options(hostOptions) + options.GC = "conservative" + options.Scheduler = scheduler + options.Tags = append(append([]string(nil), hostOptions.Tags...), "runtime_asserts") + runTest("finalizerinvariants.go", options, t, nil, nil) + }) + } + } }) // Test a few build options. @@ -387,7 +404,7 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { case "finalizerinvariants.go": // The default GC on these is boehm, where SetFinalizer is // unimplemented, so there is nothing to assert. The explicit - // -gc=conservative variants below cover the host instead. + // -gc=conservative host variants cover it instead. continue } } @@ -406,7 +423,12 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { name := name // redefine to avoid race condition t.Run(name, func(t *testing.T) { t.Parallel() - runTest(name, options, t, nil, nil) + testOptions := compileopts.Options(options) + if name == "finalizerinvariants.go" { + // Exercise finalizer bookkeeping assertions during registration and GC. + testOptions.Tags = append(append([]string(nil), options.Tags...), "runtime_asserts") + } + runTest(name, testOptions, t, nil, nil) }) } if !strings.HasPrefix(spec.Emulator, "simavr ") { @@ -423,33 +445,6 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { runTest("alias.go", options, t, nil, nil) }) } - buildGOOS := options.GOOS - if buildGOOS == "" { - buildGOOS = runtime.GOOS - } - if options.Target == "" && (buildGOOS == "linux" || buildGOOS == "darwin") { - // The host default GC is boehm, where SetFinalizer is unimplemented, so - // the plain host run of finalizerinvariants.go is skipped above. Run it - // on the block GC instead, which also covers the two schedulers no other - // target in this suite reaches: threads (the host default) and none. - // Together with cortex-m-qemu (tasks), riscv-qemu (cores) and the wasm - // targets (asyncify), that covers every scheduler variant. - // - // Restricted to linux and darwin: internal/task only defines threadID - // for those two, so scheduler.threads does not build anywhere else, and - // scheduler.none does not link on Windows either. Both predate this test - // (they reproduce with any testdata file), so this skips rather than - // works around them. Same reasoning as TestTimerStopResetRace above. - for _, scheduler := range []string{"threads", "none"} { - t.Run("finalizerinvariants.go-gc-conservative-scheduler-"+scheduler, func(t *testing.T) { - t.Parallel() - options := compileopts.Options(options) - options.GC = "conservative" - options.Scheduler = scheduler - runTest("finalizerinvariants.go", options, t, nil, nil) - }) - } - } if options.Target == "" || isWASI { t.Run("filesystem.go", func(t *testing.T) { t.Parallel() diff --git a/src/internal/task/task_finishing_tasks.go b/src/internal/task/task_finishing_tasks.go index 5ba351a58d..0802f52951 100644 --- a/src/internal/task/task_finishing_tasks.go +++ b/src/internal/task/task_finishing_tasks.go @@ -6,5 +6,5 @@ package task // goroutine's stack to drop the stale pointers its returned frames leave behind // is only implemented for the asyncify scheduler, whose goroutine stacks are // heap buffers scanned conservatively (see the asyncify MarkFinishing and -// Resume). deadlock and goexit in the cooperative scheduler call this for both. +// Resume). exitGoroutine and goexit in the cooperative scheduler call this for both. func MarkFinishing() {} diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index 103b9208bb..e4aa931eca 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -33,16 +33,17 @@ type finalizerEntry struct { fn interface{} } -// finalizerGCThreshold bounds how many finalizers may be registered since the -// last collection before the scheduler proactively runs one at its idle point. +// finalizerGCThreshold is the minimum net registration pressure accumulated +// since the last collection before the scheduler runs one at its idle point. +// finalizerGCTrigger raises this floor proportionally for larger tables. // A registered finalizer almost always guards an external resource, most // importantly a syscall/js bridge-table slot (js.Value or js.Func), that costs // only a few bytes of Go heap but pins a whole JS object and its slot. Without // this, a long-lived instance with a large resident heap defers GC (and thus // finalizer draining) until the Go heap itself fills, which for a bursty, // mostly-idle workload may be never, so the external resources accumulate -// without bound. Coupling a GC to finalizer-registration pressure caps that -// accumulation at roughly this many entries regardless of heap size. +// without bound. Coupling a GC to finalizer-registration pressure bounds that +// accumulation. // // This is a compile-time policy constant, in the spirit of Go's forcegcperiod. // The trigger only fires at the scheduler's idle point (a drained run queue) and @@ -59,7 +60,7 @@ var ( finalizers *finalizerEntry // registered finalizers; a GC root that keeps fn values alive finalizerPending *finalizerEntry // finalizers whose object died, waiting to run numFinalizers uintptr // number of registered finalizers; fast-path gate for scanFinalizers - finalizersSinceGC uintptr // finalizers registered since the last GC; drives the scheduler idle-point pressure trigger + finalizersSinceGC uintptr // net registration pressure since the last GC; drives the scheduler idle-point trigger finalizersQueued bool // set when scanFinalizers queued at least one finalizer to run finalizerFutex task.Futex // wakes the finalizerRunner goroutine after a GC queues work finalizerDraining bool // guards against re-entrant inline draining (scheduler.none) @@ -74,12 +75,12 @@ var ( ) // finalizerGCDivisor scales the registration trigger with the size of the -// table: the next collection is due after roughly numFinalizers/this many new -// registrations, never fewer than finalizerGCThreshold. +// table: the next collection is due after roughly numFinalizers/this much net +// pressure, never less than finalizerGCThreshold. const finalizerGCDivisor = 2 -// finalizerGCTrigger returns how many registrations since the last collection -// are needed to run the next one. Each collection scans the whole table, which +// finalizerGCTrigger returns how much net registration pressure is needed to run +// the next collection. Each collection scans the whole table, which // costs O(numFinalizers), so a trigger that stays constant while the table grows // makes N registrations cost O(N^2) in scanning alone. Scaling the trigger with // the table keeps the amortized scan cost per registration constant, the same @@ -250,6 +251,11 @@ func registerFinalizer(addr uintptr, fn interface{}) { if n.obj == enc { *prev = n.next numFinalizers-- + // Clearing offsets net registration pressure. Saturate at zero + // because the cleared entry may predate the last collection. + if finalizersSinceGC != 0 { + finalizersSinceGC-- + } } else { prev = &n.next } @@ -466,9 +472,9 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { return n, objPtr } -// finalizerPressureGC collects when at least finalizerGCThreshold finalizers -// have been registered since the last GC, then hands any freshly-queued -// finalizers to the runner. It reports whether it collected. A registered +// finalizerPressureGC collects when net registration pressure since the last GC +// reaches finalizerGCTrigger, then hands any freshly-queued finalizers to the +// runner. It reports whether it collected. A registered // finalizer almost always guards an external resource whose Go-heap cost is tiny // (a few bytes) relative to what it pins, so the registration count is a proxy // for external memory pressure that the heap-size GC trigger cannot see. diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index e31e0caa73..426017054c 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -39,6 +39,7 @@ var ( runqueue task.Queue sleepQueue *task.Task sleepQueueBaseTime timeUnit + deadlockedTasks task.Queue ) // finalizerIdleGC, when non-nil, is called at the scheduler's idle point to @@ -56,18 +57,28 @@ var finalizerIdleGC func() bool // //go:noinline func deadlock() { - // A goroutine reaches deadlock when it can make no further progress. The - // common case by far is a goroutine that ran to completion: the compiler - // emits a deadlock call at the end of every goroutine wrapper. Flag it so - // the scheduler can reclaim the finished goroutine's stack. - task.MarkFinishing() - // call yield without requesting a wakeup + // Keep permanently blocked tasks reachable so their suspended stacks remain + // GC roots, but never put them back on the runnable queue. + deadlockedTasks.Push(task.Current()) + task.Pause() + runtimeFatal("unreachable") +} + +// exitGoroutine is called by asyncify goroutine wrappers after the wrapped +// function returns. Unlike deadlock, this path means the task is truly finished +// and will never be resumed. +func exitGoroutine() { + if finalizerIdleGC != nil { + task.MarkFinishing() + } task.Pause() runtimeFatal("unreachable") } func goexit() { - task.MarkFinishing() + if finalizerIdleGC != nil { + task.MarkFinishing() + } task.Exit() } diff --git a/testdata/finalizeridle.go b/testdata/finalizeridle.go index 172b82bd60..e29fcd1e91 100644 --- a/testdata/finalizeridle.go +++ b/testdata/finalizeridle.go @@ -1,20 +1,19 @@ package main -// Tests that the cooperative scheduler reclaims finalizer-guarded objects on its -// own, without an explicit runtime.GC(), once enough finalizers have been -// registered since the last collection. A registered finalizer usually guards an -// external resource whose Go-heap cost is tiny relative to what it pins, so the -// registration count drives a proactive collection at the scheduler's idle -// point. The second case additionally checks that a finished goroutine's stack -// no longer pins the objects its frames held. +// Tests idle finalizer collection and goroutine-stack lifetime on precise wasm. +// The idle-collection cases verify that registration pressure triggers a +// collection without an explicit runtime.GC. The permanently blocked goroutine +// cases use explicit GCs and a control finalizer to distinguish live stacks from +// stalled GC progress. // // Like finalizer.go, this is only run on the precise wasm target (see the tests // slice and the skip in main_test.go): there a dropped object is deterministically -// collected, so the finalizers fire predictably. It never calls runtime.GC(): the -// point is that the idle-point trigger collects on its own. +// collected, so the finalizers fire predictably. The idle-collection cases do +// not call runtime.GC: their purpose is to prove the idle trigger itself works. import ( "runtime" + "sync/atomic" "time" ) @@ -27,8 +26,71 @@ var ( ranOnStack int ranInArgs int sink int + + blockedRan [3]atomic.Int32 + controlRan atomic.Int32 ) +type blockedObject struct{ x int } + +//go:noinline +func blockOperation(kind int, ready chan<- struct{}, ch chan struct{}) { + ready <- struct{}{} + switch kind { + case 0: + select {} + case 1: + ch <- struct{}{} + case 2: + <-ch + } +} + +//go:noinline +func holdWhileBlocked(kind int, ready chan<- struct{}, ch chan struct{}) { + p := &blockedObject{x: kind} + runtime.SetFinalizer(p, func(*blockedObject) { blockedRan[kind].Add(1) }) + blockOperation(kind, ready, ch) + // blockOperation can return, so p remains live on this suspended stack. + runtime.KeepAlive(p) +} + +//go:noinline +func dropProgressControl() { + p := &blockedObject{x: 8} + runtime.SetFinalizer(p, func(*blockedObject) { controlRan.Add(1) }) +} + +// testPermanentlyBlockedStacks verifies that permanent blocks preserve their +// suspended stacks. The control object is intentionally unreachable and is +// deterministic on precise wasm; once its finalizer runs, GC and finalizer +// progress are proven without sleeps. A blocked-object finalizer running by +// then therefore means its task was incorrectly treated as completed. +func testPermanentlyBlockedStacks() { + ready := make(chan struct{}, 3) + go holdWhileBlocked(0, ready, nil) // select{} + go holdWhileBlocked(1, ready, nil) // nil-channel send + go holdWhileBlocked(2, ready, nil) // nil-channel receive + <-ready + <-ready + <-ready + + dropProgressControl() + for i := 0; i < 100 && controlRan.Load() == 0; i++ { + sink += scrubStack(40) + runtime.GC() + runtime.Gosched() + } + if controlRan.Load() != 1 { + panic("control finalizer did not prove GC progress") + } + for i, name := range [...]string{"select{}", "nil-channel send", "nil-channel receive"} { + if blockedRan[i].Load() != 0 { + panic(name + " stack-held object was finalized") + } + } +} + // scrubStack overwrites the stack region used by an alloc-and-drop helper with // non-pointer words. It is called at the same depth as that helper so this // recursion reuses (and clears) the frame that just held the dropped pointers; @@ -141,6 +203,7 @@ func testFinishedGoroutineArgs() { } func main() { + testPermanentlyBlockedStacks() testIdleCollect() testFinishedGoroutineStacks() testFinishedGoroutineArgs() diff --git a/testdata/finalizerinvariants.go b/testdata/finalizerinvariants.go index fd31e2579f..8492d16dbc 100644 --- a/testdata/finalizerinvariants.go +++ b/testdata/finalizerinvariants.go @@ -1,36 +1,29 @@ package main -// Invariants of runtime.SetFinalizer that hold on every target the block GC -// supports, not only the ones where a dropped object is deterministically -// collected. +// Portable negative invariants of runtime.SetFinalizer on the block GCs. +// Conservative stack scanning may retain an unreachable object indefinitely, +// so this test never requires a dropped object's finalizer to run. On targets +// that do collect one, its callbacks reject invalid behavior: cleared and +// replaced finalizers running, reachable objects being finalized, or one +// registration running more than once. // -// finalizer.go, finalizerbits.go and finalizeridle.go all assert that a -// finalizer fired, which needs the dropped object to actually be collected, so -// they only run on wasm (see the skip in main_test.go). Conservative stack -// scanning elsewhere can keep a dropped object alive and the finalizer then -// correctly does not run. -// -// The opposite direction is portable: a conservative collector only ever -// over-retains, never under-retains, so "this finalizer must never run" holds -// on every target. Those are exactly the invariants the per-block registration -// bitmap can break, because a wrong bit skips the table walk that the clear and -// replace semantics of SetFinalizer depend on. A stale or torn bit therefore -// shows up here as a finalizer that runs when it must not. +// The harness builds this file with runtime_asserts. Those checks +// deterministically validate clear/replace behavior and agreement between the +// finalizer table, count, and registration bitmap during these operations and +// collections. Bookkeeping coverage therefore does not depend on a conservative +// collector reclaiming any particular dropped object. import ( "runtime" "sync/atomic" - "time" ) type obj struct{ x int } const batch = 8 -// The counters are written by finalizers and read by main. Under -// scheduler.threads and scheduler.cores those are different threads running at -// the same time, so every access goes through sync/atomic rather than a plain -// int. +// Finalizers may run concurrently with main under the threads and cores +// schedulers, so all observations shared with a finalizer are atomic. var ( clearedRan atomic.Int32 replacedRan atomic.Int32 @@ -38,29 +31,8 @@ var ( ranTwice atomic.Int32 seen [batch]atomic.Int32 reachable []*obj - sink int ) -// scrubStack overwrites the stack region used by an alloc-and-drop helper with -// non-pointer words, so a stale frame does not keep the dropped object marked. -// It must be called at the same call depth as those helpers. -// -//go:noinline -func scrubStack(depth int) int { - if depth <= 0 { - return sink - } - var buf [16]int - for i := range buf { - buf[i] = depth + i - } - sink += buf[depth&15] - return scrubStack(depth-1) + buf[0] -} - -// dropCleared registers a finalizer, clears it, then drops the object. Clearing -// must remove the registration, so this finalizer may never run. -// //go:noinline func dropCleared() { p := &obj{} @@ -68,10 +40,6 @@ func dropCleared() { runtime.SetFinalizer(p, nil) } -// dropReplaced registers a finalizer and then replaces it. Registering twice -// must replace rather than accumulate, so the first func may never run and the -// second may run at most once. -// //go:noinline func dropReplaced(id int) { p := &obj{} @@ -83,9 +51,6 @@ func dropReplaced(id int) { }) } -// keepReachable registers a finalizer on an object held by a global. A -// reachable object must never be finalized. -// //go:noinline func keepReachable(id int) { p := &obj{x: id} @@ -93,72 +58,22 @@ func keepReachable(id int) { reachable = append(reachable, p) } -// replacementsRan is how many of the batch replacement finalizers have run. -func replacementsRan() int { - n := 0 - for i := range seen { - n += int(seen[i].Load()) - } - return n -} - -// maxRounds bounds waitForDrain. Measured, every target here reaches the full -// count within a handful of rounds, so this is headroom for a loaded machine -// rather than an expected cost. -const maxRounds = 500 - -// waitForDrain collects until every replacement finalizer has run, and reports -// whether it got there. -// -// It waits for a known count rather than for the queue to look idle. Idleness -// cannot be observed from here: runtime exposes no way to ask whether the -// finalizer queue is empty, and under scheduler.threads the runner is a -// separate thread, so a stretch with no new invocation is indistinguishable -// from a runner that has simply not been scheduled yet. Waiting for a specific -// number of invocations has no such ambiguity, and not reaching it is a test -// failure rather than a silently short wait. -// -// batch is the right target because these objects are allocated and dropped -// inside a //go:noinline helper whose frame scrubStack then overwrites, so -// nothing is left pointing at them for a conservative scan to find. -func waitForDrain() bool { - for i := 0; i < maxRounds; i++ { - runtime.GC() - runtime.Gosched() - time.Sleep(time.Millisecond) - if replacementsRan() == batch { - // The runner has worked through the queue these objects were in. Do - // one more pass so that a finalizer which must NOT run, but which a - // wrong bitmap bit left registered, is queued and drained here - // instead of after the counters are read. - runtime.GC() - runtime.Gosched() - time.Sleep(time.Millisecond) - return true - } - } - return false -} - func main() { for i := 0; i < batch; i++ { dropCleared() - } - scrubStack(12) - - for i := 0; i < batch; i++ { dropReplaced(i) - } - scrubStack(12) - - for i := 0; i < batch; i++ { keepReachable(i) } - drained := waitForDrain() + // Exercise scan-time bookkeeping assertions and give finalizers bounded + // opportunities to run on targets which collect the dropped objects. No + // assertion below requires one of them to have been collected. + for i := 0; i < 8; i++ { + runtime.GC() + runtime.Gosched() + } - // Touch the reachable set after the collections so it stays a live root - // across all of them. + // Keep the reachable objects live across every collection above. total := 0 for _, p := range reachable { total += p.x @@ -169,8 +84,6 @@ func main() { } switch { - case !drained: - println("FAIL: only", replacementsRan(), "of", batch, "replacement finalizers ran, the assertions below prove nothing") case clearedRan.Load() != 0: println("FAIL: cleared finalizer ran:", clearedRan.Load()) case replacedRan.Load() != 0: From abb41c23df030dd23b065d20531a228954a0b68d Mon Sep 17 00:00:00 2001 From: felipegenef Date: Thu, 20 Aug 2026 18:17:08 -0300 Subject: [PATCH 12/16] testdata: strengthen blocked stack finalizer test --- testdata/finalizeridle.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testdata/finalizeridle.go b/testdata/finalizeridle.go index e29fcd1e91..0e5a7b10c0 100644 --- a/testdata/finalizeridle.go +++ b/testdata/finalizeridle.go @@ -84,6 +84,10 @@ func testPermanentlyBlockedStacks() { if controlRan.Load() != 1 { panic("control finalizer did not prove GC progress") } + // Collect once more so transient scheduler roots cannot mask an unrooted + // blocked task during the progress-control collection. + runtime.GC() + runtime.Gosched() for i, name := range [...]string{"select{}", "nil-channel send", "nil-channel receive"} { if blockedRan[i].Load() != 0 { panic(name + " stack-held object was finalized") From 8a2f85343d560c7d62e99053149170d998eafe19 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Thu, 20 Aug 2026 20:09:45 -0300 Subject: [PATCH 13/16] runtime: fix finalizer cleanup edge cases --- main_test.go | 31 ++++++++++++++++++--- src/runtime/gc_finalizer.go | 14 +++++----- src/runtime/runtime_wasmentry.go | 17 ------------ src/runtime/scheduler_cooperative.go | 40 ++++++++++++++++++++++++---- testdata/finalizerlarge.go | 29 ++++++++++++++++++++ testdata/finalizerlarge.txt | 1 + testdata/wasmexport-finalizer.go | 27 +++++++++++++++++++ testdata/wasmexport-finalizer.js | 32 ++++++++++++++++++++++ 8 files changed, 159 insertions(+), 32 deletions(-) create mode 100644 testdata/finalizerlarge.go create mode 100644 testdata/finalizerlarge.txt create mode 100644 testdata/wasmexport-finalizer.go create mode 100644 testdata/wasmexport-finalizer.js diff --git a/main_test.go b/main_test.go index 98dfca12cc..c671c1b37d 100644 --- a/main_test.go +++ b/main_test.go @@ -63,6 +63,7 @@ func TestBuild(t *testing.T) { "finalizerbits.go", "finalizeridle.go", "finalizerinvariants.go", + "finalizerlarge.go", "float.go", "gc.go", "generics.go", @@ -386,7 +387,7 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { } if options.Target != "wasm" { switch name { - case "finalizer.go", "finalizerbits.go", "finalizeridle.go": + case "finalizer.go", "finalizerbits.go", "finalizeridle.go", "finalizerlarge.go": // runtime.SetFinalizer is implemented for the block GC, but these // tests assert deterministic collection of a dropped object, // which only holds on the GOOS=js wasm target. The host default @@ -424,8 +425,8 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() testOptions := compileopts.Options(options) - if name == "finalizerinvariants.go" { - // Exercise finalizer bookkeeping assertions during registration and GC. + if name == "finalizerinvariants.go" || name == "finalizerlarge.go" { + // Exercise finalizer assertions during registration and GC. testOptions.Tags = append(append([]string(nil), options.Tags...), "runtime_asserts") } runTest(name, testOptions, t, nil, nil) @@ -969,6 +970,30 @@ func TestWasmExportJS(t *testing.T) { } } +func TestWasmExportFinalizersJS(t *testing.T) { + t.Parallel() + + tmpdir := t.TempDir() + options := optionsFromTarget("wasm", sema) + options.BuildMode = "c-shared" + buildConfig, err := builder.NewConfig(&options) + if err != nil { + t.Fatal(err) + } + result, err := builder.Build("testdata/wasmexport-finalizer.go", ".wasm", tmpdir, buildConfig) + if err != nil { + t.Fatal("failed to build binary:", err) + } + + output := &bytes.Buffer{} + cmd := exec.Command("node", "testdata/wasmexport-finalizer.js", result.Binary) + cmd.Stdout = output + cmd.Stderr = output + if err := cmd.Run(); err != nil { + t.Fatalf("failed to run node: %v\n%s", err, output) + } +} + // Test whether Go.run() (in wasm_exec.js) normally returns and returns the // right exit code. func TestWasmExit(t *testing.T) { diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index e4aa931eca..1749389b44 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -391,7 +391,7 @@ func scanFinalizers() { for n := finalizerPending; n != nil; n = n.next { // Inside the collection, so the resurrected object is expected // to carry the mark state rather than plain head. - if blockFromAddr(decodeFinalizerPtr(n.obj)).state() != blockStateMark { + if blockFromAddr(decodeFinalizerPtr(n.obj)).findHead().state() != blockStateMark { runtimeFatal("gc: pending finalizer object was not resurrected") } } @@ -479,12 +479,12 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { // (a few bytes) relative to what it pins, so the registration count is a proxy // for external memory pressure that the heap-size GC trigger cannot see. // -// It is installed as the cooperative scheduler's idle hook by the first -// SetFinalizer (see spawnFinalizerRunner) and called only from the scheduler's -// drained-runqueue point, where no goroutine is running on its own stack. That -// reclaims a completed run of goroutines' now-dead values in a single pass, -// rather than forcing a collection synchronously inside alloc while an -// operation's values are still live, which would scale GC frequency with +// It is installed as the cooperative scheduler's pressure hook by the first +// SetFinalizer (see spawnFinalizerRunner) and called where no goroutine is +// running on its own stack: at a drained run queue or before a top-level wasm +// export returns. That reclaims a completed run of goroutines' now-dead values +// in one pass, rather than forcing a collection synchronously inside alloc while +// an operation's values are still live, which would scale GC frequency with // allocation churn and waste most collections on still-live values. func finalizerPressureGC() bool { trigger := finalizerGCTrigger() diff --git a/src/runtime/runtime_wasmentry.go b/src/runtime/runtime_wasmentry.go index 59cacb3b04..b621f38831 100644 --- a/src/runtime/runtime_wasmentry.go +++ b/src/runtime/runtime_wasmentry.go @@ -7,7 +7,6 @@ package runtime // compiler for //go:wasmexport support. import ( - "internal/task" "unsafe" ) @@ -85,19 +84,3 @@ func wasmExportRun(done *bool) { runtimePanic("//go:wasmexport function did not finish") } } - -// Called from the goroutine wrapper for the //go:wasmexport function. It just -// signals to the runtime that the //go:wasmexport call has finished, and can -// switch back to the wasmExportRun function. -// -// This function is not called when the scheduler is disabled. -func wasmExportExit() { - // Signal to the scheduler that it should return, since this call to a - // //go:wasmexport function has exited. - schedulerExit = true - - task.Pause() - - // TODO: we could cache the allocated stack so we don't have to keep - // allocating a new stack on every //go:wasmexport call. -} diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index 426017054c..78e7f43bf6 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -42,11 +42,12 @@ var ( deadlockedTasks task.Queue ) -// finalizerIdleGC, when non-nil, is called at the scheduler's idle point to -// collect on finalizer-registration pressure (returning whether it did). It is -// installed lazily by the first SetFinalizer, so a program that never registers -// a finalizer never assigns it and the linker drops the whole collection path. -// It is nil under GCs without a finalizer table. +// finalizerIdleGC, when non-nil, collects on finalizer-registration pressure +// (returning whether it did). It is called at the scheduler's idle point and +// before a top-level wasm export returns. It is installed lazily by the first +// SetFinalizer, so a program that never registers a finalizer never assigns it +// and the linker drops the whole collection path. It is nil under GCs without a +// finalizer table. var finalizerIdleGC func() bool // deadlock is called when a goroutine cannot proceed any more, but is in theory @@ -75,6 +76,25 @@ func exitGoroutine() { runtimeFatal("unreachable") } +// Called from the goroutine wrapper for the //go:wasmexport function. It just +// signals to the runtime that the //go:wasmexport call has finished, and can +// switch back to the wasmExportRun function. +// +// This function is not called when the scheduler is disabled. +func wasmExportExit() { + // Signal to the scheduler that it should return, since this call to a + // //go:wasmexport function has exited. + schedulerExit = true + if finalizerIdleGC != nil { + task.MarkFinishing() + } + + task.Pause() + + // TODO: we could cache the allocated stack so we don't have to keep + // allocating a new stack on every //go:wasmexport call. +} + func goexit() { if finalizerIdleGC != nil { task.MarkFinishing() @@ -274,6 +294,16 @@ func scheduler(returnAtDeadlock bool) { // //go:wasmexport function returned. if GOARCH == "wasm" && schedulerExit { schedulerExit = false // reset the signal + if task.Current() == nil && finalizerIdleGC != nil { + finalizerIdleGC() + // A wasm export must return as soon as its own task finishes instead + // of also running the finalizer runner or unrelated goroutines. On + // JavaScript, resume the scheduler in a fresh event-loop turn after + // control has returned to the caller. + if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { + sleepTicks(0) + } + } return } } diff --git a/testdata/finalizerlarge.go b/testdata/finalizerlarge.go new file mode 100644 index 0000000000..4ba8ddc04f --- /dev/null +++ b/testdata/finalizerlarge.go @@ -0,0 +1,29 @@ +package main + +import "runtime" + +type largeFinalizerObject struct { + data [128]byte +} + +var largeFinalizerRan bool + +//go:noinline +func registerLargeFinalizer() { + p := new(largeFinalizerObject) + runtime.SetFinalizer(p, func(*largeFinalizerObject) { + largeFinalizerRan = true + }) +} + +func main() { + registerLargeFinalizer() + for i := 0; i < 100 && !largeFinalizerRan; i++ { + runtime.GC() + runtime.Gosched() + } + if !largeFinalizerRan { + panic("large object finalizer did not run") + } + println("ok") +} diff --git a/testdata/finalizerlarge.txt b/testdata/finalizerlarge.txt new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/testdata/finalizerlarge.txt @@ -0,0 +1 @@ +ok diff --git a/testdata/wasmexport-finalizer.go b/testdata/wasmexport-finalizer.go new file mode 100644 index 0000000000..db37c6890a --- /dev/null +++ b/testdata/wasmexport-finalizer.go @@ -0,0 +1,27 @@ +package main + +import "runtime" + +//go:wasmimport tester finalizerRan +func finalizerRan() + +//go:wasmimport tester backgroundRan +func backgroundRan() + +//go:noinline +func registerFinalizersImpl() { + for i := 0; i < 32; i++ { + p := new([2]int) + runtime.SetFinalizer(p, func(*[2]int) { finalizerRan() }) + } +} + +//go:wasmexport registerFinalizers +func registerFinalizers() { + registerFinalizersImpl() + go func() { + backgroundRan() + }() +} + +func main() {} diff --git a/testdata/wasmexport-finalizer.js b/testdata/wasmexport-finalizer.js new file mode 100644 index 0000000000..103bb5908b --- /dev/null +++ b/testdata/wasmexport-finalizer.js @@ -0,0 +1,32 @@ +const fs = require('fs'); + +require('../targets/wasm_exec.js'); + +let finalized = 0; +let backgroundRuns = 0; +const go = new Go(); +go.importObject.tester = { + finalizerRan: () => { + finalized++; + }, + backgroundRan: () => { + backgroundRuns++; + }, +}; + +WebAssembly.instantiate(fs.readFileSync(process.argv[2]), go.importObject).then(async result => { + await go.run(result.instance); + result.instance.exports.registerFinalizers(); + if (backgroundRuns !== 0) { + throw new Error('wasm export ran an unrelated goroutine before returning'); + } + for (let i = 0; i < 500 && finalized === 0; i++) { + await new Promise(resolve => setTimeout(resolve, 1)); + } + if (finalized === 0) { + throw new Error('no wasm-export finalizer ran after returning to JavaScript'); + } +}).catch(err => { + console.error(err); + process.exit(1); +}); From 9c0cecc3c672d9c62b2ac42d6d68032cfea374ae Mon Sep 17 00:00:00 2001 From: felipegenef Date: Mon, 24 Aug 2026 15:26:47 -0300 Subject: [PATCH 14/16] runtime: decouple wasm export scheduling from finalizers --- main_test.go | 2 ++ src/runtime/scheduler_cooperative.go | 18 +++++++++++------- testdata/wasmexport-finalizer.go | 7 +++++++ testdata/wasmexport-finalizer.js | 18 ++++++++++++++++-- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/main_test.go b/main_test.go index c671c1b37d..5a26baaf91 100644 --- a/main_test.go +++ b/main_test.go @@ -124,6 +124,8 @@ func TestBuild(t *testing.T) { runPlatTests(hostOptions, tests, t) // Exercise the host-only schedulers with a GC that implements finalizers. + // scheduler.threads needs threadID, which internal/task only defines on + // Linux and Darwin, while scheduler.none does not link on Windows. switch runtime.GOOS { case "darwin", "linux": for _, scheduler := range []string{"threads", "none"} { diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index 78e7f43bf6..6d9abdc01c 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -70,6 +70,8 @@ func deadlock() { // and will never be resumed. func exitGoroutine() { if finalizerIdleGC != nil { + // Finalizer use enables clearing finished asyncify stacks so stale + // pointers do not delay collection of finalized objects. task.MarkFinishing() } task.Pause() @@ -86,6 +88,7 @@ func wasmExportExit() { // //go:wasmexport function has exited. schedulerExit = true if finalizerIdleGC != nil { + // See exitGoroutine: stack clearing is enabled only after finalizer use. task.MarkFinishing() } @@ -97,6 +100,7 @@ func wasmExportExit() { func goexit() { if finalizerIdleGC != nil { + // See exitGoroutine: stack clearing is enabled only after finalizer use. task.MarkFinishing() } task.Exit() @@ -296,13 +300,13 @@ func scheduler(returnAtDeadlock bool) { schedulerExit = false // reset the signal if task.Current() == nil && finalizerIdleGC != nil { finalizerIdleGC() - // A wasm export must return as soon as its own task finishes instead - // of also running the finalizer runner or unrelated goroutines. On - // JavaScript, resume the scheduler in a fresh event-loop turn after - // control has returned to the caller. - if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { - sleepTicks(0) - } + } + // A wasm export must return as soon as its own task finishes instead + // of also running the finalizer runner or unrelated goroutines. On + // JavaScript, resume the scheduler in a fresh event-loop turn after + // control has returned to the caller. + if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { + sleepTicks(0) } return } diff --git a/testdata/wasmexport-finalizer.go b/testdata/wasmexport-finalizer.go index db37c6890a..5e76bf847e 100644 --- a/testdata/wasmexport-finalizer.go +++ b/testdata/wasmexport-finalizer.go @@ -8,6 +8,13 @@ func finalizerRan() //go:wasmimport tester backgroundRan func backgroundRan() +//go:wasmexport launchBackground +func launchBackground() { + go func() { + backgroundRan() + }() +} + //go:noinline func registerFinalizersImpl() { for i := 0; i < 32; i++ { diff --git a/testdata/wasmexport-finalizer.js b/testdata/wasmexport-finalizer.js index 103bb5908b..b5f23fa861 100644 --- a/testdata/wasmexport-finalizer.js +++ b/testdata/wasmexport-finalizer.js @@ -16,16 +16,30 @@ go.importObject.tester = { WebAssembly.instantiate(fs.readFileSync(process.argv[2]), go.importObject).then(async result => { await go.run(result.instance); - result.instance.exports.registerFinalizers(); + result.instance.exports.launchBackground(); if (backgroundRuns !== 0) { + throw new Error('wasm export ran a background goroutine before returning'); + } + for (let i = 0; i < 500 && backgroundRuns === 0; i++) { + await new Promise(resolve => setTimeout(resolve, 1)); + } + if (backgroundRuns !== 1) { + throw new Error('wasm scheduler did not resume after an export without finalizers'); + } + + result.instance.exports.registerFinalizers(); + if (backgroundRuns !== 1) { throw new Error('wasm export ran an unrelated goroutine before returning'); } - for (let i = 0; i < 500 && finalized === 0; i++) { + for (let i = 0; i < 500 && (finalized === 0 || backgroundRuns < 2); i++) { await new Promise(resolve => setTimeout(resolve, 1)); } if (finalized === 0) { throw new Error('no wasm-export finalizer ran after returning to JavaScript'); } + if (backgroundRuns !== 2) { + throw new Error('wasm scheduler did not resume after a finalizer export'); + } }).catch(err => { console.error(err); process.exit(1); From e2af23168e47a20d9d1cef6bd0f43b14e848dc7b Mon Sep 17 00:00:00 2001 From: felipegenef Date: Tue, 25 Aug 2026 18:38:05 -0300 Subject: [PATCH 15/16] runtime: avoid redundant wakeups for re-entrant wasm exports --- src/runtime/scheduler_cooperative.go | 21 ++++++++------- testdata/wasmexport-finalizer.go | 19 ++++++++++++- testdata/wasmexport-finalizer.js | 40 +++++++++++++++++++++++----- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index 6d9abdc01c..574247ae85 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -298,15 +298,18 @@ func scheduler(returnAtDeadlock bool) { // //go:wasmexport function returned. if GOARCH == "wasm" && schedulerExit { schedulerExit = false // reset the signal - if task.Current() == nil && finalizerIdleGC != nil { - finalizerIdleGC() - } - // A wasm export must return as soon as its own task finishes instead - // of also running the finalizer runner or unrelated goroutines. On - // JavaScript, resume the scheduler in a fresh event-loop turn after - // control has returned to the caller. - if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { - sleepTicks(0) + if task.Current() == nil { + if finalizerIdleGC != nil { + finalizerIdleGC() + } + // A top-level wasm export must return as soon as its own task finishes + // instead of also running unrelated goroutines. On JavaScript, resume + // the scheduler in a fresh event-loop turn after control has returned + // to the caller. A re-entrant export leaves its outer scheduler active, + // so that scheduler will drain the queues without another wakeup. + if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { + sleepTicks(0) + } } return } diff --git a/testdata/wasmexport-finalizer.go b/testdata/wasmexport-finalizer.go index 5e76bf847e..99a0fdd411 100644 --- a/testdata/wasmexport-finalizer.go +++ b/testdata/wasmexport-finalizer.go @@ -1,6 +1,9 @@ package main -import "runtime" +import ( + "runtime" + "syscall/js" +) //go:wasmimport tester finalizerRan func finalizerRan() @@ -8,6 +11,11 @@ func finalizerRan() //go:wasmimport tester backgroundRan func backgroundRan() +//go:wasmimport tester callNestedExport +func callNestedExport() + +var nestedCallback js.Func + //go:wasmexport launchBackground func launchBackground() { go func() { @@ -15,6 +23,15 @@ func launchBackground() { }() } +//go:wasmexport installNestedCallback +func installNestedCallback() { + nestedCallback = js.FuncOf(func(js.Value, []js.Value) any { + callNestedExport() + return nil + }) + js.Global().Set("nestedExportCallback", nestedCallback) +} + //go:noinline func registerFinalizersImpl() { for i := 0; i < 32; i++ { diff --git a/testdata/wasmexport-finalizer.js b/testdata/wasmexport-finalizer.js index b5f23fa861..44305b9de5 100644 --- a/testdata/wasmexport-finalizer.js +++ b/testdata/wasmexport-finalizer.js @@ -4,7 +4,16 @@ require('../targets/wasm_exec.js'); let finalized = 0; let backgroundRuns = 0; +let instance; const go = new Go(); +const sleepTicks = go.importObject.gojs['runtime.sleepTicks']; +let zeroRearms = 0; +go.importObject.gojs['runtime.sleepTicks'] = timeout => { + if (Number(timeout) === 0) { + zeroRearms++; + } + return sleepTicks(timeout); +}; go.importObject.tester = { finalizerRan: () => { finalized++; @@ -12,14 +21,23 @@ go.importObject.tester = { backgroundRan: () => { backgroundRuns++; }, + callNestedExport: () => { + instance.exports.launchBackground(); + }, }; WebAssembly.instantiate(fs.readFileSync(process.argv[2]), go.importObject).then(async result => { - await go.run(result.instance); - result.instance.exports.launchBackground(); + instance = result.instance; + await go.run(instance); + + const topLevelRearms = zeroRearms; + instance.exports.launchBackground(); if (backgroundRuns !== 0) { throw new Error('wasm export ran a background goroutine before returning'); } + if (zeroRearms !== topLevelRearms + 1) { + throw new Error('top-level wasm export did not schedule exactly one wakeup'); + } for (let i = 0; i < 500 && backgroundRuns === 0; i++) { await new Promise(resolve => setTimeout(resolve, 1)); } @@ -27,17 +45,27 @@ WebAssembly.instantiate(fs.readFileSync(process.argv[2]), go.importObject).then( throw new Error('wasm scheduler did not resume after an export without finalizers'); } - result.instance.exports.registerFinalizers(); - if (backgroundRuns !== 1) { + instance.exports.installNestedCallback(); + const nestedRearms = zeroRearms; + global.nestedExportCallback(); + if (zeroRearms !== nestedRearms) { + throw new Error('re-entrant wasm export scheduled a redundant wakeup'); + } + if (backgroundRuns !== 2) { + throw new Error('outer scheduler did not drain work from a re-entrant wasm export'); + } + + instance.exports.registerFinalizers(); + if (backgroundRuns !== 2) { throw new Error('wasm export ran an unrelated goroutine before returning'); } - for (let i = 0; i < 500 && (finalized === 0 || backgroundRuns < 2); i++) { + for (let i = 0; i < 500 && (finalized === 0 || backgroundRuns < 3); i++) { await new Promise(resolve => setTimeout(resolve, 1)); } if (finalized === 0) { throw new Error('no wasm-export finalizer ran after returning to JavaScript'); } - if (backgroundRuns !== 2) { + if (backgroundRuns !== 3) { throw new Error('wasm scheduler did not resume after a finalizer export'); } }).catch(err => { From 214274adabc4bb402893e5a4fa395837488975e6 Mon Sep 17 00:00:00 2001 From: felipegenef Date: Wed, 26 Aug 2026 12:15:47 -0300 Subject: [PATCH 16/16] runtime: simplify finalizer comments --- compileopts/finalizer_coverage_test.go | 13 +- compiler/goroutine.go | 1 - main_test.go | 28 +--- src/internal/task/task_asyncify.go | 31 +---- src/internal/task/task_finishing_tasks.go | 6 +- src/runtime/gc_finalizer.go | 152 +++++----------------- src/runtime/gc_finalizer_sched.go | 7 +- src/runtime/gc_finalizer_sched_other.go | 12 +- src/runtime/scheduler_cooperative.go | 46 ++----- testdata/finalizerbits.go | 43 +----- testdata/finalizeridle.go | 63 +++------ testdata/finalizerinvariants.go | 19 +-- 12 files changed, 87 insertions(+), 334 deletions(-) diff --git a/compileopts/finalizer_coverage_test.go b/compileopts/finalizer_coverage_test.go index 604d8b6de7..bf12e0c5e4 100644 --- a/compileopts/finalizer_coverage_test.go +++ b/compileopts/finalizer_coverage_test.go @@ -8,12 +8,8 @@ import ( "testing" ) -// TestFinalizerRunnerSchedulerCoverage checks that the build constraints on the -// gc_finalizer_sched*.go files define spawnFinalizerRunner for exactly one file -// per scheduler. The three constraints must partition the scheduler space: every -// scheduler matches exactly one file, so none can be left with the symbol -// undefined or defined twice. It iterates validSchedulerOptions as the source of -// truth, so a newly added scheduler is covered by this check automatically. +// TestFinalizerRunnerSchedulerCoverage verifies that each scheduler selects one runner file. +// It uses validSchedulerOptions so new schedulers are included. func TestFinalizerRunnerSchedulerCoverage(t *testing.T) { files := []string{ "gc_finalizer_sched.go", @@ -26,8 +22,8 @@ func TestFinalizerRunnerSchedulerCoverage(t *testing.T) { } for _, sched := range validSchedulerOptions { - // The finalizer table exists under the block GCs; gc.conservative - // satisfies the "gc.conservative || gc.precise" half of every constraint. + // The finalizer table exists under block GCs. + // gc.conservative satisfies the GC condition in every constraint. tags := map[string]bool{ "gc.conservative": true, "scheduler." + sched: true, @@ -45,7 +41,6 @@ func TestFinalizerRunnerSchedulerCoverage(t *testing.T) { } } -// readBuildConstraint returns the parsed //go:build expression of a Go file. func readBuildConstraint(t *testing.T, path string) constraint.Expr { t.Helper() data, err := os.ReadFile(path) diff --git a/compiler/goroutine.go b/compiler/goroutine.go index 497c3a25bf..8bc7da53cd 100644 --- a/compiler/goroutine.go +++ b/compiler/goroutine.go @@ -535,7 +535,6 @@ func (c *compilerContext) createGoroutineStartWrapper(fnType llvm.Type, fn llvm. } if c.Scheduler == "asyncify" { - // The goroutine was terminated via exitGoroutine. b.CreateUnreachable() } else { // Finish the function. Every basic block must end in a terminator, and diff --git a/main_test.go b/main_test.go index 5a26baaf91..183c39e519 100644 --- a/main_test.go +++ b/main_test.go @@ -123,9 +123,8 @@ func TestBuild(t *testing.T) { hostOptions := optionsFromTarget("", sema) runPlatTests(hostOptions, tests, t) - // Exercise the host-only schedulers with a GC that implements finalizers. - // scheduler.threads needs threadID, which internal/task only defines on - // Linux and Darwin, while scheduler.none does not link on Windows. + // scheduler.threads needs threadID, which exists only on Linux and Darwin. + // scheduler.none does not link on Windows. switch runtime.GOOS { case "darwin", "linux": for _, scheduler := range []string{"threads", "none"} { @@ -390,35 +389,23 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { if options.Target != "wasm" { switch name { case "finalizer.go", "finalizerbits.go", "finalizeridle.go", "finalizerlarge.go": - // runtime.SetFinalizer is implemented for the block GC, but these - // tests assert deterministic collection of a dropped object, - // which only holds on the GOOS=js wasm target. The host default - // GC is boehm (SetFinalizer is a no-op there); conservative stack - // scanning on the emulated targets can pin the object; and the - // wasip2 component entry lays out the stack differently, so - // collection is not deterministic on those. The feature still - // works on all of them, it just can't be golden-tested for - // firing, which is what finalizerinvariants.go covers instead. + // These tests require deterministic finalization on target wasm. + // finalizerinvariants.go covers other block GC targets. continue } } if options.Target == "" && options.GC == "" { switch name { case "finalizerinvariants.go": - // The default GC on these is boehm, where SetFinalizer is - // unimplemented, so there is nothing to assert. The explicit - // -gc=conservative host variants cover it instead. + // Skip the default host GC because it does not implement finalizers. + // Explicit conservative GC variants cover this test. continue } } if options.Target == "simavr" { switch name { case "finalizerinvariants.go": - // Finalizers are detected by the GC, and gc.go is already skipped - // on AVR for its high mark false positive rate (see the simavr - // switch above). Registering and clearing a finalizer works - // there, but a single runtime.GC() call does not return, so this - // test inherits that limitation rather than adding a new one. + // Skip because runtime.GC does not return. See the gc.go exclusion above. continue } } @@ -428,7 +415,6 @@ func runPlatTests(options compileopts.Options, tests []string, t *testing.T) { t.Parallel() testOptions := compileopts.Options(options) if name == "finalizerinvariants.go" || name == "finalizerlarge.go" { - // Exercise finalizer assertions during registration and GC. testOptions.Tags = append(append([]string(nil), options.Tags...), "runtime_asserts") } runTest(name, testOptions, t, nil, nil) diff --git a/src/internal/task/task_asyncify.go b/src/internal/task/task_asyncify.go index 9423e40447..4d78e19373 100644 --- a/src/internal/task/task_asyncify.go +++ b/src/internal/task/task_asyncify.go @@ -26,10 +26,8 @@ type state struct { launched bool - // finishing is set immediately before this goroutine, having run to - // completion, pauses for the last time. Resume observes it and clears the - // goroutine's stack. It lives on the task so each finishing goroutine owns - // its own flag, independent of scheduler timing. + // finishing marks a goroutine that paused after it completed. + // Resume uses this per task flag to clear the stack. finishing bool } @@ -49,9 +47,7 @@ type stackState struct { // overflow happened in the past. canaryPtr *uintptr - // top is the first address past the end of the stack allocation (the - // initial C stack pointer). Kept so the whole stack buffer can be located - // again after the goroutine finishes. + // top marks the end of the stack buffer so it can be cleared after completion. top unsafe.Pointer } @@ -95,22 +91,13 @@ func (s *state) initialize(fn uintptr, args unsafe.Pointer, stackSize uintptr) { //go:linkname memzero runtime.memzero func memzero(ptr unsafe.Pointer, size uintptr) -// MarkFinishing records that the current goroutine has finished and will not be -// resumed, so Resume may reclaim its stack once control returns to the scheduler. -// The flag lives on the task itself, so each finishing goroutine owns its own and -// the handoff to Resume does not depend on scheduler timing. +// MarkFinishing marks the current goroutine for stack cleanup after it returns to the scheduler. func MarkFinishing() { currentTask.state.finishing = true } -// clearStack zeroes a finished goroutine's entire stack buffer. The buffer is a -// plain heap allocation scanned conservatively by the GC (it can hold arbitrary -// pointers), so any stale pointer left in it by the goroutine's now-returned -// call frames would keep unrelated objects reachable (and, transitively, other -// finished stacks reachable through them) until a later collection happens to -// break the chain. Zeroing the buffer the moment the goroutine finishes drops -// those stale references immediately, so the objects they pointed at (and the -// stack itself) become collectable at the next cycle. +// clearStack removes stale pointers from a finished asyncify stack. +// The GC can then collect the stack and referenced objects. func (t *Task) clearStack() { base := unsafe.Pointer(t.state.canaryPtr) memzero(base, uintptr(t.state.top)-uintptr(base)) @@ -160,11 +147,7 @@ func (t *Task) Resume() { runtimeFatal("stack overflow") } if t.state.finishing { - // The goroutine just ran to completion and paused for the last time. It - // will never be resumed, so its stack can be cleared now to drop any - // pointers its returned frames left behind (see clearStack). The args - // bundle is likewise no longer needed, so drop that reference too, else - // any pointers in the arguments would keep their objects reachable. + // The task is complete. Clear stale stack pointers and release its argument bundle. t.state.finishing = false t.clearStack() t.state.args = nil diff --git a/src/internal/task/task_finishing_tasks.go b/src/internal/task/task_finishing_tasks.go index 0802f52951..0c6f032612 100644 --- a/src/internal/task/task_finishing_tasks.go +++ b/src/internal/task/task_finishing_tasks.go @@ -2,9 +2,5 @@ package task -// MarkFinishing is a no-op for the stack-based scheduler. Zeroing a finished -// goroutine's stack to drop the stale pointers its returned frames leave behind -// is only implemented for the asyncify scheduler, whose goroutine stacks are -// heap buffers scanned conservatively (see the asyncify MarkFinishing and -// Resume). exitGoroutine and goexit in the cooperative scheduler call this for both. +// MarkFinishing does nothing for scheduler.tasks because it does not use asyncify heap stacks. func MarkFinishing() {} diff --git a/src/runtime/gc_finalizer.go b/src/runtime/gc_finalizer.go index 1749389b44..df3a4c6c6f 100644 --- a/src/runtime/gc_finalizer.go +++ b/src/runtime/gc_finalizer.go @@ -33,34 +33,15 @@ type finalizerEntry struct { fn interface{} } -// finalizerGCThreshold is the minimum net registration pressure accumulated -// since the last collection before the scheduler runs one at its idle point. -// finalizerGCTrigger raises this floor proportionally for larger tables. -// A registered finalizer almost always guards an external resource, most -// importantly a syscall/js bridge-table slot (js.Value or js.Func), that costs -// only a few bytes of Go heap but pins a whole JS object and its slot. Without -// this, a long-lived instance with a large resident heap defers GC (and thus -// finalizer draining) until the Go heap itself fills, which for a bursty, -// mostly-idle workload may be never, so the external resources accumulate -// without bound. Coupling a GC to finalizer-registration pressure bounds that -// accumulation. -// -// This is a compile-time policy constant, in the spirit of Go's forcegcperiod. -// The trigger only fires at the scheduler's idle point (a drained run queue) and -// each firing resets the count (see scanFinalizers), so it is throttled to that -// point rather than firing once per this-many registrations: a setup phase that -// registers many long-lived finalizers pays at most one extra collection at the -// first idle point after it, not one per threshold, and that collection just -// marks still-live data during otherwise-idle time without freeing anything -// early. Keeping it a const also lets the compiler constant-fold the check and, -// with zero, drop the pressure path entirely. Zero disables the trigger. +// finalizerGCThreshold starts pressure GC when registrations indicate external memory pressure. +// Larger tables use a proportional threshold. Zero disables this trigger. const finalizerGCThreshold = 32 var ( finalizers *finalizerEntry // registered finalizers; a GC root that keeps fn values alive finalizerPending *finalizerEntry // finalizers whose object died, waiting to run numFinalizers uintptr // number of registered finalizers; fast-path gate for scanFinalizers - finalizersSinceGC uintptr // net registration pressure since the last GC; drives the scheduler idle-point trigger + finalizersSinceGC uintptr // tracks registration pressure for the scheduler trigger finalizersQueued bool // set when scanFinalizers queued at least one finalizer to run finalizerFutex task.Futex // wakes the finalizerRunner goroutine after a GC queues work finalizerDraining bool // guards against re-entrant inline draining (scheduler.none) @@ -74,21 +55,10 @@ var ( finalizerRunnerStarted bool ) -// finalizerGCDivisor scales the registration trigger with the size of the -// table: the next collection is due after roughly numFinalizers/this much net -// pressure, never less than finalizerGCThreshold. const finalizerGCDivisor = 2 -// finalizerGCTrigger returns how much net registration pressure is needed to run -// the next collection. Each collection scans the whole table, which -// costs O(numFinalizers), so a trigger that stays constant while the table grows -// makes N registrations cost O(N^2) in scanning alone. Scaling the trigger with -// the table keeps the amortized scan cost per registration constant, the same -// reasoning behind Go's proportional GOGC pacing: collect when the tracked set -// has grown by a fraction of itself, not by a fixed count. -// -// The floor keeps the original behaviour for small tables, where a proportional -// trigger would fire too rarely to be useful. +// finalizerGCTrigger scales the threshold so scan work stays proportional to registrations. +// It uses finalizerGCThreshold as the minimum. func finalizerGCTrigger() uintptr { if finalizerGCThreshold == 0 { return 0 @@ -99,31 +69,12 @@ func finalizerGCTrigger() uintptr { return finalizerGCThreshold } -// finalizerBits records, one bit per heap block, whether the object starting at -// that block already has a registered finalizer. It answers the "is this object -// already registered?" question that SetFinalizer's replace semantics require -// without walking the table, so the common case (a fresh object, which is every -// syscall/js value) never scans anything. -// -// This mirrors what upstream Go gets from its per-span specials plus the -// arena-level "span has specials" bitmap: a constant-time way to skip objects -// that have nothing registered. -// -// The bitmap is allocated on the first registration and grown with the heap, so -// a program that never registers a finalizer keeps the whole feature dead. -// -// Every access goes through gcLock, including the reads. The slice header itself -// is replaced when the heap grows, so an unlocked reader on a parallel scheduler -// (cores, threads) could observe a stale bit, or tear the header and index the -// old, shorter buffer with the new length. +// finalizerBits records finalizer registrations by heap block for fast lookup. +// Hold gcLock for every access because heap growth can replace the slice. var finalizerBits []byte -// finalizerBitsShortfall returns the bitmap length needed to cover the current -// heap, or zero if the current bitmap already covers it. It must be called under -// gcLock: that is what makes reading finalizerBits and endBlock safe against a -// concurrent adoptFinalizerBits on another core. The caller then allocates with -// the lock released (allocating takes gcLock) and installs the result with -// adoptFinalizerBits. +// finalizerBitsShortfall returns the required bitmap size or zero. +// Call it with gcLock held and release the lock before allocation. func finalizerBitsShortfall() uintptr { need := (uintptr(endBlock) + 7) / 8 if uintptr(len(finalizerBits)) >= need { @@ -132,12 +83,8 @@ func finalizerBitsShortfall() uintptr { return need } -// adoptFinalizerBits installs a wider bitmap under gcLock, carrying the old bits -// over. A nil or already-obsolete buffer is ignored, which is what makes it safe -// for the heap to have grown again (or another core to have installed its own -// wider bitmap) while the caller was allocating with the lock released. A buffer -// that covers less than the current heap is still an improvement: addresses past -// its end just keep answering conservatively in finalizerBitGet. +// adoptFinalizerBits installs a wider bitmap while gcLock is held. +// It accepts a stale size because the heap can grow during allocation. func adoptFinalizerBits(buf []byte) { if len(buf) <= len(finalizerBits) { return @@ -151,10 +98,8 @@ func finalizerBitIndex(addr uintptr) uintptr { return uintptr(blockFromAddr(addr func finalizerBitGet(addr uintptr) bool { i := finalizerBitIndex(addr) if i/8 >= uintptr(len(finalizerBits)) { - // The bitmap does not describe this address yet (the heap grew since it - // was sized). Answer conservatively: a spurious "maybe" only costs one - // scan, while a wrong "no" would let a second entry be registered for an - // object that already has one, and its finalizer would run twice. + // Return true when the bitmap does not cover the address. + // A false result could register a second finalizer for the object. return true } return finalizerBits[i/8]&(1<<(i%8)) != 0 @@ -184,9 +129,7 @@ func finalizerBitClear(addr uintptr) { func encodeFinalizerPtr(addr uintptr) uintptr { return ^addr } func decodeFinalizerPtr(enc uintptr) uintptr { return ^enc } -// finalizerRegistered reports whether the table holds an entry for enc. This is -// the linear answer the registration bitmap exists to avoid, so it is only used -// under gcAsserts, to check the bitmap against the table it summarizes. +// finalizerRegistered checks the table when gcAsserts validates the bitmap. func finalizerRegistered(enc uintptr) bool { for n := finalizers; n != nil; n = n.next { if n.obj == enc { @@ -196,12 +139,8 @@ func finalizerRegistered(enc uintptr) bool { return false } -// assertFinalizerTable verifies the bookkeeping that the table, the counter and -// the registration bitmap have to agree on. A registered entry without its bit -// is the dangerous direction: the clear and replace paths trust a clear bit to -// mean "nothing registered" and skip the table walk, so a missing bit turns -// SetFinalizer(obj, nil) into a silent no-op and lets a re-registration add a -// second entry, which runs the finalizer twice. Only called under gcAsserts. +// assertFinalizerTable checks that the table, count, and bitmap agree. +// A missing bit can prevent removal or allow two finalizers for one object. func assertFinalizerTable() { var count uintptr for n := finalizers; n != nil; n = n.next { @@ -225,13 +164,8 @@ func registerFinalizer(addr uintptr, fn interface{}) { enc := encodeFinalizerPtr(addr) if fn == nil { - // Clear: remove every registration for this object. The bit proves in - // one test that there is nothing to remove, but only while gcLock is - // held: a registration on another core may be setting that same bit (and - // replacing the bitmap) right now, and a stale read of zero would skip - // the removal and leave the finalizer registered on a live object. - // Holding the lock for the check costs nothing extra, because the removal - // below needs it anyway; what the bit saves is the O(numFinalizers) walk. + // Hold gcLock while checking the bit because another core can update the bitmap. + // A clear bit avoids a scan of the finalizer table. gcLock.Lock() tracked := isOnHeap(addr) if tracked && !finalizerBitGet(addr) { @@ -264,12 +198,8 @@ func registerFinalizer(addr uintptr, fn interface{}) { return } - // Register or replace. Allocating acquires gcLock, so the entry is allocated - // before the lock is taken and a wider bitmap is allocated by dropping the - // lock for just that call. Only a heap that outgrew the bitmap pays that - // round trip; the common case holds the lock once, and adoptFinalizerBits - // tolerates the heap having grown again (or another core having installed a - // wider bitmap) while this one was allocating. + // Allocate before taking gcLock because allocation also takes this lock. + // Release gcLock only when the bitmap must grow. entry := &finalizerEntry{obj: enc, fn: fn} gcLock.Lock() if shortfall := finalizerBitsShortfall(); shortfall != 0 { @@ -284,9 +214,8 @@ func registerFinalizer(addr uintptr, fn interface{}) { if gcAsserts && tracked && !finalizerBitGet(addr) && finalizerRegistered(enc) { runtimeFatal("gc: finalizer bit clear but the object is registered") } - // Only an object whose bit is set can already be in the table, so a fresh - // object skips the scan entirely. An address the bitmap cannot describe - // (not on the heap) always scans, as before. + // Scan only if the bitmap can contain a registration for this object. + // Always scan addresses that the bitmap does not cover. for n := finalizers; (!tracked || finalizerBitGet(addr)) && n != nil; n = n.next { if n.obj == enc { // Replace the finalizer for an already-registered object, so it @@ -310,7 +239,7 @@ func registerFinalizer(addr uintptr, fn interface{}) { finalizerBitSet(addr) } numFinalizers++ - finalizersSinceGC++ // pressure signal for the proactive GC trigger at the scheduler's idle point + finalizersSinceGC++ // A finalizer is registered, so make sure the runner exists. The flag is // serialized by gcLock; the spawn itself allocates, so it must run after the // lock is released. @@ -326,9 +255,7 @@ func registerFinalizer(addr uintptr, fn interface{}) { // current GC cycle and queues their finalizers. It must be called under gcLock, // after marking is complete and before sweep frees anything. func scanFinalizers() { - // A collection is running now, so reset the registration-pressure counter - // that drives the proactive idle-point trigger, regardless of whether any - // finalizer is registered or fires this cycle. + // Reset pressure at the start of every collection, even if no finalizer runs. finalizersSinceGC = 0 // Nothing registered and nothing waiting to run: fast path. @@ -356,8 +283,7 @@ func scanFinalizers() { // and into the pending queue (alloc-free), so its finalizer runs once. *prev = n.next numFinalizers-- - // The object is gone; clear its bit so a later object reusing the - // address starts clean. + // Clear the bit so a later object at this address starts clean. finalizerBitClear(addr) n.next = finalizerPending finalizerPending = n @@ -385,9 +311,8 @@ func scanFinalizers() { // survive this sweep. finishMark() if gcAsserts { - // Every pending object must have survived the resurrection above. - // One that did not is about to be swept while its finalizer is - // still queued, which is a use-after-free in callFinalizer. + // Verify that every pending object survived resurrection. + // Otherwise callFinalizer can use memory that sweep freed. for n := finalizerPending; n != nil; n = n.next { // Inside the collection, so the resurrected object is expected // to carry the mark state rather than plain head. @@ -457,11 +382,8 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { if !isOnHeap(addr) { runtimeFatal("gc: dequeued finalizer for an object off the heap") } - // scanFinalizers resurrects everything still pending, so sweep must - // have left the object allocated. A freed block here means - // callFinalizer is about to run on memory that is back in the free - // list. The mark bit is not the thing to check: this runs outside a - // collection, where unmark has already turned mark back into head. + // A pending object must remain allocated until its finalizer runs. + // Check the block state because this runs after marks become heads. if blockFromAddr(addr).state() == blockStateFree { runtimeFatal("gc: dequeued finalizer for a freed object") } @@ -472,20 +394,8 @@ func dequeueFinalizer() (*finalizerEntry, unsafe.Pointer) { return n, objPtr } -// finalizerPressureGC collects when net registration pressure since the last GC -// reaches finalizerGCTrigger, then hands any freshly-queued finalizers to the -// runner. It reports whether it collected. A registered -// finalizer almost always guards an external resource whose Go-heap cost is tiny -// (a few bytes) relative to what it pins, so the registration count is a proxy -// for external memory pressure that the heap-size GC trigger cannot see. -// -// It is installed as the cooperative scheduler's pressure hook by the first -// SetFinalizer (see spawnFinalizerRunner) and called where no goroutine is -// running on its own stack: at a drained run queue or before a top-level wasm -// export returns. That reclaims a completed run of goroutines' now-dead values -// in one pass, rather than forcing a collection synchronously inside alloc while -// an operation's values are still live, which would scale GC frequency with -// allocation churn and waste most collections on still-live values. +// finalizerPressureGC runs a GC when registrations indicate external memory pressure. +// It wakes the finalizer runner when the GC queues work. func finalizerPressureGC() bool { trigger := finalizerGCTrigger() if trigger == 0 || finalizersSinceGC < trigger { diff --git a/src/runtime/gc_finalizer_sched.go b/src/runtime/gc_finalizer_sched.go index 7c2dcb4554..0a4783eb8c 100644 --- a/src/runtime/gc_finalizer_sched.go +++ b/src/runtime/gc_finalizer_sched.go @@ -2,11 +2,8 @@ package runtime -// The go statement and the idle-hook install live in this scheduler-gated file, -// not inline in registerFinalizer, so a build that never calls SetFinalizer -// keeps internal/task.start and the whole finalizer collection path DCE'd. The -// cooperative scheduler additionally collects on finalizer-registration pressure -// at its idle point (see finalizerIdleGC in scheduler_cooperative.go). +// Keep this setup in a file for these schedulers so unused finalizer code can be removed. +// Cooperative schedulers also install the idle GC hook. func spawnFinalizerRunner() { finalizerIdleGC = finalizerPressureGC go finalizerRunner() diff --git a/src/runtime/gc_finalizer_sched_other.go b/src/runtime/gc_finalizer_sched_other.go index dbade9d323..f874ec46b9 100644 --- a/src/runtime/gc_finalizer_sched_other.go +++ b/src/runtime/gc_finalizer_sched_other.go @@ -2,14 +2,6 @@ package runtime -// spawnFinalizerRunner is defined once per scheduler class, and the three build -// constraints partition the scheduler space exactly (exactly one scheduler.* tag -// is ever set): scheduler.none in gc_finalizer_sched_none.go, scheduler.tasks and -// scheduler.asyncify in gc_finalizer_sched.go, and every other variant here. This -// is the catch-all, so a new scheduler variant lands here and stays defined -// rather than falling through to an undefined reference. -// -// Non-cooperative schedulers (cores, threads) spawn the finalizer runner but have -// no cooperative idle point, so they do not install the idle-pressure collector; -// the runner drains finalizers as GCs queue them. +// spawnFinalizerRunner is the fallback for noncooperative schedulers. +// These schedulers run finalizers but do not install the idle GC hook. func spawnFinalizerRunner() { go finalizerRunner() } diff --git a/src/runtime/scheduler_cooperative.go b/src/runtime/scheduler_cooperative.go index 574247ae85..72d9e175cf 100644 --- a/src/runtime/scheduler_cooperative.go +++ b/src/runtime/scheduler_cooperative.go @@ -42,12 +42,8 @@ var ( deadlockedTasks task.Queue ) -// finalizerIdleGC, when non-nil, collects on finalizer-registration pressure -// (returning whether it did). It is called at the scheduler's idle point and -// before a top-level wasm export returns. It is installed lazily by the first -// SetFinalizer, so a program that never registers a finalizer never assigns it -// and the linker drops the whole collection path. It is nil under GCs without a -// finalizer table. +// finalizerIdleGC runs pressure GC at safe points and enables asyncify stack cleanup. +// The first finalizer installs it so unused code can be removed. var finalizerIdleGC func() bool // deadlock is called when a goroutine cannot proceed any more, but is in theory @@ -65,30 +61,21 @@ func deadlock() { runtimeFatal("unreachable") } -// exitGoroutine is called by asyncify goroutine wrappers after the wrapped -// function returns. Unlike deadlock, this path means the task is truly finished -// and will never be resumed. +// exitGoroutine ends an asyncify task that returned from its function. +// Unlike deadlock, this task will not resume. func exitGoroutine() { if finalizerIdleGC != nil { - // Finalizer use enables clearing finished asyncify stacks so stale - // pointers do not delay collection of finalized objects. task.MarkFinishing() } task.Pause() runtimeFatal("unreachable") } -// Called from the goroutine wrapper for the //go:wasmexport function. It just -// signals to the runtime that the //go:wasmexport call has finished, and can -// switch back to the wasmExportRun function. -// -// This function is not called when the scheduler is disabled. +// wasmExportExit stops the scheduler after a //go:wasmexport function returns. +// It is not used when the scheduler is disabled. func wasmExportExit() { - // Signal to the scheduler that it should return, since this call to a - // //go:wasmexport function has exited. schedulerExit = true if finalizerIdleGC != nil { - // See exitGoroutine: stack clearing is enabled only after finalizer use. task.MarkFinishing() } @@ -100,7 +87,6 @@ func wasmExportExit() { func goexit() { if finalizerIdleGC != nil { - // See exitGoroutine: stack clearing is enabled only after finalizer use. task.MarkFinishing() } task.Exit() @@ -231,17 +217,8 @@ func scheduler(returnAtDeadlock bool) { t := runqueue.Pop() if t == nil { - // Idle point: the run queue is drained, so no goroutine is running on - // its own stack. This is the safe place to reclaim external resources - // whose finalizers have piled up since the last collection. Running it - // here, once per drained run queue and only at the top level - // (task.Current() == nil, so a re-entrant call from a suspended - // goroutine does not collect while that goroutine is mid-operation), - // reclaims a completed run of goroutines' now-dead values in one pass. - // Forcing the collection inside alloc instead would scale GC frequency - // with allocation churn: a goroutine that allocates hundreds of - // short-lived finalized objects would trigger dozens of collections - // mid-run, most of them wasted on values that are still live. + // Run the pressure GC only when the scheduler is idle at the top level. + // This batches completed work and avoids collections during allocation. if task.Current() == nil && finalizerIdleGC != nil && finalizerIdleGC() { continue } @@ -302,11 +279,8 @@ func scheduler(returnAtDeadlock bool) { if finalizerIdleGC != nil { finalizerIdleGC() } - // A top-level wasm export must return as soon as its own task finishes - // instead of also running unrelated goroutines. On JavaScript, resume - // the scheduler in a fresh event-loop turn after control has returned - // to the caller. A re-entrant export leaves its outer scheduler active, - // so that scheduler will drain the queues without another wakeup. + // Return from an export at the top level before unrelated goroutines run. + // A nested export returns to its active outer scheduler. if asyncScheduler && (!runqueue.Empty() || sleepQueue != nil || timerQueue != nil) { sleepTicks(0) } diff --git a/testdata/finalizerbits.go b/testdata/finalizerbits.go index 5ab10de32c..60ffadc860 100644 --- a/testdata/finalizerbits.go +++ b/testdata/finalizerbits.go @@ -1,25 +1,7 @@ package main -// Tests the registration bookkeeping behind runtime.SetFinalizer on the block -// GC: the per-block bit that records whether an object already has a finalizer. -// The bit is what lets a fresh object skip the registered-finalizer scan, so -// these cases pin the invariants that skipping must never break: -// -// - an object whose finalizer was cleared and then registered again still runs -// it exactly once, so clearing resets the bookkeeping; -// - registering twice replaces, it never leaves two registrations behind -// (which would run the finalizer twice); -// - churning register/clear on one object leaves no residue; -// - memory reused by a later object registers correctly, so a dead object's -// bookkeeping does not leak onto whatever lands at its address next; -// - a batch where only some objects keep a finalizer runs exactly those. -// -// Like finalizer.go, this is only run on the precise wasm target (see the tests -// slice and the skip in main_test.go): there a dropped object is deterministically -// collected, so the finalizers fire predictably. -// -// Each test calls its alloc helper and scrubStack at the same call depth, so the -// recursion reuses and clears the frame that just held the dropped pointers. +// Test finalizer registration, replacement, removal, and reused heap addresses. +// The wasm target provides deterministic finalization for these tests. import "runtime" @@ -39,11 +21,8 @@ var ( sink int ) -// scrubStack overwrites the stack region used by an alloc-and-drop helper with -// non-pointer words. It must be called at the same call depth as that helper so -// this recursion reuses (and clears) the frame that just held the dropped -// pointer; otherwise a stale copy keeps the object marked and it is never -// collected. The returned value derived from buf keeps the writes live. +// scrubStack removes stale pointers from the helper frame so collection is deterministic. +// Call it at the same call depth as the allocation helper. // //go:noinline func scrubStack(depth int) int { @@ -66,9 +45,6 @@ func allocClearThenRegister() { runtime.SetFinalizer(p, func(*box) { reregisteredRan++ }) } -// testClearThenRegister checks that clearing a finalizer and registering a new -// one leaves exactly the new one: clearing has to reset the bookkeeping, not -// just unlink the entry. func testClearThenRegister() { allocClearThenRegister() for i := 0; i < 200 && reregisteredRan == 0; i++ { @@ -90,10 +66,6 @@ func allocRegisterTwice() { } } -// testRegisterTwiceLeavesOne checks the replace path over a whole batch: the -// second registration must find the first one and take its place. A missed -// lookup would leave two registrations for the same object, and its finalizer -// would run twice. func testRegisterTwiceLeavesOne() { allocRegisterTwice() for i := 0; i < 200 && replacedNewRan < batch; i++ { @@ -118,8 +90,6 @@ func allocChurn() { } } -// testChurnLeavesNothing checks that many register/clear rounds on one object -// leave nothing behind: the object dies with no finalizer, so nothing runs. func testChurnLeavesNothing() { allocChurn() for i := 0; i < 200; i++ { @@ -148,9 +118,6 @@ func allocSecondRound() { } } -// testAddressReuse checks that objects allocated into memory freed by a previous -// finalized batch register correctly themselves. A dead object's bookkeeping must -// not survive onto whatever lands at its address next. func testAddressReuse() { allocFirstRound() for i := 0; i < 200 && reuseFirstRan < batch; i++ { @@ -185,8 +152,6 @@ func allocMixedBatch() { } } -// testMixedBatch checks that clearing some registrations inside a batch affects -// only those objects: the ones still registered run, the cleared ones do not. func testMixedBatch() { allocMixedBatch() for i := 0; i < 200 && keptRan < batch/2; i++ { diff --git a/testdata/finalizeridle.go b/testdata/finalizeridle.go index 0e5a7b10c0..f533ec77bf 100644 --- a/testdata/finalizeridle.go +++ b/testdata/finalizeridle.go @@ -1,15 +1,7 @@ package main -// Tests idle finalizer collection and goroutine-stack lifetime on precise wasm. -// The idle-collection cases verify that registration pressure triggers a -// collection without an explicit runtime.GC. The permanently blocked goroutine -// cases use explicit GCs and a control finalizer to distinguish live stacks from -// stalled GC progress. -// -// Like finalizer.go, this is only run on the precise wasm target (see the tests -// slice and the skip in main_test.go): there a dropped object is deterministically -// collected, so the finalizers fire predictably. The idle-collection cases do -// not call runtime.GC: their purpose is to prove the idle trigger itself works. +// Test idle finalizer collection and the lifetime of blocked and completed asyncify stacks. +// The wasm target provides deterministic finalization for these tests. import ( "runtime" @@ -17,8 +9,7 @@ import ( "time" ) -// batch must exceed the runtime's finalizer-registration threshold so the idle -// collection is guaranteed to trigger. +// batch must exceed the finalizer registration threshold to trigger idle collection. const batch = 64 var ( @@ -61,16 +52,13 @@ func dropProgressControl() { runtime.SetFinalizer(p, func(*blockedObject) { controlRan.Add(1) }) } -// testPermanentlyBlockedStacks verifies that permanent blocks preserve their -// suspended stacks. The control object is intentionally unreachable and is -// deterministic on precise wasm; once its finalizer runs, GC and finalizer -// progress are proven without sleeps. A blocked-object finalizer running by -// then therefore means its task was incorrectly treated as completed. +// testPermanentlyBlockedStacks checks that blocked task stacks remain GC roots. +// A control finalizer confirms that GC and finalizer processing made progress. func testPermanentlyBlockedStacks() { ready := make(chan struct{}, 3) go holdWhileBlocked(0, ready, nil) // select{} - go holdWhileBlocked(1, ready, nil) // nil-channel send - go holdWhileBlocked(2, ready, nil) // nil-channel receive + go holdWhileBlocked(1, ready, nil) // nil channel send + go holdWhileBlocked(2, ready, nil) // nil channel receive <-ready <-ready <-ready @@ -84,8 +72,8 @@ func testPermanentlyBlockedStacks() { if controlRan.Load() != 1 { panic("control finalizer did not prove GC progress") } - // Collect once more so transient scheduler roots cannot mask an unrooted - // blocked task during the progress-control collection. + // Collect once more so temporary scheduler roots cannot hide an unrooted + // blocked task during the control collection. runtime.GC() runtime.Gosched() for i, name := range [...]string{"select{}", "nil-channel send", "nil-channel receive"} { @@ -95,11 +83,8 @@ func testPermanentlyBlockedStacks() { } } -// scrubStack overwrites the stack region used by an alloc-and-drop helper with -// non-pointer words. It is called at the same depth as that helper so this -// recursion reuses (and clears) the frame that just held the dropped pointers; -// otherwise a stale copy keeps an object marked and it is never collected. The -// returned value derived from buf keeps the writes live. +// scrubStack removes stale pointers from the helper frame so collection is deterministic. +// Call it at the same call depth as the allocation helper. // //go:noinline func scrubStack(depth int) int { @@ -114,10 +99,8 @@ func scrubStack(depth int) int { return scrubStack(depth-1) + buf[0] } -// registerAndDrop registers `batch` finalizers and returns without leaking any -// reference to the objects, so they become unreachable. The finalizer must not -// capture its object (that would pin it forever): it takes the pointer as its -// argument and touches only a package global. +// registerAndDrop creates unreachable objects with finalizers that do not capture them. +// This allows the idle GC to collect the objects. // //go:noinline func registerAndDrop() { @@ -127,9 +110,6 @@ func registerAndDrop() { } } -// testIdleCollect checks that registering many finalizers and then only parking -// the goroutine (time.Sleep, never runtime.GC()) is enough for the objects to be -// collected and their finalizers to run. func testIdleCollect() { registerAndDrop() for i := 0; i < 500 && ranDropped < batch; i++ { @@ -141,11 +121,6 @@ func testIdleCollect() { } } -// testFinishedGoroutineStacks checks that a goroutine which registers a finalizer -// on a stack-local object and then returns no longer pins that object: once the -// goroutine has finished, the idle collection reclaims the object. Without -// zeroing a finished goroutine's conservatively scanned stack, the stale pointer -// would keep the object alive. func testFinishedGoroutineStacks() { done := make(chan struct{}) for i := 0; i < batch; i++ { @@ -168,11 +143,8 @@ func testFinishedGoroutineStacks() { } } -// launchArgGoroutine allocates a finalized object and launches a goroutine that -// receives it as an argument, then returns without leaving any reference behind. -// The object reaches the goroutine only through its argument bundle, and the -// only transient copies (of the pointer and the bundle) live in this frame, which -// returns immediately so the later scrubStack recursion reuses and clears it. +// launchArgGoroutine passes an object through the task argument bundle. +// The caller returns so scrubStack can remove its transient pointer. // //go:noinline func launchArgGoroutine(done chan struct{}) { @@ -184,11 +156,6 @@ func launchArgGoroutine(done chan struct{}) { }(p) } -// testFinishedGoroutineArgs checks that a goroutine which receives a finalized -// object as an argument no longer pins it once finished: the argument bundle the -// goroutine was launched with is dropped when it completes, so the idle collection -// reclaims the object. Without clearing a finished goroutine's args pointer the -// bundle would keep the object alive even after its stack has been zeroed. func testFinishedGoroutineArgs() { done := make(chan struct{}) for i := 0; i < batch; i++ { diff --git a/testdata/finalizerinvariants.go b/testdata/finalizerinvariants.go index 8492d16dbc..9939f27eac 100644 --- a/testdata/finalizerinvariants.go +++ b/testdata/finalizerinvariants.go @@ -1,17 +1,7 @@ package main -// Portable negative invariants of runtime.SetFinalizer on the block GCs. -// Conservative stack scanning may retain an unreachable object indefinitely, -// so this test never requires a dropped object's finalizer to run. On targets -// that do collect one, its callbacks reject invalid behavior: cleared and -// replaced finalizers running, reachable objects being finalized, or one -// registration running more than once. -// -// The harness builds this file with runtime_asserts. Those checks -// deterministically validate clear/replace behavior and agreement between the -// finalizer table, count, and registration bitmap during these operations and -// collections. Bookkeeping coverage therefore does not depend on a conservative -// collector reclaiming any particular dropped object. +// Test finalizer invariants that do not require unreachable objects to be collected. +// runtime_asserts checks the finalizer table, count, and bitmap. import ( "runtime" @@ -65,9 +55,8 @@ func main() { keepReachable(i) } - // Exercise scan-time bookkeeping assertions and give finalizers bounded - // opportunities to run on targets which collect the dropped objects. No - // assertion below requires one of them to have been collected. + // Run GC to check bookkeeping and give finalizers bounded opportunities to run. + // The checks do not require finalization of an unreachable object. for i := 0; i < 8; i++ { runtime.GC() runtime.Gosched()