Skip to content

Commit 83dfc3b

Browse files
committed
runtime: ignore races between close and len/cap
They aren't really races, or at least they don't have any observable effect. The spec is silent on whether these are actually races or not. Fix this problem by not using the address of len (or of cap) as the location where channel operations are recorded to occur. Use a random other field of hchan for that. I'm not 100% sure we should in fact fix this. Opinions welcome. Fixes #27070 Change-Id: Ib4efd4b62e0d1ef32fa51e373035ef207a655084 Reviewed-on: https://go-review.googlesource.com/135698 Reviewed-by: Dmitry Vyukov <[email protected]>
1 parent 77f9b27 commit 83dfc3b

File tree

3 files changed

+40
-17
lines changed

3 files changed

+40
-17
lines changed

src/runtime/chan.go

+16-7
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func makechan(t *chantype, size int) *hchan {
9292
// Queue or element size is zero.
9393
c = (*hchan)(mallocgc(hchanSize, nil, true))
9494
// Race detector uses this location for synchronization.
95-
c.buf = unsafe.Pointer(c)
95+
c.buf = c.raceaddr()
9696
case elem.kind&kindNoPointers != 0:
9797
// Elements do not contain pointers.
9898
// Allocate hchan and buf in one call.
@@ -151,7 +151,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
151151
}
152152

153153
if raceenabled {
154-
racereadpc(unsafe.Pointer(c), callerpc, funcPC(chansend))
154+
racereadpc(c.raceaddr(), callerpc, funcPC(chansend))
155155
}
156156

157157
// Fast path: check for failed non-blocking operation without acquiring the lock.
@@ -337,8 +337,8 @@ func closechan(c *hchan) {
337337

338338
if raceenabled {
339339
callerpc := getcallerpc()
340-
racewritepc(unsafe.Pointer(c), callerpc, funcPC(closechan))
341-
racerelease(unsafe.Pointer(c))
340+
racewritepc(c.raceaddr(), callerpc, funcPC(closechan))
341+
racerelease(c.raceaddr())
342342
}
343343

344344
c.closed = 1
@@ -361,7 +361,7 @@ func closechan(c *hchan) {
361361
gp := sg.g
362362
gp.param = nil
363363
if raceenabled {
364-
raceacquireg(gp, unsafe.Pointer(c))
364+
raceacquireg(gp, c.raceaddr())
365365
}
366366
glist.push(gp)
367367
}
@@ -379,7 +379,7 @@ func closechan(c *hchan) {
379379
gp := sg.g
380380
gp.param = nil
381381
if raceenabled {
382-
raceacquireg(gp, unsafe.Pointer(c))
382+
raceacquireg(gp, c.raceaddr())
383383
}
384384
glist.push(gp)
385385
}
@@ -454,7 +454,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
454454

455455
if c.closed != 0 && c.qcount == 0 {
456456
if raceenabled {
457-
raceacquire(unsafe.Pointer(c))
457+
raceacquire(c.raceaddr())
458458
}
459459
unlock(&c.lock)
460460
if ep != nil {
@@ -732,6 +732,15 @@ func (q *waitq) dequeue() *sudog {
732732
}
733733
}
734734

735+
func (c *hchan) raceaddr() unsafe.Pointer {
736+
// Treat read-like and write-like operations on the channel to
737+
// happen at this address. Avoid using the address of qcount
738+
// or dataqsiz, because the len() and cap() builtins read
739+
// those addresses, and we don't want them racing with
740+
// operations like close().
741+
return unsafe.Pointer(&c.buf)
742+
}
743+
735744
func racesync(c *hchan, sg *sudog) {
736745
racerelease(chanbuf(c, 0))
737746
raceacquireg(sg.g, chanbuf(c, 0))

src/runtime/race/testdata/chan_test.go

+22-8
Original file line numberDiff line numberDiff line change
@@ -577,18 +577,32 @@ func TestRaceChanItselfCap(t *testing.T) {
577577
<-compl
578578
}
579579

580-
func TestRaceChanCloseLen(t *testing.T) {
581-
v := 0
582-
_ = v
580+
func TestNoRaceChanCloseLen(t *testing.T) {
583581
c := make(chan int, 10)
584-
c <- 0
582+
r := make(chan int, 10)
583+
go func() {
584+
r <- len(c)
585+
}()
585586
go func() {
586-
v = 1
587587
close(c)
588+
r <- 0
588589
}()
589-
time.Sleep(1e7)
590-
_ = len(c)
591-
v = 2
590+
<-r
591+
<-r
592+
}
593+
594+
func TestNoRaceChanCloseCap(t *testing.T) {
595+
c := make(chan int, 10)
596+
r := make(chan int, 10)
597+
go func() {
598+
r <- cap(c)
599+
}()
600+
go func() {
601+
close(c)
602+
r <- 0
603+
}()
604+
<-r
605+
<-r
592606
}
593607

594608
func TestRaceChanCloseSend(t *testing.T) {

src/runtime/select.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ loop:
245245

246246
case caseSend:
247247
if raceenabled {
248-
racereadpc(unsafe.Pointer(c), cas.pc, chansendpc)
248+
racereadpc(c.raceaddr(), cas.pc, chansendpc)
249249
}
250250
if c.closed != 0 {
251251
goto sclose
@@ -462,7 +462,7 @@ rclose:
462462
typedmemclr(c.elemtype, cas.elem)
463463
}
464464
if raceenabled {
465-
raceacquire(unsafe.Pointer(c))
465+
raceacquire(c.raceaddr())
466466
}
467467
goto retc
468468

0 commit comments

Comments
 (0)