diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go index d7f41334cd6b62..586610727563c2 100644 --- a/src/runtime/metrics_test.go +++ b/src/runtime/metrics_test.go @@ -1290,3 +1290,38 @@ func (w *contentionWorker) run() { for w.fn() { } } + +func TestMetricHeapUnusedLargeObjectOverflow(t *testing.T) { + // This test makes sure /memory/classes/heap/unused:bytes + // doesn't overflow when allocating and deallocating large + // objects. It is a regression test for #67019. + done := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + for i := 0; i < 10; i++ { + runtime.Escape(make([]byte, 1<<20)) + } + runtime.GC() + select { + case <-done: + return + default: + } + } + }() + s := []metrics.Sample{ + {Name: "/memory/classes/heap/unused:bytes"}, + } + for i := 0; i < 1000; i++ { + metrics.Read(s) + if s[0].Value.Uint64() > 1<<40 { + t.Errorf("overflow") + break + } + } + done <- struct{}{} + wg.Wait() +} diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 3dbe9bcec72fb1..35be7949472f8c 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -770,6 +770,19 @@ func (sl *sweepLocked) sweep(preserve bool) bool { if nfreed != 0 { // Free large object span to heap. + // Count the free in the consistent, external stats. + // + // Do this before freeSpan, which might update heapStats' inHeap + // value. If it does so, then metrics that subtract object footprint + // from inHeap might overflow. See #67019. + stats := memstats.heapStats.acquire() + atomic.Xadd64(&stats.largeFreeCount, 1) + atomic.Xadd64(&stats.largeFree, int64(size)) + memstats.heapStats.release() + + // Count the free in the inconsistent, internal stats. + gcController.totalFree.Add(int64(size)) + // NOTE(rsc,dvyukov): The original implementation of efence // in CL 22060046 used sysFree instead of sysFault, so that // the operating system would eventually give the memory @@ -802,16 +815,6 @@ func (sl *sweepLocked) sweep(preserve bool) bool { // invalid pointer. See arena.go:(*mheap).allocUserArenaChunk. *(*uintptr)(unsafe.Pointer(&s.largeType)) = 0 } - - // Count the free in the consistent, external stats. - stats := memstats.heapStats.acquire() - atomic.Xadd64(&stats.largeFreeCount, 1) - atomic.Xadd64(&stats.largeFree, int64(size)) - memstats.heapStats.release() - - // Count the free in the inconsistent, internal stats. - gcController.totalFree.Add(int64(size)) - return true }