runtime: dequeue the correct SudoG

select {
       case <- c:
       case <- c:
}

In this case, c.recvq lists two SudoGs which have the same G.
So we can't use the G as the key to dequeue the correct SudoG,
as that key is ambiguous.  Dequeueing the wrong SudoG ends up
freeing a SudoG that is still in c.recvq.

The fix is to use the actual SudoG pointer as the key.

LGTM=dvyukov
R=rsc, bradfitz, dvyukov, khr
CC=austin, golang-codereviews
https://golang.org/cl/159040043

diff --git a/src/runtime/chan_test.go b/src/runtime/chan_test.go
index 01632892ed2de8a1b69a31322bfc5720f74506d4..e689ceaed1e1b2eecebfad8bb37c4ed15e41d39c 100644 (file)
--- a/src/runtime/chan_test.go
+++ b/src/runtime/chan_test.go
@@ -482,6 +482,35 @@ func TestShrinkStackDuringBlockedSend(t *testing.T) {
        <-done
 }
 
+func TestSelectDuplicateChannel(t *testing.T) {
+       // This test makes sure we can queue a G on
+       // the same channel multiple times.
+       c := make(chan int)
+       d := make(chan int)
+       e := make(chan int)
+
+       // goroutine A
+       go func() {
+               select {
+               case <-c:
+               case <-c:
+               case <-d:
+               }
+               e <- 9
+       }()
+       time.Sleep(time.Millisecond) // make sure goroutine A gets qeueued first on c
+
+       // goroutine B
+       go func() {
+               <-c
+       }()
+       time.Sleep(time.Millisecond) // make sure goroutine B gets queued on c before continuing
+
+       d <- 7 // wake up A, it dequeues itself from c.  This operation used to corrupt c.recvq.
+       <-e    // A tells us it's done
+       c <- 8 // wake up B.  This operation used to fail because c.recvq was corrupted (it tries to wake up an already running G instead of B)
+}
+
 func BenchmarkChanNonblocking(b *testing.B) {
        myc := make(chan int)
        b.RunParallel(func(pb *testing.PB) {

diff --git a/src/runtime/select.go b/src/runtime/select.go
index 9de057b8711e78df10e90afa9f90bcb4abccebf1..efe68c1f5cbe5dc28763cbd47670ce6743e0b252 100644 (file)
--- a/src/runtime/select.go
+++ b/src/runtime/select.go
@@ -398,9 +398,9 @@ loop:
                } else {
                        c = k._chan
                        if k.kind == _CaseSend {
-                               c.sendq.dequeueg(gp)
+                               c.sendq.dequeueSudoG(sglist)
                        } else {
-                               c.recvq.dequeueg(gp)
+                               c.recvq.dequeueSudoG(sglist)
                        }
                }
                sgnext = sglist.waitlink
@@ -628,7 +628,7 @@ func reflect_rselect(cases []runtimeSelect) (chosen int, recvOK bool) {
        return
 }
 
-func (q *waitq) dequeueg(gp *g) {
+func (q *waitq) dequeueSudoG(s *sudog) {
        var prevsgp *sudog
        l := &q.first
        for {
@@ -636,7 +636,7 @@ func (q *waitq) dequeueg(gp *g) {
                if sgp == nil {
                        return
                }
-               if sgp.g == gp {
+               if sgp == s {
                        *l = sgp.next
                        if q.last == sgp {
                                q.last = prevsgp