From e31d383e8d839354812886696962e40aa0489f97 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Fri, 24 Oct 2025 16:22:19 +0200 Subject: [PATCH 1/8] removed goroutineProfile semaphore from goroutine leak profile collection. --- src/runtime/mprof.go | 23 ++-- src/runtime/pprof/pprof_test.go | 229 +++++++++++++++++++++++++++++++- 2 files changed, 239 insertions(+), 13 deletions(-) diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index d745d5f5b95885..a59fff5bf1522a 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1261,7 +1261,7 @@ func goroutineProfileWithLabels(p []profilerecord.StackRecord, labels []unsafe.P //go:linkname pprof_goroutineLeakProfileWithLabels func pprof_goroutineLeakProfileWithLabels(p []profilerecord.StackRecord, labels []unsafe.Pointer) (n int, ok bool) { - return goroutineLeakProfileWithLabelsConcurrent(p, labels) + return goroutineLeakProfileWithLabels(p, labels) } // labels may be nil. If labels is non-nil, it must have the same length as p. @@ -1323,30 +1323,30 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab return work.goroutineLeak.count, false } - // Use the same semaphore as goroutineProfileWithLabelsConcurrent, - // because ultimately we still use goroutine profiles. - semacquire(&goroutineProfile.sema) - // Unlike in goroutineProfileWithLabelsConcurrent, we don't need to // save the current goroutine stack, because it is obviously not leaked. - + // We also do not need acquire any semaphores on goroutineProfile, because + // we don't use it for storage. pcbuf := makeProfStack() // see saveg() for explanation // Prepare a profile large enough to store all leaked goroutines. n = work.goroutineLeak.count if n > len(p) { - // There's not enough space in p to store the whole profile, so (per the - // contract of runtime.GoroutineProfile) we're not allowed to write to p - // at all and must return n, false. - semrelease(&goroutineProfile.sema) + // There's not enough space in p to store the whole profile, so + // we're not allowed to write to p at all and must return n, false. return n, false } // Visit each leaked goroutine and try to record its stack. + var offset int forEachGRace(func(gp1 *g) { if readgstatus(gp1) == _Gleaked { - doRecordGoroutineProfile(gp1, pcbuf) + systemstack(func() { saveg(^uintptr(0), ^uintptr(0), gp1, &p[offset], pcbuf) }) + if labels != nil { + labels[offset] = gp1.labels + } + offset++ } }) @@ -1354,7 +1354,6 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab raceacquire(unsafe.Pointer(&labelSync)) } - semrelease(&goroutineProfile.sema) return n, true } diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index b816833e52285f..7f807e9821350c 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -11,6 +11,7 @@ import ( "context" "fmt" "internal/abi" + "internal/goexperiment" "internal/profile" "internal/syscall/unix" "internal/testenv" @@ -774,7 +775,11 @@ func TestMorestack(t *testing.T) { for { go func() { growstack1() - c <- true + // NOTE(vsaioc): This goroutine may leak without this select. + select { + case c <- true: + case <-time.After(duration): + } }() select { case <-t: @@ -1565,6 +1570,228 @@ func containsCountsLabels(prof *profile.Profile, countLabels map[int64]map[strin return true } +func goroutineLeakExample() { + <-make(chan struct{}) + panic("unreachable") +} + +func TestGoroutineLeakProfileConcurrency(t *testing.T) { + if !goexperiment.GoroutineLeakProfile { + // Do not run this test if the experimental flag is not enabled. + t.Skip("goroutine leak profile is not enabled") + } + const leakCount = 3 + + testenv.MustHaveParallelism(t) + regexLeakCount := regexp.MustCompile("goroutineleak profile: total ") + whiteSpace := regexp.MustCompile("\\s+") + + // Regular goroutine profile. Used to check that there is no interference between + // the two profile types. + goroutineProf := Lookup("goroutine") + goroutineLeakProf := Lookup("goroutineleak") + + // Check that a profile with debug information contains + includesLeak := func(t *testing.T, name, s string) { + if !strings.Contains(s, "runtime/pprof.goroutineLeakExample") { + t.Errorf("%s profile does not contain expected leaked goroutine (runtime/pprof.goroutineLeakExample): %s", name, s) + } + } + + checkFrame := func(i int, j int, locations []*profile.Location, expectedFunctionName string) { + if len(locations) <= i { + t.Errorf("leaked goroutine stack locations out of range at %d of %d", i+1, len(locations)) + return + } + location := locations[i] + if len(location.Line) <= j { + t.Errorf("leaked goroutine stack location lines out of range at %d of %d", j+1, len(location.Line)) + return + } + if location.Line[j].Function.Name != expectedFunctionName { + t.Errorf("leaked goroutine stack expected %s as the location[%d].Line[%d] but found %s (%s:%d)", expectedFunctionName, i, j, location.Line[j].Function.Name, location.Line[j].Function.Filename, location.Line[j].Line) + } + } + + // We use this helper to count the total number of leaked goroutines in the profile. + // + // NOTE(vsaioc): This value should match for the number of leaks produced in this test, + // but other tests could also leak goroutines, in which case we would have a mismatch + // when bulk-running tests. + // + // The two mismatching outcomes are therefore: + // - More leaks than expected, which is a correctness issue with other tests. + // In this case, this test effectively checks other tests wrt + // goroutine leaks running tests in bulk (e.g., by running all.bash). + // + // - Fewer leaks than expected; this is an unfortunate symptom of scheduling + // non-determinism, which may occur once in a blue moon. We make + // a best-effort attempt to allow the expected leaks to occur, by yielding + // the main thread, but it is never a guarantee. + countLeaks := func(t *testing.T, number int, s string) { + // Strip the profile header + parts := regexLeakCount.Split(s, -1) + if len(parts) < 2 { + t.Fatalf("goroutineleak profile does not contain 'goroutineleak profile: total ': %s\nparts: %v", s, parts) + return + } + + parts = whiteSpace.Split(parts[1], -1) + + count, err := strconv.ParseInt(parts[0], 10, 64) + if err != nil { + t.Fatalf("goroutineleak profile count is not a number: %s\nerror: %v", s, err) + } + + // Check that the total number of leaked goroutines is exactly the expected number. + if count != int64(number) { + t.Errorf("goroutineleak profile does not contain exactly %d leaked goroutines: %d", number, count) + } + } + + checkLeakStack := func(t *testing.T) func(pc uintptr, locations []*profile.Location, _ map[string][]string) { + return func(pc uintptr, locations []*profile.Location, _ map[string][]string) { + if pc != leakCount { + t.Errorf("expected %d leaked goroutines with specific stack configurations, but found %d", leakCount, pc) + return + } + switch len(locations) { + case 4: + // We expect a receive operation. This is the typical stack. + checkFrame(0, 0, locations, "runtime.gopark") + checkFrame(1, 0, locations, "runtime.chanrecv") + checkFrame(2, 0, locations, "runtime.chanrecv1") + switch len(locations[3].Line) { + case 2: + // Running `go func() { goroutineLeakExample() }()` will produce a stack with 2 lines. + // The anonymous function will have the call to goroutineLeakExample inlined. + checkFrame(3, 1, locations, "runtime/pprof.TestGoroutineLeakProfileConcurrency.func5") + fallthrough + case 1: + // Running `go goroutineLeakExample()` will produce a stack with 1 line. + checkFrame(3, 0, locations, "runtime/pprof.goroutineLeakExample") + default: + t.Errorf("leaked goroutine stack location expected 1 or 2 lines in the 4th location but found %d", len(locations[3].Line)) + return + } + default: + message := fmt.Sprintf("leaked goroutine stack expected 4 or 5 locations but found %d", len(locations)) + for _, location := range locations { + for _, line := range location.Line { + message += fmt.Sprintf("\n%s:%d", line.Function.Name, line.Line) + } + } + t.Errorf("%s", message) + } + } + } + // Leak some goroutines that will feature in the goroutine leak profile + for i := 0; i < leakCount; i++ { + go goroutineLeakExample() + go func() { + // Leak another goroutine that will feature a slightly different stack. + // This includes the frame runtime/pprof.TestGoroutineLeakProfileConcurrency.func1. + goroutineLeakExample() + panic("unreachable") + }() + // Yield several times to allow the goroutines to leak. + runtime.Gosched() + runtime.Gosched() + } + + // Give all goroutines a chance to leak. + time.Sleep(time.Second) + + t.Run("profile contains leak", func(t *testing.T) { + var w strings.Builder + goroutineLeakProf.WriteTo(&w, 0) + parseProfile(t, []byte(w.String()), checkLeakStack(t)) + }) + + t.Run("leak persists between sequential profiling runs", func(t *testing.T) { + for i := 0; i < 2; i++ { + var w strings.Builder + goroutineLeakProf.WriteTo(&w, 0) + parseProfile(t, []byte(w.String()), checkLeakStack(t)) + } + }) + + // Concurrent calls to the goroutine leak profiler should not trigger data races + // or corruption. + t.Run("overlapping profile requests", func(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + Do(ctx, Labels("i", fmt.Sprint(i)), func(context.Context) { + go func() { + defer wg.Done() + for ctx.Err() == nil { + var w strings.Builder + goroutineLeakProf.WriteTo(&w, 1) + // NOTE(vsaioc): We cannot always guarantee that the leak will actually be recorded in + // the profile when making concurrent goroutine leak requests, because the GC runs + // concurrently with the profiler and may reset the leaked goroutine status before + // a concurrent profiler has the chance to record it. + // + // However, the goroutine leak count will persist. Still, we give some leeway by making + // it an inequality, just in case other tests in the suite start leaking goroutines. + countLeaks(t, 2*leakCount, w.String()) + } + }() + }) + } + wg.Wait() + }) + + // Concurrent calls to the goroutine leak profiler should not trigger data races + // or corruption, or interfere with regular goroutine profiles. + t.Run("overlapping goroutine and goroutine leak profile requests", func(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(2) + Do(ctx, Labels("i", fmt.Sprint(i)), func(context.Context) { + go func() { + defer wg.Done() + for ctx.Err() == nil { + var w strings.Builder + goroutineLeakProf.WriteTo(&w, 1) + // NOTE(vsaioc): We cannot always guarantee that the leak will + // actually be recorded in the profile during concurrent + // goroutine leak profile requests. The GC runs concurrently with + // the profiler and may reset the leaked goroutine status before + // the profiler has the chance to record the leaked stacks. + // + // However, the leaked goroutine count is not reset. + // Other tests are not expected to leak goroutines, + // so the leak count is expected to be consistent. + countLeaks(t, 2*leakCount, w.String()) + } + }() + go func() { + defer wg.Done() + for ctx.Err() == nil { + var w strings.Builder + goroutineProf.WriteTo(&w, 1) + // The regular goroutine profile should see the leaked + // goroutines. We simply check that the goroutine leak + // profile does not corrupt the goroutine profile state. + includesLeak(t, "goroutine", w.String()) + } + }() + }) + } + wg.Wait() + }) +} + func TestGoroutineProfileConcurrency(t *testing.T) { testenv.MustHaveParallelism(t) From 590b52c08c34b7ed723394e89e8489ffdd74b8b9 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Mon, 27 Oct 2025 10:54:06 +0100 Subject: [PATCH 2/8] Removed Goexperiment flag. --- src/runtime/pprof/pprof_test.go | 39 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 7f807e9821350c..f8e1e36dee5cd6 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -11,7 +11,6 @@ import ( "context" "fmt" "internal/abi" - "internal/goexperiment" "internal/profile" "internal/syscall/unix" "internal/testenv" @@ -1576,10 +1575,6 @@ func goroutineLeakExample() { } func TestGoroutineLeakProfileConcurrency(t *testing.T) { - if !goexperiment.GoroutineLeakProfile { - // Do not run this test if the experimental flag is not enabled. - t.Skip("goroutine leak profile is not enabled") - } const leakCount = 3 testenv.MustHaveParallelism(t) @@ -1620,12 +1615,12 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { // when bulk-running tests. // // The two mismatching outcomes are therefore: - // - More leaks than expected, which is a correctness issue with other tests. + // - More leaks than expected, which is a correctness issue with other tests. // In this case, this test effectively checks other tests wrt - // goroutine leaks running tests in bulk (e.g., by running all.bash). + // goroutine leaks during bulk executions (e.g., running all.bash). // - // - Fewer leaks than expected; this is an unfortunate symptom of scheduling - // non-determinism, which may occur once in a blue moon. We make + // - Fewer leaks than expected; this is an unfortunate symptom of scheduling + // non-determinism, which may occur once in a blue moon. We make // a best-effort attempt to allow the expected leaks to occur, by yielding // the main thread, but it is never a guarantee. countLeaks := func(t *testing.T, number int, s string) { @@ -1732,13 +1727,15 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { for ctx.Err() == nil { var w strings.Builder goroutineLeakProf.WriteTo(&w, 1) - // NOTE(vsaioc): We cannot always guarantee that the leak will actually be recorded in - // the profile when making concurrent goroutine leak requests, because the GC runs - // concurrently with the profiler and may reset the leaked goroutine status before - // a concurrent profiler has the chance to record it. + // NOTE(vsaioc): We cannot always guarantee that the leak will + // actually be recorded in the profile when making concurrent + // goroutine leak requests, because the GC runs concurrently with + // the profiler and may reset the leaked goroutines' status before + // a concurrent profiler has the chance to record them. However, + // the goroutine leak count will persist throughout. // - // However, the goroutine leak count will persist. Still, we give some leeway by making - // it an inequality, just in case other tests in the suite start leaking goroutines. + // Other tests are not expected to leak goroutines, + // so the count should be consistent. countLeaks(t, 2*leakCount, w.String()) } }() @@ -1764,14 +1761,14 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { var w strings.Builder goroutineLeakProf.WriteTo(&w, 1) // NOTE(vsaioc): We cannot always guarantee that the leak will - // actually be recorded in the profile during concurrent - // goroutine leak profile requests. The GC runs concurrently with - // the profiler and may reset the leaked goroutine status before - // the profiler has the chance to record the leaked stacks. + // actually be recorded in the profile when making concurrent + // goroutine leak requests, because the GC runs concurrently with + // the profiler and may reset the leaked goroutines' status before + // a concurrent profiler has the chance to record them. However, + // the goroutine leak count will persist throughout. // - // However, the leaked goroutine count is not reset. // Other tests are not expected to leak goroutines, - // so the leak count is expected to be consistent. + // so the count should be consistent. countLeaks(t, 2*leakCount, w.String()) } }() From 89f02ee61fadbf84a6e7366e6d40027f6b80b9fc Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Mon, 27 Oct 2025 12:41:50 +0100 Subject: [PATCH 3/8] Fixed accessing the goroutine leak profile through Lookup when experiment is off. --- src/runtime/pprof/pprof_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index f8e1e36dee5cd6..2a21f56386c691 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -1584,7 +1584,7 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { // Regular goroutine profile. Used to check that there is no interference between // the two profile types. goroutineProf := Lookup("goroutine") - goroutineLeakProf := Lookup("goroutineleak") + goroutineLeakProf := goroutineLeakProfile // Check that a profile with debug information contains includesLeak := func(t *testing.T, name, s string) { From cdb40531a4fb1303b47556612ea3e446d5b9e967 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Thu, 30 Oct 2025 11:20:49 +0100 Subject: [PATCH 4/8] Addressed comments. --- src/runtime/mprof.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index a59fff5bf1522a..a5a43f438062d4 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1341,7 +1341,32 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab // Visit each leaked goroutine and try to record its stack. var offset int forEachGRace(func(gp1 *g) { - if readgstatus(gp1) == _Gleaked { + // NOTE(vsaioc): Each goroutine leak profile request is preceded by a run of the GC + // in goroutine leak detection mode. At the beginning of the GC cycle, all previously + // reported goroutine leaks are reset to _Gwaiting. As a result, incomplete goroutine + // leak profiles may be produced if multiple goroutine leak profile requests are issued + // concurrently. + // + // Example trace: + // + // G1 | GC | G2 + // ----------------------+-----------------------------+--------------------- + // Request profile | . | . + // . | . | Request profile + // . | [G1] Resets leaked g status | . + // . | [G1] Leaks detected | . + // . | | . + // . | [G2] Resets leaked g status | . + // . | . | Write profile + // . | [G2] Leaks detected | . + // Write profile | . | . + // ----------------------+-----------------------------+--------------------- + // G1 completes profile |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| G2 misses leaks + // + // While this is a bug, normal use cases presume that goroutine leak profile + // requests are issued on a single track. Adding synchronization between over + // goroutine leak profile requests would only needlessly increase overhead. + if readgstatus(gp1)&^_Gscan == _Gleaked { systemstack(func() { saveg(^uintptr(0), ^uintptr(0), gp1, &p[offset], pcbuf) }) if labels != nil { labels[offset] = gp1.labels From 77e0b311c9dc4d24651b5c9a760b9dfdb9d498ba Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Thu, 30 Oct 2025 17:49:25 +0100 Subject: [PATCH 5/8] Fixed comment. --- src/runtime/mprof.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index a5a43f438062d4..38168398ec9adc 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1364,7 +1364,7 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab // G1 completes profile |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| G2 misses leaks // // While this is a bug, normal use cases presume that goroutine leak profile - // requests are issued on a single track. Adding synchronization between over + // requests are issued on a single track. Adding synchronization between // goroutine leak profile requests would only needlessly increase overhead. if readgstatus(gp1)&^_Gscan == _Gleaked { systemstack(func() { saveg(^uintptr(0), ^uintptr(0), gp1, &p[offset], pcbuf) }) From 233dd505b10a5f9deebf2088b838854fbc940d50 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Wed, 5 Nov 2025 12:59:56 +0100 Subject: [PATCH 6/8] Use lock to prevent racy profile writes. --- src/runtime/mprof.go | 25 ------------------------- src/runtime/pprof/pprof.go | 18 ++++++++++++++++++ src/runtime/pprof/pprof_test.go | 20 ++------------------ 3 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index 38168398ec9adc..12d2d6e3317567 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1341,31 +1341,6 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab // Visit each leaked goroutine and try to record its stack. var offset int forEachGRace(func(gp1 *g) { - // NOTE(vsaioc): Each goroutine leak profile request is preceded by a run of the GC - // in goroutine leak detection mode. At the beginning of the GC cycle, all previously - // reported goroutine leaks are reset to _Gwaiting. As a result, incomplete goroutine - // leak profiles may be produced if multiple goroutine leak profile requests are issued - // concurrently. - // - // Example trace: - // - // G1 | GC | G2 - // ----------------------+-----------------------------+--------------------- - // Request profile | . | . - // . | . | Request profile - // . | [G1] Resets leaked g status | . - // . | [G1] Leaks detected | . - // . | | . - // . | [G2] Resets leaked g status | . - // . | . | Write profile - // . | [G2] Leaks detected | . - // Write profile | . | . - // ----------------------+-----------------------------+--------------------- - // G1 completes profile |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| G2 misses leaks - // - // While this is a bug, normal use cases presume that goroutine leak profile - // requests are issued on a single track. Adding synchronization between - // goroutine leak profile requests would only needlessly increase overhead. if readgstatus(gp1)&^_Gscan == _Gleaked { systemstack(func() { saveg(^uintptr(0), ^uintptr(0), gp1, &p[offset], pcbuf) }) if labels != nil { diff --git a/src/runtime/pprof/pprof.go b/src/runtime/pprof/pprof.go index b524e992b8b209..23d3da7adc171f 100644 --- a/src/runtime/pprof/pprof.go +++ b/src/runtime/pprof/pprof.go @@ -228,6 +228,15 @@ var mutexProfile = &Profile{ write: writeMutex, } +// goroutineLeakProfileLock ensures that the goroutine leak profile writer observes the +// leaked goroutines discovered during the goroutine leak detection GC cycle +// that was triggered when the profile was requested. +// +// This is needed to prevent a race condition between the garbage collector +// and the goroutine leak profile writer when multiple profile requests are +// issued concurrently. +var goroutineLeakProfileLock sync.Mutex + func lockProfiles() { profiles.mu.Lock() if profiles.m == nil { @@ -763,6 +772,15 @@ func writeGoroutine(w io.Writer, debug int) error { // writeGoroutineLeak first invokes a GC cycle that performs goroutine leak detection. // It then writes the goroutine profile, filtering for leaked goroutines. func writeGoroutineLeak(w io.Writer, debug int) error { + // Acquire the goroutine leak detection lock and release + // it after the goroutine leak profile is written. + // + // While the critical section is long, this is needed to prevent + // a race condition between the garbage collector and the goroutine + // leak profile writer when multiple profile requests are issued concurrently. + goroutineLeakProfileLock.Lock() + defer goroutineLeakProfileLock.Unlock() + // Run the GC with leak detection first so that leaked goroutines // may transition to the leaked state. runtime_goroutineLeakGC() diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 2a21f56386c691..496716a78e766f 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -1727,16 +1727,8 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { for ctx.Err() == nil { var w strings.Builder goroutineLeakProf.WriteTo(&w, 1) - // NOTE(vsaioc): We cannot always guarantee that the leak will - // actually be recorded in the profile when making concurrent - // goroutine leak requests, because the GC runs concurrently with - // the profiler and may reset the leaked goroutines' status before - // a concurrent profiler has the chance to record them. However, - // the goroutine leak count will persist throughout. - // - // Other tests are not expected to leak goroutines, - // so the count should be consistent. countLeaks(t, 2*leakCount, w.String()) + includesLeak(t, "goroutineleak", w.String()) } }() }) @@ -1760,16 +1752,8 @@ func TestGoroutineLeakProfileConcurrency(t *testing.T) { for ctx.Err() == nil { var w strings.Builder goroutineLeakProf.WriteTo(&w, 1) - // NOTE(vsaioc): We cannot always guarantee that the leak will - // actually be recorded in the profile when making concurrent - // goroutine leak requests, because the GC runs concurrently with - // the profiler and may reset the leaked goroutines' status before - // a concurrent profiler has the chance to record them. However, - // the goroutine leak count will persist throughout. - // - // Other tests are not expected to leak goroutines, - // so the count should be consistent. countLeaks(t, 2*leakCount, w.String()) + includesLeak(t, "goroutineleak", w.String()) } }() go func() { From 7130fe45734af773f3d5e76fd66b62b055097580 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Wed, 5 Nov 2025 18:10:30 +0100 Subject: [PATCH 7/8] Removed useless comment. --- src/runtime/mprof.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go index 12d2d6e3317567..f1703a7ebab802 100644 --- a/src/runtime/mprof.go +++ b/src/runtime/mprof.go @@ -1323,10 +1323,6 @@ func goroutineLeakProfileWithLabelsConcurrent(p []profilerecord.StackRecord, lab return work.goroutineLeak.count, false } - // Unlike in goroutineProfileWithLabelsConcurrent, we don't need to - // save the current goroutine stack, because it is obviously not leaked. - // We also do not need acquire any semaphores on goroutineProfile, because - // we don't use it for storage. pcbuf := makeProfStack() // see saveg() for explanation // Prepare a profile large enough to store all leaked goroutines. From 5e9eb3b1d80c4d2d9b668a01f6b39a7b42f7bb45 Mon Sep 17 00:00:00 2001 From: Vlad Saioc Date: Thu, 6 Nov 2025 10:48:35 +0100 Subject: [PATCH 8/8] Re-added trace example comment on goroutine leak profile lock. --- src/runtime/pprof/pprof.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/runtime/pprof/pprof.go b/src/runtime/pprof/pprof.go index 23d3da7adc171f..78d00af6cac2fd 100644 --- a/src/runtime/pprof/pprof.go +++ b/src/runtime/pprof/pprof.go @@ -230,11 +230,27 @@ var mutexProfile = &Profile{ // goroutineLeakProfileLock ensures that the goroutine leak profile writer observes the // leaked goroutines discovered during the goroutine leak detection GC cycle -// that was triggered when the profile was requested. +// that was triggered by the profile request. +// This prevents a race condition between the garbage collector and the profile writer +// when multiple profile requests are issued concurrently: the status of leaked goroutines +// is reset to _Gwaiting at the beginning of a leak detection cycle, which may lead the +// profile writer of another concurrent request to produce an incomplete profile. // -// This is needed to prevent a race condition between the garbage collector -// and the goroutine leak profile writer when multiple profile requests are -// issued concurrently. +// Example trace: +// +// G1 | GC | G2 +// ----------------------+-----------------------------+--------------------- +// Request profile | . | . +// . | . | Request profile +// . | [G1] Resets leaked g status | . +// . | [G1] Leaks detected | . +// . | | . +// . | [G2] Resets leaked g status | . +// Write profile | . | . +// . | [G2] Leaks detected | . +// . | . | Write profile +// ----------------------+-----------------------------+--------------------- +// Incomplete profile |+++++++++++++++++++++++++++++| Complete profile var goroutineLeakProfileLock sync.Mutex func lockProfiles() {