From 1e1ea66991ce840d6f52cb8385e23624f16e9f01 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 14 Dec 2015 15:19:07 -0500 Subject: [PATCH] runtime: print gctrace before releasing worldsema Currently we drop worldsema and then print the gctrace. We did this so that if stderr is a pipe or a blocked terminal, blocking on printing the gctrace would not block another GC from starting. However, this is a bit of a fool's errand because a blocked runtime print will block the whole M/P, so after GOMAXPROCS GC cycles, the whole system will freeze. Furthermore, now this is much less of an issue because allocation will block indefinitely if it can't start a GC (whereas it used to be that allocation could run away). Finally, this allows another GC cycle to start while the previous cycle is printing the gctrace, which leads to races on reading various statistics to print them and the next GC cycle overwriting those statistics. Fix this by moving the release of worldsema after the gctrace print. Change-Id: I3d044ea0f77d80f3b4050af6b771e7912258662a Reviewed-on: https://go-review.googlesource.com/17812 Run-TryBot: Austin Clements Reviewed-by: Rick Hudson TryBot-Result: Gobot Gobot Reviewed-by: Russ Cox --- src/runtime/mgc.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 878734b377..756d74e4fd 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1254,11 +1254,9 @@ func gcMarkTermination() { // Free stack spans. This must be done between GC cycles. systemstack(freeStackSpans) - semrelease(&worldsema) - - releasem(mp) - mp = nil - + // Print gctrace before dropping worldsema. As soon as we drop + // worldsema another cycle could start and smash the stats + // we're trying to print. if debug.gctrace > 0 { util := int(memstats.gc_cpu_fraction * 100) @@ -1306,6 +1304,12 @@ func gcMarkTermination() { printunlock() } + semrelease(&worldsema) + // Careful: another GC cycle may start now. + + releasem(mp) + mp = nil + // now that gc is done, kick off finalizer thread if needed if !concurrentSweep { // give the queued finalizers, if any, a chance to run -- 2.50.0