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

diff --git a/src/pkg/runtime/malloc.h b/src/pkg/runtime/malloc.h
index f22cae4b0512a746259c04970991916bba37fa86..eb3bba3431c39620e96d4793a695176a9afbe264 100644 (file)
--- a/src/pkg/runtime/malloc.h
+++ b/src/pkg/runtime/malloc.h
@@ -407,7 +407,7 @@ enum
 
 void   runtime·MProf_Malloc(void*, uintptr);
 void   runtime·MProf_Free(void*, uintptr);
-int32  runtime·helpgc(void);
+int32  runtime·helpgc(bool*);
 void   runtime·gchelper(void);
 
 // Malloc profiling settings.

diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c
index a2ae8a4109a9b0c4ce952a41e8092323e0a22074..37a495dd2ca792afbd8da49bdff8f540659cb672 100644 (file)
--- a/src/pkg/runtime/mgc0.c
+++ b/src/pkg/runtime/mgc0.c
@@ -916,10 +916,11 @@ runtime·gc(int32 force)
        runtime·lock(&work.markgate);
        runtime·lock(&work.sweepgate);
 
+       extra = false;
        work.nproc = 1;
        if(runtime·gomaxprocs > 1 && runtime·ncpu > 1) {
                runtime·noteclear(&work.alldone);
-               work.nproc += runtime·helpgc();
+               work.nproc += runtime·helpgc(&extra);
        }
        work.nwait = 0;
        work.ndone = 0;
@@ -984,7 +985,6 @@ runtime·gc(int32 force)
        // coordinate.  This lazy approach works out in practice:
        // we don't mind if the first couple gc rounds don't have quite
        // the maximum number of procs.
-       extra = work.nproc < runtime·gomaxprocs && work.nproc < runtime·ncpu && work.nproc < MaxGcproc;
        runtime·starttheworld(extra);
 
        // give the queued finalizers, if any, a chance to run
@@ -1008,7 +1008,7 @@ runtime·UpdateMemStats(void)
        cachestats();
        m->gcing = 0;
        runtime·semrelease(&gcsema);
-       runtime·starttheworld(0);
+       runtime·starttheworld(false);
 }
 
 static void

diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c
index 3655412005035619b253a97525d1f3134cf4f466..5a9d477bc7711d6f8e1a65d2885126a8c7d5eb03 100644 (file)
--- a/src/pkg/runtime/proc.c
+++ b/src/pkg/runtime/proc.c
@@ -602,9 +602,9 @@ top:
 }
 
 int32
-runtime·helpgc(void)
+runtime·helpgc(bool *extra)
 {
-       M *m;
+       M *mp;
        int32 n, max;
 
        // Figure out how many CPUs to use.
@@ -621,13 +621,15 @@ runtime·helpgc(void)
 
        runtime·lock(&runtime·sched);
        n = 0;
-       while(n < max && (m = mget(nil)) != nil) {
+       while(n < max && (mp = mget(nil)) != nil) {
                n++;
-               m->helpgc = 1;
-               m->waitnextg = 0;
-               runtime·notewakeup(&m->havenextg);
+               mp->helpgc = 1;
+               mp->waitnextg = 0;
+               runtime·notewakeup(&mp->havenextg);
        }
        runtime·unlock(&runtime·sched);
+       if(extra)
+               *extra = n != max;
        return n;
 }
 
@@ -685,9 +687,10 @@ runtime·starttheworld(bool extra)
                // initialization work so is definitely running),
                // but m is not running a specific goroutine,
                // 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->helpgc = 1;
+               runtime·sched.grunning++;
        }
        schedunlock();
 }
@@ -833,6 +836,8 @@ schedule(G *gp)
                v = runtime·xadd(&runtime·sched.atomic, -1<<mcpuShift);
                if(atomic_mcpu(v) > maxgomaxprocs)
                        runtime·throw("negative mcpu in scheduler");
+               // Compensate for increment in starttheworld().
+               runtime·sched.grunning--;
                m->helpgc = 0;
        }
 

diff --git a/test/fixedbugs/bug370.go b/test/fixedbugs/bug370.go
new file mode 100644 (file)
index 0000000..9cb45f6
--- /dev/null
+++ b/test/fixedbugs/bug370.go
@@ -0,0 +1,18 @@
+// $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)
+}