runtime: protect sudog.elem with hchan.lock

Currently sudog.elem is never accessed concurrently, so in several
cases we drop the channel lock just before reading/writing the
sent/received value from/to sudog.elem. However, concurrent stack
shrinking is going to have to adjust sudog.elem to point to the new
stack, which means it needs a way to synchronize with accesses to
sudog.elem. Hence, add sudog.elem to the fields protected by
hchan.lock and scoot the unlocks down past the uses of sudog.elem.

While we're here, better document the channel synchronization rules.

For #12967.

Change-Id: I3ad0ca71f0a74b0716c261aef21b2f7f13f74917
Reviewed-on: https://go-review.googlesource.com/20040Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/runtime/chan.go

@@ -33,7 +33,10 @@ type hchan struct {
 	recvx    uint   // receive index
 	recvq    waitq  // list of recv waiters
 	sendq    waitq  // list of send waiters
-	lock     mutex
+
+	// lock protects all fields in hchan, as well as several
+	// fields in sudogs blocked on this channel.
+	lock mutex
 }
 
 type waitq struct {
@@ -261,12 +264,12 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func()) {
 			c.sendx = c.recvx // c.sendx = (c.sendx+1) % c.dataqsiz
 		}
 	}
-	unlockf()
 	if sg.elem != nil {
 		sendDirect(c.elemtype, sg, ep)
 		sg.elem = nil
 	}
 	gp := sg.g
+	unlockf()
 	gp.param = unsafe.Pointer(sg)
 	if sg.releasetime != 0 {
 		sg.releasetime = cputicks()
@@ -509,7 +512,6 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func()) {
 		if raceenabled {
 			racesync(c, sg)
 		}
-		unlockf()
 		if ep != nil {
 			// copy data from sender
 			// ep points to our own stack or heap, so nothing
@@ -539,10 +541,10 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func()) {
 			c.recvx = 0
 		}
 		c.sendx = c.recvx // c.sendx = (c.sendx+1) % c.dataqsiz
-		unlockf()
 	}
 	sg.elem = nil
 	gp := sg.g
+	unlockf()
 	gp.param = unsafe.Pointer(sg)
 	if sg.releasetime != 0 {
 		sg.releasetime = cputicks()

src/runtime/runtime2.go

@@ -214,11 +214,18 @@ type gobuf struct {
 // Changes here must also be made in src/cmd/compile/internal/gc/select.go's
 // selecttype.
 type sudog struct {
-	g           *g
-	selectdone  *uint32 // CAS to 1 to win select race (may point to stack)
-	next        *sudog
-	prev        *sudog
-	elem        unsafe.Pointer // data element (may point to stack)
+	// The following fields are protected by the hchan.lock of the
+	// channel this sudog is blocking on.
+
+	g          *g
+	selectdone *uint32 // CAS to 1 to win select race (may point to stack)
+	next       *sudog
+	prev       *sudog
+	elem       unsafe.Pointer // data element (may point to stack)
+
+	// The following fields are never accessed concurrently.
+	// waitlink is only accessed by g.
+
 	releasetime int64
 	ticket      uint32
 	waitlink    *sudog // g.waiting list