Skip to content

Commit 80c91ee

Browse files
mknyszekgopherbot
authored andcommitted
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 <[email protected]> Auto-Submit: Michael Knyszek <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 7a8d0b5 commit 80c91ee

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

src/runtime/mheap.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2534,7 +2534,15 @@ func getOrAddWeakHandle(p unsafe.Pointer) *atomic.Uintptr {
25342534
s := (*specialWeakHandle)(mheap_.specialWeakHandleAlloc.alloc())
25352535
unlock(&mheap_.speciallock)
25362536

2537-
handle := new(atomic.Uintptr)
2537+
// N.B. Pad the weak handle to ensure it doesn't share a tiny
2538+
// block with any other allocations. This can lead to leaks, such
2539+
// as in go.dev/issue/76007. As an alternative, we could consider
2540+
// using the currently-unused 8-byte noscan size class.
2541+
type weakHandleBox struct {
2542+
h atomic.Uintptr
2543+
_ [maxTinySize - unsafe.Sizeof(atomic.Uintptr{})]byte
2544+
}
2545+
handle := &(new(weakHandleBox).h)
25382546
s.special.kind = _KindSpecialWeakHandle
25392547
s.handle = handle
25402548
handle.Store(uintptr(p))

src/weak/pointer_test.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import (
1616
)
1717

1818
type T struct {
19-
// N.B. This must contain a pointer, otherwise the weak handle might get placed
20-
// in a tiny block making the tests in this package flaky.
19+
// N.B. T is what it is to avoid having test values get tiny-allocated
20+
// in the same block as the weak handle, but since the fix to
21+
// go.dev/issue/76007, this should no longer be possible.
22+
// TODO(mknyszek): Consider using tiny-allocated values for all the tests.
2123
t *T
2224
a int
2325
b int
@@ -327,3 +329,37 @@ func TestImmortalPointer(t *testing.T) {
327329
t.Errorf("immortal weak pointer to %p has unexpected Value %p", want, got)
328330
}
329331
}
332+
333+
func TestPointerTiny(t *testing.T) {
334+
runtime.GC() // Clear tiny-alloc caches.
335+
336+
const N = 1000
337+
wps := make([]weak.Pointer[int], N)
338+
for i := range N {
339+
// N.B. *x is just an int, so the value is very likely
340+
// tiny-allocated alongside the weak handle, assuming bug
341+
// from go.dev/issue/76007 exists.
342+
x := new(int)
343+
*x = i
344+
wps[i] = weak.Make(x)
345+
}
346+
347+
// Get the cleanups to start running.
348+
runtime.GC()
349+
350+
// Expect at least 3/4ths of the weak pointers to have gone nil.
351+
//
352+
// Note that we provide some leeway since it's possible our allocation
353+
// gets grouped with some other long-lived tiny allocation, but this
354+
// shouldn't be the case for the vast majority of allocations.
355+
n := 0
356+
for _, wp := range wps {
357+
if wp.Value() == nil {
358+
n++
359+
}
360+
}
361+
const want = 3 * N / 4
362+
if n < want {
363+
t.Fatalf("not enough weak pointers are nil: expected at least %v, got %v", want, n)
364+
}
365+
}

0 commit comments

Comments
 (0)