runtime: don't hold trace.lock over semrelease in readTrace0

semrelease may unblock a goroutine, but the act of unblocking a
goroutine may emit an event, which in turn may try to acquire trace.lock
again.

It's safe to release trace.lock in readTrace0 for this because all of
the state (one variable) it uses under the lock will be recomputed when
it reacquires the lock. There's also no other synchronization
requirement to hold trace.lock. This is just a mistake.

Change-Id: Iff6c6b02efa298ebed8e60cdf6539ec161d5ec48
Reviewed-on: https://go-review.googlesource.com/c/go/+/544178
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/runtime/trace2.go b/src/runtime/trace2.go
index 59ea19008934dfdd36fb83887efd44c9d1b57991..c2a1c1ca1e19e42f56e58a2bbcaf5e9a2d81287f 100644 (file)
--- a/src/runtime/trace2.go
+++ b/src/runtime/trace2.go
@@ -764,6 +764,8 @@ func readTrace0() (buf []byte, park bool) {
                // can continue to advance.
                if trace.flushedGen.Load() == gen {
                        if trace.shutdown.Load() {
+                               unlock(&trace.lock)
+
                                // Wake up anyone waiting for us to be done with this generation.
                                //
                                // Do this after reading trace.shutdown, because the thread we're
@@ -778,13 +780,13 @@ func readTrace0() (buf []byte, park bool) {
 
                                // We're shutting down, and the last generation is fully
                                // read. We're done.
-                               unlock(&trace.lock)
                                return nil, false
                        }
                        // The previous gen has had all of its buffers flushed, and
                        // there's nothing else for us to read. Advance the generation
                        // we're reading from and try again.
                        trace.readerGen.Store(trace.gen.Load())
+                       unlock(&trace.lock)
 
                        // Wake up anyone waiting for us to be done with this generation.
                        //
@@ -795,6 +797,9 @@ func readTrace0() (buf []byte, park bool) {
                                racerelease(unsafe.Pointer(&trace.doneSema[gen%2]))
                        }
                        semrelease(&trace.doneSema[gen%2])
+
+                       // Reacquire the lock and go back to the top of the loop.
+                       lock(&trace.lock)
                        continue
                }
                // Wait for new data.