From b0b1a660526925d39c5c31e18df68db4d5b6687a Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 24 Apr 2015 22:33:13 -0400 Subject: [PATCH] runtime: reset spinning in mspinning if work was ready()ed This fixes a bug where the runtime ready()s a goroutine while setting up a new M that's initially marked as spinning, causing the scheduler to later panic when it finds work in the run queue of a P associated with a spinning M. Specifically, the sequence of events that can lead to this is: 1) sysmon calls handoffp to hand off a P stolen from a syscall. 2) handoffp sees no pending work on the P, so it calls startm with spinning set. 3) startm calls newm, which in turn calls allocm to allocate a new M. 4) allocm "borrows" the P we're handing off in order to do allocation and performs this allocation. 5) This allocation may assist the garbage collector, and this assist may detect the end of concurrent mark and ready() the main GC goroutine to signal this. 6) This ready()ing puts the GC goroutine on the run queue of the borrowed P. 7) newm starts the OS thread, which runs mstart and subsequently mstart1, which marks the M spinning because startm was called with spinning set. 8) mstart1 enters the scheduler, which panics because there's work on the run queue, but the M is marked spinning. To fix this, before marking the M spinning in step 7, add a check to see if work was been added to the P's run queue. If this is the case, undo the spinning instead. Fixes #10573. Change-Id: I4670495ae00582144a55ce88c45ae71de597cfa5 Reviewed-on: https://go-review.googlesource.com/9332 Reviewed-by: Russ Cox Run-TryBot: Austin Clements --- src/runtime/proc1.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go index 350d6bfbdf..aced04aa88 100644 --- a/src/runtime/proc1.go +++ b/src/runtime/proc1.go @@ -1031,7 +1031,17 @@ retry: } func mspinning() { - getg().m.spinning = true + gp := getg() + if !runqempty(gp.m.nextp.ptr()) { + // Something (presumably the GC) was readied while the + // runtime was starting up this M, so the M is no + // longer spinning. + if int32(xadd(&sched.nmspinning, -1)) < 0 { + throw("mspinning: nmspinning underflowed") + } + } else { + gp.m.spinning = true + } } // Schedules some M to run the p (creates an M if necessary). @@ -1066,6 +1076,9 @@ func startm(_p_ *p, spinning bool) { if mp.nextp != 0 { throw("startm: m has p") } + if spinning && !runqempty(_p_) { + throw("startm: p has runnable gs") + } mp.spinning = spinning mp.nextp.set(_p_) notewakeup(&mp.park) -- 2.50.0