Skip to content

Commit 8c3533c

Browse files
committed
runtime: add memory barrier for sync send in select
Missed select case when adding the barrier last time. All the more reason to refactor this code in Go 1.6. Fixes #11643. Change-Id: Ib0d19d6e0939296c0a3e06dda5e9b76f813bbc7e Reviewed-on: https://go-review.googlesource.com/12086 Reviewed-by: Austin Clements <[email protected]>
1 parent 242ec16 commit 8c3533c

File tree

3 files changed

+97
-12
lines changed

3 files changed

+97
-12
lines changed

src/runtime/chan.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,7 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin
165165

166166
recvg := sg.g
167167
if sg.elem != nil {
168-
// This is the only place in the entire runtime where one goroutine
169-
// writes to the stack of another goroutine. The GC assumes that
170-
// stack writes only happen when the goroutine is running and are
171-
// only done by that goroutine. Using a write barrier is sufficient to
172-
// make up for violating that assumption, but the write barrier has to work.
173-
// typedmemmove will call heapBitsBulkBarrier, but the target bytes
174-
// are not in the heap, so that will not help. We arrange to call
175-
// memmove and typeBitsBulkBarrier instead.
176-
memmove(sg.elem, ep, c.elemtype.size)
177-
typeBitsBulkBarrier(c.elemtype, uintptr(sg.elem), c.elemtype.size)
178-
sg.elem = nil
168+
syncsend(c, sg, ep)
179169
}
180170
recvg.param = unsafe.Pointer(sg)
181171
if sg.releasetime != 0 {
@@ -287,6 +277,21 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin
287277
return true
288278
}
289279

280+
func syncsend(c *hchan, sg *sudog, elem unsafe.Pointer) {
281+
// Send on unbuffered channel is the only operation
282+
// in the entire runtime where one goroutine
283+
// writes to the stack of another goroutine. The GC assumes that
284+
// stack writes only happen when the goroutine is running and are
285+
// only done by that goroutine. Using a write barrier is sufficient to
286+
// make up for violating that assumption, but the write barrier has to work.
287+
// typedmemmove will call heapBitsBulkBarrier, but the target bytes
288+
// are not in the heap, so that will not help. We arrange to call
289+
// memmove and typeBitsBulkBarrier instead.
290+
memmove(sg.elem, elem, c.elemtype.size)
291+
typeBitsBulkBarrier(c.elemtype, uintptr(sg.elem), c.elemtype.size)
292+
sg.elem = nil
293+
}
294+
290295
func closechan(c *hchan) {
291296
if c == nil {
292297
panic("close of nil channel")

src/runtime/chanbarrier_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright 2015 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package runtime_test
6+
7+
import (
8+
"runtime"
9+
"sync"
10+
"testing"
11+
)
12+
13+
type response struct {
14+
}
15+
16+
type myError struct {
17+
}
18+
19+
func (myError) Error() string { return "" }
20+
21+
func doRequest(useSelect bool) (*response, error) {
22+
type async struct {
23+
resp *response
24+
err error
25+
}
26+
ch := make(chan *async, 0)
27+
done := make(chan struct{}, 0)
28+
29+
if useSelect {
30+
go func() {
31+
select {
32+
case ch <- &async{resp: nil, err: myError{}}:
33+
case <-done:
34+
}
35+
}()
36+
} else {
37+
go func() {
38+
ch <- &async{resp: nil, err: myError{}}
39+
}()
40+
}
41+
42+
r := <-ch
43+
runtime.Gosched()
44+
return r.resp, r.err
45+
}
46+
47+
func TestChanSendSelectBarrier(t *testing.T) {
48+
testChanSendBarrier(true)
49+
}
50+
51+
func TestChanSendBarrier(t *testing.T) {
52+
testChanSendBarrier(false)
53+
}
54+
55+
func testChanSendBarrier(useSelect bool) {
56+
var wg sync.WaitGroup
57+
outer := 100
58+
inner := 100000
59+
if testing.Short() {
60+
outer = 10
61+
inner = 1000
62+
}
63+
for i := 0; i < outer; i++ {
64+
wg.Add(1)
65+
go func() {
66+
defer wg.Done()
67+
var garbage []byte
68+
for j := 0; j < inner; j++ {
69+
_, err := doRequest(useSelect)
70+
_, ok := err.(myError)
71+
if !ok {
72+
panic(1)
73+
}
74+
garbage = make([]byte, 1<<10)
75+
}
76+
global = garbage
77+
}()
78+
}
79+
wg.Wait()
80+
}

src/runtime/select.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ syncsend:
575575
print("syncsend: sel=", sel, " c=", c, "\n")
576576
}
577577
if sg.elem != nil {
578-
typedmemmove(c.elemtype, sg.elem, cas.elem)
578+
syncsend(c, sg, cas.elem)
579579
}
580580
sg.elem = nil
581581
gp = sg.g

0 commit comments

Comments
 (0)