Skip to content

Commit 84bb9e6

Browse files
ianlancetayloradg
authored andcommitted
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>
1 parent e5ff529 commit 84bb9e6

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

src/runtime/chan_test.go

+25-8
Original file line numberDiff line numberDiff line change
@@ -593,8 +593,10 @@ func TestSelectStackAdjust(t *testing.T) {
593593
// pointers are adjusted correctly by stack shrinking.
594594
c := make(chan *int)
595595
d := make(chan *int)
596-
ready := make(chan bool)
597-
go func() {
596+
ready1 := make(chan bool)
597+
ready2 := make(chan bool)
598+
599+
f := func(ready chan bool, dup bool) {
598600
// Temporarily grow the stack to 10K.
599601
stackGrowthRecursive((10 << 10) / (128 * 8))
600602

@@ -604,10 +606,20 @@ func TestSelectStackAdjust(t *testing.T) {
604606
val := 42
605607
var cx *int
606608
cx = &val
609+
610+
var c2 chan *int
611+
var d2 chan *int
612+
if dup {
613+
c2 = c
614+
d2 = d
615+
}
616+
607617
// Receive from d. cx won't be affected.
608618
select {
609619
case cx = <-c:
620+
case <-c2:
610621
case <-d:
622+
case <-d2:
611623
}
612624

613625
// Check that pointer in cx was adjusted correctly.
@@ -622,10 +634,14 @@ func TestSelectStackAdjust(t *testing.T) {
622634
}
623635
}
624636
ready <- true
625-
}()
637+
}
638+
639+
go f(ready1, false)
640+
go f(ready2, true)
626641

627-
// Let the goroutine get into the select.
628-
<-ready
642+
// Let the goroutines get into the select.
643+
<-ready1
644+
<-ready2
629645
time.Sleep(10 * time.Millisecond)
630646

631647
// Force concurrent GC a few times.
@@ -642,9 +658,10 @@ func TestSelectStackAdjust(t *testing.T) {
642658
done:
643659
selectSink = nil
644660

645-
// Wake select.
646-
d <- nil
647-
<-ready
661+
// Wake selects.
662+
close(d)
663+
<-ready1
664+
<-ready2
648665
}
649666

650667
func BenchmarkChanNonblocking(b *testing.B) {

src/runtime/stack.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -784,8 +784,12 @@ func syncadjustsudogs(gp *g, used uintptr, adjinfo *adjustinfo) uintptr {
784784
// copystack; otherwise, gp may be in the middle of
785785
// putting itself on wait queues and this would
786786
// self-deadlock.
787+
var lastc *hchan
787788
for sg := gp.waiting; sg != nil; sg = sg.waitlink {
788-
lock(&sg.c.lock)
789+
if sg.c != lastc {
790+
lock(&sg.c.lock)
791+
}
792+
lastc = sg.c
789793
}
790794

791795
// Adjust sudogs.
@@ -803,8 +807,12 @@ func syncadjustsudogs(gp *g, used uintptr, adjinfo *adjustinfo) uintptr {
803807
}
804808

805809
// Unlock channels.
810+
lastc = nil
806811
for sg := gp.waiting; sg != nil; sg = sg.waitlink {
807-
unlock(&sg.c.lock)
812+
if sg.c != lastc {
813+
unlock(&sg.c.lock)
814+
}
815+
lastc = sg.c
808816
}
809817

810818
return sgsize

0 commit comments

Comments
 (0)