Skip to content

Commit 30a68bf

Browse files
mdempskymdempsky
committed
runtime: add "success" field to sudog
The current wakeup protocol for channel communications is that the second goroutine sets gp.param to the sudog when a value is successfully communicated over the channel, and to nil when the wakeup is due to closing the channel. Setting nil to indicate channel closure works okay for chansend and chanrecv, because they're only communicating with one channel, so they know it must be the channel that was closed. However, it means selectgo has to re-poll all of the channels to figure out which one was closed. This commit adds a "success" field to sudog, and changes the wakeup protocol to always set gp.param to sg, and to use sg.success to indicate successful communication vs channel closure. While here, this also reorganizes the chansend code slightly so that the sudog is still released to the pool if the send blocks and then is awoken because the channel closed. Updates golang#40410. Change-Id: I6cd9a20ebf9febe370a15af1b8afe24c5539efc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/245019 Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cuong Manh Le <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 861a948 commit 30a68bf

3 files changed

Lines changed: 28 additions & 22 deletions

File tree

‎src/runtime/chan.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -263,18 +263,19 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
263263
}
264264
gp.waiting = nil
265265
gp.activeStackChans = false
266-
if gp.param == nil {
267-
if c.closed == 0 {
268-
throw("chansend: spurious wakeup")
269-
}
270-
panic(plainError("send on closed channel"))
271-
}
266+
closed := !mysg.success
272267
gp.param = nil
273268
if mysg.releasetime > 0 {
274269
blockevent(mysg.releasetime-t0, 2)
275270
}
276271
mysg.c = nil
277272
releaseSudog(mysg)
273+
if closed {
274+
if c.closed == 0 {
275+
throw("chansend: spurious wakeup")
276+
}
277+
panic(plainError("send on closed channel"))
278+
}
278279
return true
279280
}
280281

@@ -311,6 +312,7 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
311312
gp := sg.g
312313
unlockf()
313314
gp.param = unsafe.Pointer(sg)
315+
sg.success = true
314316
if sg.releasetime != 0 {
315317
sg.releasetime = cputicks()
316318
}
@@ -384,7 +386,8 @@ func closechan(c *hchan) {
384386
sg.releasetime = cputicks()
385387
}
386388
gp := sg.g
387-
gp.param = nil
389+
gp.param = unsafe.Pointer(sg)
390+
sg.success = false
388391
if raceenabled {
389392
raceacquireg(gp, c.raceaddr())
390393
}
@@ -402,7 +405,8 @@ func closechan(c *hchan) {
402405
sg.releasetime = cputicks()
403406
}
404407
gp := sg.g
405-
gp.param = nil
408+
gp.param = unsafe.Pointer(sg)
409+
sg.success = false
406410
if raceenabled {
407411
raceacquireg(gp, c.raceaddr())
408412
}
@@ -575,11 +579,11 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
575579
if mysg.releasetime > 0 {
576580
blockevent(mysg.releasetime-t0, 2)
577581
}
578-
closed := gp.param == nil
582+
success := mysg.success
579583
gp.param = nil
580584
mysg.c = nil
581585
releaseSudog(mysg)
582-
return true, !closed
586+
return true, success
583587
}
584588

585589
// recv processes a receive operation on a full channel c.
@@ -632,6 +636,7 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
632636
gp := sg.g
633637
unlockf()
634638
gp.param = unsafe.Pointer(sg)
639+
sg.success = true
635640
if sg.releasetime != 0 {
636641
sg.releasetime = cputicks()
637642
}

‎src/runtime/runtime2.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,12 @@ type sudog struct {
366366
// g.selectDone must be CAS'd to win the wake-up race.
367367
isSelect bool
368368

369+
// success indicates whether communication over channel c
370+
// succeeded. It is true if the goroutine was awoken because a
371+
// value was delivered over channel c, and false if awoken
372+
// because c was closed.
373+
success bool
374+
369375
parent *sudog // semaRoot binary tree
370376
waitlink *sudog // g.waiting list or semaRoot
371377
waittail *sudog // semaRoot

‎src/runtime/select.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,12 @@ func selectgo(cas0 *scase, order0 *uint16, ncases int) (int, bool) {
221221
nextp **sudog
222222
)
223223

224-
loop:
225224
// pass 1 - look for something already waiting
226225
var dfli int
227226
var dfl *scase
228227
var casi int
229228
var cas *scase
229+
var caseSuccess bool
230230
var recvOK bool
231231
for i := 0; i < ncases; i++ {
232232
casi = int(pollorder[i])
@@ -331,6 +331,7 @@ loop:
331331
// We singly-linked up the SudoGs in lock order.
332332
casi = -1
333333
cas = nil
334+
caseSuccess = false
334335
sglist = gp.waiting
335336
// Clear all elem before unlinking from gp.waiting.
336337
for sg1 := gp.waiting; sg1 != nil; sg1 = sg1.waitlink {
@@ -352,6 +353,7 @@ loop:
352353
// sg has already been dequeued by the G that woke us up.
353354
casi = int(casei)
354355
cas = k
356+
caseSuccess = sglist.success
355357
} else {
356358
c = k.c
357359
if k.kind == caseSend {
@@ -367,16 +369,7 @@ loop:
367369
}
368370

369371
if cas == nil {
370-
// We can wake up with gp.param == nil (so cas == nil)
371-
// when a channel involved in the select has been closed.
372-
// It is easiest to loop and re-run the operation;
373-
// we'll see that it's now closed.
374-
// Maybe some day we can signal the close explicitly,
375-
// but we'd have to distinguish close-on-reader from close-on-writer.
376-
// It's easiest not to duplicate the code and just recheck above.
377-
// We know that something closed, and things never un-close,
378-
// so we won't block again.
379-
goto loop
372+
throw("selectgo: bad wakeup")
380373
}
381374

382375
c = cas.c
@@ -386,7 +379,9 @@ loop:
386379
}
387380

388381
if cas.kind == caseRecv {
389-
recvOK = true
382+
recvOK = caseSuccess
383+
} else if cas.kind == caseSend && !caseSuccess {
384+
goto sclose
390385
}
391386

392387
if raceenabled {

0 commit comments

Comments
 (0)