From 4a45ac577f4739916b98f959015ce6a234327457 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 3 Feb 2015 13:01:09 +0300 Subject: [PATCH] runtime: fix false race report during tracing MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently race detector produces the following reports on pprof tests: WARNING: DATA RACE Read by goroutine 4: runtime/pprof_test.TestTraceStartStop() src/runtime/pprof/trace_test.go:38 +0x1da testing.tRunner() src/testing/testing.go:448 +0x13a Previous write by goroutine 5: bytes.(*Buffer).grow() src/bytes/buffer.go:102 +0x190 bytes.(*Buffer).Write() src/bytes/buffer.go:127 +0x75 runtime/pprof.func·002() src/runtime/pprof/pprof.go:633 +0xae Trace writer goroutine synchronizes with StopTrace using trace.shutdownSema runtime semaphore. But race detector does not see that synchronization and so produces false reports. Teach race detector about the synchronization. Change-Id: I1219817325d4e16b423f29a0cbee94c929793881 Reviewed-on: https://go-review.googlesource.com/3746 Reviewed-by: Russ Cox --- src/runtime/trace.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 5b168c7bfc..e7937b3d17 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -231,6 +231,9 @@ func StopTrace() { // The world is started but we've set trace.shutdown, so new tracing can't start. // Wait for the trace reader to flush pending buffers and stop. semacquire(&trace.shutdownSema, false) + if raceenabled { + raceacquire(unsafe.Pointer(&trace.shutdownSema)) + } // The lock protects us from races with StartTrace/StopTrace because they do stop-the-world. lock(&trace.lock) @@ -331,6 +334,12 @@ func ReadTrace() []byte { if trace.shutdown { trace.lockOwner = nil unlock(&trace.lock) + if raceenabled { + // Model synchronization on trace.shutdownSema, which race + // detector does not see. This is required to avoid false + // race reports on writer passed to pprof.StartTrace. + racerelease(unsafe.Pointer(&trace.shutdownSema)) + } // trace.enabled is already reset, so can call traceable functions. semrelease(&trace.shutdownSema) return nil -- 2.50.0