runtime: handle selects with duplicate channels in shrinkstack

The shrinkstack code locks all the channels a goroutine is waiting for,
but didn't handle the case of the same channel appearing in the list
multiple times. This led to a deadlock. The channels are sorted so it's
easy to avoid locking the same channel twice.

Fixes #16286.

Change-Id: Ie514805d0532f61c942e85af5b7b8ac405e2ff65
Reviewed-on: https://go-review.googlesource.com/24815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/chan_test.go b/src/runtime/chan_test.go
index 219f2b449bf03fa7bd69db94f64c63f428c32726..8e8c47b48d441059c5b5a534e9a214a2ec354470 100644 (file)
--- a/src/runtime/chan_test.go
+++ b/src/runtime/chan_test.go
@@ -593,8 +593,10 @@ func TestSelectStackAdjust(t *testing.T) {
        // pointers are adjusted correctly by stack shrinking.
        c := make(chan *int)
        d := make(chan *int)
-       ready := make(chan bool)
-       go func() {
+       ready1 := make(chan bool)
+       ready2 := make(chan bool)
+
+       f := func(ready chan bool, dup bool) {
                // Temporarily grow the stack to 10K.
                stackGrowthRecursive((10 << 10) / (128 * 8))
 
@@ -604,10 +606,20 @@ func TestSelectStackAdjust(t *testing.T) {
                val := 42
                var cx *int
                cx = &val
+
+               var c2 chan *int
+               var d2 chan *int
+               if dup {
+                       c2 = c
+                       d2 = d
+               }
+
                // Receive from d. cx won't be affected.
                select {
                case cx = <-c:
+               case <-c2:
                case <-d:
+               case <-d2:
                }
 
                // Check that pointer in cx was adjusted correctly.
@@ -622,10 +634,14 @@ func TestSelectStackAdjust(t *testing.T) {
                        }
                }
                ready <- true
-       }()
+       }
+
+       go f(ready1, false)
+       go f(ready2, true)
 
-       // Let the goroutine get into the select.
-       <-ready
+       // Let the goroutines get into the select.
+       <-ready1
+       <-ready2
        time.Sleep(10 * time.Millisecond)
 
        // Force concurrent GC a few times.
@@ -642,9 +658,10 @@ func TestSelectStackAdjust(t *testing.T) {
 done:
        selectSink = nil
 
-       // Wake select.
-       d <- nil
-       <-ready
+       // Wake selects.
+       close(d)
+       <-ready1
+       <-ready2
 }
 
 func BenchmarkChanNonblocking(b *testing.B) {

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index b14b4005d80fa1e93a91af65e00aa41992b2a8b0..8398a101fd8f5d8713969654eae6b50b33bea59f 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -784,8 +784,12 @@ func syncadjustsudogs(gp *g, used uintptr, adjinfo *adjustinfo) uintptr {
        // copystack; otherwise, gp may be in the middle of
        // putting itself on wait queues and this would
        // self-deadlock.
+       var lastc *hchan
        for sg := gp.waiting; sg != nil; sg = sg.waitlink {
-               lock(&sg.c.lock)
+               if sg.c != lastc {
+                       lock(&sg.c.lock)
+               }
+               lastc = sg.c
        }
 
        // Adjust sudogs.
@@ -803,8 +807,12 @@ func syncadjustsudogs(gp *g, used uintptr, adjinfo *adjustinfo) uintptr {
        }
 
        // Unlock channels.
+       lastc = nil
        for sg := gp.waiting; sg != nil; sg = sg.waitlink {
-               unlock(&sg.c.lock)
+               if sg.c != lastc {
+                       unlock(&sg.c.lock)
+               }
+               lastc = sg.c
        }
 
        return sgsize