mirror of
https://github.com/golang/go
synced 2024-11-11 19:11:35 -07:00
runtime: fix race condition
(Thanks to ken and rsc for pointing this out) rsc: ken pointed out that there's a race in the new one-lock-per-channel code. the issue is that if one goroutine has gone to sleep doing select { case <-c1: case <-c2: } and then two more goroutines try to send on c1 and c2 simultaneously, the way that the code makes sure only one wins is the selgen field manipulation in dequeue: // if sgp is stale, ignore it if(sgp->selgen != sgp->g->selgen) { //prints("INVALID PSEUDOG POINTER\n"); freesg(c, sgp); goto loop; } // invalidate any others sgp->g->selgen++; but because the global lock is gone both goroutines will be fiddling with sgp->g->selgen at the same time. This results in a 7% slowdown in the single threaded case for a ping-pong microbenchmark. Since the cas predominantly succeeds, adding a simple check first didn't make any difference. R=rsc CC=golang-dev https://golang.org/cl/180068
This commit is contained in:
parent
057e7d9fae
commit
50d6c81d4a
@ -24,7 +24,7 @@ typedef struct Scase Scase;
|
||||
struct SudoG
|
||||
{
|
||||
G* g; // g and selgen constitute
|
||||
int32 selgen; // a weak pointer to g
|
||||
uint32 selgen; // a weak pointer to g
|
||||
int16 offset; // offset of case number
|
||||
int8 isfree; // offset of case number
|
||||
SudoG* link;
|
||||
@ -982,14 +982,12 @@ loop:
|
||||
q->first = sgp->link;
|
||||
|
||||
// if sgp is stale, ignore it
|
||||
if(sgp->selgen != sgp->g->selgen) {
|
||||
if(!cas(&sgp->g->selgen, sgp->selgen, sgp->selgen + 1)) {
|
||||
//prints("INVALID PSEUDOG POINTER\n");
|
||||
freesg(c, sgp);
|
||||
goto loop;
|
||||
}
|
||||
|
||||
// invalidate any others
|
||||
sgp->g->selgen++;
|
||||
return sgp;
|
||||
}
|
||||
|
||||
|
@ -167,7 +167,7 @@ struct G
|
||||
void* param; // passed parameter on wakeup
|
||||
int16 status;
|
||||
int32 goid;
|
||||
int32 selgen; // valid sudog pointer
|
||||
uint32 selgen; // valid sudog pointer
|
||||
G* schedlink;
|
||||
bool readyonstop;
|
||||
M* m; // for debuggers, but offset not hard-coded
|
||||
|
83
test/chan/doubleselect.go
Normal file
83
test/chan/doubleselect.go
Normal file
@ -0,0 +1,83 @@
|
||||
// $G $D/$F.go && $L $F.$A && ./$A.out
|
||||
|
||||
// Copyright 2009 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
// This test is designed to flush out the case where two cases of a select can
|
||||
// both end up running. See http://codereview.appspot.com/180068.
|
||||
package main
|
||||
|
||||
import (
|
||||
"flag"
|
||||
"runtime"
|
||||
)
|
||||
|
||||
var iterations *int = flag.Int("n", 100000, "number of iterations")
|
||||
|
||||
// sender sends a counter to one of four different channels. If two
|
||||
// cases both end up running in the same iteration, the same value will be sent
|
||||
// to two different channels.
|
||||
func sender(n int, c1, c2, c3, c4 chan<- int) {
|
||||
defer close(c1)
|
||||
defer close(c2)
|
||||
|
||||
for i := 0; i < n; i++ {
|
||||
select {
|
||||
case c1 <- i:
|
||||
case c2 <- i:
|
||||
case c3 <- i:
|
||||
case c4 <- i:
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// mux receives the values from sender and forwards them onto another channel.
|
||||
// It would be simplier to just have sender's four cases all be the same
|
||||
// channel, but this doesn't actually trigger the bug.
|
||||
func mux(out chan<- int, in <-chan int) {
|
||||
for {
|
||||
v := <-in
|
||||
if closed(in) {
|
||||
close(out)
|
||||
break
|
||||
}
|
||||
out <- v
|
||||
}
|
||||
}
|
||||
|
||||
// recver gets a steam of values from the four mux's and checks for duplicates.
|
||||
func recver(in <-chan int) {
|
||||
seen := make(map[int]bool)
|
||||
|
||||
for {
|
||||
v := <-in
|
||||
if closed(in) {
|
||||
break
|
||||
}
|
||||
if _, ok := seen[v]; ok {
|
||||
panic("got duplicate value: ", v)
|
||||
}
|
||||
seen[v] = true
|
||||
}
|
||||
}
|
||||
|
||||
func main() {
|
||||
runtime.GOMAXPROCS(2)
|
||||
|
||||
c1 := make(chan int)
|
||||
c2 := make(chan int)
|
||||
c3 := make(chan int)
|
||||
c4 := make(chan int)
|
||||
cmux := make(chan int)
|
||||
go sender(*iterations, c1, c2, c3, c4)
|
||||
go mux(cmux, c1)
|
||||
go mux(cmux, c2)
|
||||
go mux(cmux, c3)
|
||||
go mux(cmux, c4)
|
||||
// We keep the recver because it might catch more bugs in the future.
|
||||
// However, the result of the bug linked to at the top is that we'll
|
||||
// end up panicing with: "throw: bad g->status in ready".
|
||||
recver(cmux)
|
||||
print("PASS\n")
|
||||
}
|
@ -75,6 +75,9 @@ abcxyz-abcxyz-abcxyz-abcxyz-abcxyz-abcxyz-abcxyz
|
||||
|
||||
== chan/
|
||||
|
||||
=========== chan/doubleselect.go
|
||||
PASS
|
||||
|
||||
=========== chan/nonblock.go
|
||||
PASS
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user