Commit 56959158 authored by Dmitriy Vyukov's avatar Dmitriy Vyukov

runtime: fix spurious deadlock reporting

Fixes #2337.
Unfortunate sequence of events is:
1. maxcpu=2, mcpu=1, grunning=1
2. starttheworld creates an extra M:
   maxcpu=2, mcpu=2, grunning=1
4. the goroutine calls runtime.GOMAXPROCS(1)
   maxcpu=1, mcpu=2, grunning=1
5. since it sees mcpu>maxcpu, it calls gosched()
6. schedule() deschedules the goroutine:
   maxcpu=1, mcpu=1, grunning=0
7. schedule() call getnextandunlock() which
   fails to pick up the goroutine again,
   because canaddcpu() fails, because mcpu==maxcpu
8. then it sees that grunning==0,
   reports deadlock and terminates

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/5191044
parent 504963e6
...@@ -407,7 +407,7 @@ enum ...@@ -407,7 +407,7 @@ enum
void runtime·MProf_Malloc(void*, uintptr); void runtime·MProf_Malloc(void*, uintptr);
void runtime·MProf_Free(void*, uintptr); void runtime·MProf_Free(void*, uintptr);
int32 runtime·helpgc(void); int32 runtime·helpgc(bool*);
void runtime·gchelper(void); void runtime·gchelper(void);
// Malloc profiling settings. // Malloc profiling settings.
......
...@@ -916,10 +916,11 @@ runtime·gc(int32 force) ...@@ -916,10 +916,11 @@ runtime·gc(int32 force)
runtime·lock(&work.markgate); runtime·lock(&work.markgate);
runtime·lock(&work.sweepgate); runtime·lock(&work.sweepgate);
extra = false;
work.nproc = 1; work.nproc = 1;
if(runtime·gomaxprocs > 1 && runtime·ncpu > 1) { if(runtime·gomaxprocs > 1 && runtime·ncpu > 1) {
runtime·noteclear(&work.alldone); runtime·noteclear(&work.alldone);
work.nproc += runtime·helpgc(); work.nproc += runtime·helpgc(&extra);
} }
work.nwait = 0; work.nwait = 0;
work.ndone = 0; work.ndone = 0;
...@@ -984,7 +985,6 @@ runtime·gc(int32 force) ...@@ -984,7 +985,6 @@ runtime·gc(int32 force)
// coordinate. This lazy approach works out in practice: // coordinate. This lazy approach works out in practice:
// we don't mind if the first couple gc rounds don't have quite // we don't mind if the first couple gc rounds don't have quite
// the maximum number of procs. // the maximum number of procs.
extra = work.nproc < runtime·gomaxprocs && work.nproc < runtime·ncpu && work.nproc < MaxGcproc;
runtime·starttheworld(extra); runtime·starttheworld(extra);
// give the queued finalizers, if any, a chance to run // give the queued finalizers, if any, a chance to run
...@@ -1008,7 +1008,7 @@ runtime·UpdateMemStats(void) ...@@ -1008,7 +1008,7 @@ runtime·UpdateMemStats(void)
cachestats(); cachestats();
m->gcing = 0; m->gcing = 0;
runtime·semrelease(&gcsema); runtime·semrelease(&gcsema);
runtime·starttheworld(0); runtime·starttheworld(false);
} }
static void static void
......
...@@ -602,9 +602,9 @@ top: ...@@ -602,9 +602,9 @@ top:
} }
int32 int32
runtime·helpgc(void) runtime·helpgc(bool *extra)
{ {
M *m; M *mp;
int32 n, max; int32 n, max;
// Figure out how many CPUs to use. // Figure out how many CPUs to use.
...@@ -621,13 +621,15 @@ runtime·helpgc(void) ...@@ -621,13 +621,15 @@ runtime·helpgc(void)
runtime·lock(&runtime·sched); runtime·lock(&runtime·sched);
n = 0; n = 0;
while(n < max && (m = mget(nil)) != nil) { while(n < max && (mp = mget(nil)) != nil) {
n++; n++;
m->helpgc = 1; mp->helpgc = 1;
m->waitnextg = 0; mp->waitnextg = 0;
runtime·notewakeup(&m->havenextg); runtime·notewakeup(&mp->havenextg);
} }
runtime·unlock(&runtime·sched); runtime·unlock(&runtime·sched);
if(extra)
*extra = n != max;
return n; return n;
} }
...@@ -685,9 +687,10 @@ runtime·starttheworld(bool extra) ...@@ -685,9 +687,10 @@ runtime·starttheworld(bool extra)
// initialization work so is definitely running), // initialization work so is definitely running),
// but m is not running a specific goroutine, // but m is not running a specific goroutine,
// so set the helpgc flag as a signal to m's // so set the helpgc flag as a signal to m's
// first schedule(nil) to mcpu--. // first schedule(nil) to mcpu-- and grunning--.
m = startm(); m = startm();
m->helpgc = 1; m->helpgc = 1;
runtime·sched.grunning++;
} }
schedunlock(); schedunlock();
} }
...@@ -833,6 +836,8 @@ schedule(G *gp) ...@@ -833,6 +836,8 @@ schedule(G *gp)
v = runtime·xadd(&runtime·sched.atomic, -1<<mcpuShift); v = runtime·xadd(&runtime·sched.atomic, -1<<mcpuShift);
if(atomic_mcpu(v) > maxgomaxprocs) if(atomic_mcpu(v) > maxgomaxprocs)
runtime·throw("negative mcpu in scheduler"); runtime·throw("negative mcpu in scheduler");
// Compensate for increment in starttheworld().
runtime·sched.grunning--;
m->helpgc = 0; m->helpgc = 0;
} }
......
// $G $D/$F.go && $L $F.$A && ./$A.out
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package main
// issue 2337
// The program deadlocked.
import "runtime"
func main() {
runtime.GOMAXPROCS(2)
runtime.GC()
runtime.GOMAXPROCS(1)
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment