From 80c91eedbbf3c041e9b0c03c28fae2c73dbb7df4 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 7 Nov 2025 18:22:29 +0000 Subject: [PATCH] runtime: ensure weak handles end up in their own allocation Currently weak handles are atomic.Uintptr values that may end up in a tiny block which can cause all sorts of surprising leaks. See #76007 for one example. This change pads out the underlying allocation of the atomic.Uintptr to 16 bytes to ensure we bypass the tiny allocator, and it gets its own block. This wastes 8 bytes per weak handle. We could potentially do better by using the 8 byte noscan size class, but this can be a follow-up change. For #76007. Change-Id: I3ab74dda9bf312ea0007f167093052de28134944 Reviewed-on: https://go-review.googlesource.com/c/go/+/719960 Reviewed-by: Cherry Mui Auto-Submit: Michael Knyszek LUCI-TryBot-Result: Go LUCI --- src/runtime/mheap.go | 10 +++++++++- src/weak/pointer_test.go | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 711c7790eb..3334099092 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -2534,7 +2534,15 @@ func getOrAddWeakHandle(p unsafe.Pointer) *atomic.Uintptr { s := (*specialWeakHandle)(mheap_.specialWeakHandleAlloc.alloc()) unlock(&mheap_.speciallock) - handle := new(atomic.Uintptr) + // N.B. Pad the weak handle to ensure it doesn't share a tiny + // block with any other allocations. This can lead to leaks, such + // as in go.dev/issue/76007. As an alternative, we could consider + // using the currently-unused 8-byte noscan size class. + type weakHandleBox struct { + h atomic.Uintptr + _ [maxTinySize - unsafe.Sizeof(atomic.Uintptr{})]byte + } + handle := &(new(weakHandleBox).h) s.special.kind = _KindSpecialWeakHandle s.handle = handle handle.Store(uintptr(p)) diff --git a/src/weak/pointer_test.go b/src/weak/pointer_test.go index da464a8d01..5e8b9bef58 100644 --- a/src/weak/pointer_test.go +++ b/src/weak/pointer_test.go @@ -16,8 +16,10 @@ import ( ) type T struct { - // N.B. This must contain a pointer, otherwise the weak handle might get placed - // in a tiny block making the tests in this package flaky. + // N.B. T is what it is to avoid having test values get tiny-allocated + // in the same block as the weak handle, but since the fix to + // go.dev/issue/76007, this should no longer be possible. + // TODO(mknyszek): Consider using tiny-allocated values for all the tests. t *T a int b int @@ -327,3 +329,37 @@ func TestImmortalPointer(t *testing.T) { t.Errorf("immortal weak pointer to %p has unexpected Value %p", want, got) } } + +func TestPointerTiny(t *testing.T) { + runtime.GC() // Clear tiny-alloc caches. + + const N = 1000 + wps := make([]weak.Pointer[int], N) + for i := range N { + // N.B. *x is just an int, so the value is very likely + // tiny-allocated alongside the weak handle, assuming bug + // from go.dev/issue/76007 exists. + x := new(int) + *x = i + wps[i] = weak.Make(x) + } + + // Get the cleanups to start running. + runtime.GC() + + // Expect at least 3/4ths of the weak pointers to have gone nil. + // + // Note that we provide some leeway since it's possible our allocation + // gets grouped with some other long-lived tiny allocation, but this + // shouldn't be the case for the vast majority of allocations. + n := 0 + for _, wp := range wps { + if wp.Value() == nil { + n++ + } + } + const want = 3 * N / 4 + if n < want { + t.Fatalf("not enough weak pointers are nil: expected at least %v, got %v", want, n) + } +} -- 2.52.0