From: Dmitry Vyukov Date: Tue, 3 Feb 2015 10:01:09 +0000 (+0300) Subject: runtime: fix false race report during tracing X-Git-Tag: go1.5beta1~2170 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=4a45ac577f4739916b98f959015ce6a234327457;p=gostls13.git runtime: fix false race report during tracing 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 --- 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