Use lock to prevent racy profile writes.

Author: VladSaiocUber

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         | .
-		//	.                     | <New cycle>                 | .
-		//	.                     | [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 {

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()

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() {