Skip to content

Commit

Permalink
runtime: add "success" field to sudog
Browse files Browse the repository at this point in the history
This is the gofrontend version of https://golang.org/cl/245019.
Original CL description:

    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.

    For golang/go#40410

This is being brought over to gofrontend as a step toward upgrading to
Go1.16beta1, setting up for more compiler changes related to select handling.

Change-Id: I8d36d7be40cae2e0fdcbdf73959a95dc815f3f1c
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/279734
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
  • Loading branch information
ianlancetaylor committed Dec 22, 2020
1 parent d0e56e8 commit eca96e3
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 22 deletions.
25 changes: 15 additions & 10 deletions libgo/go/runtime/chan.go
Expand Up @@ -285,18 +285,19 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
}
gp.waiting = nil
gp.activeStackChans = false
if gp.param == nil {
if c.closed == 0 {
throw("chansend: spurious wakeup")
}
panic(plainError("send on closed channel"))
}
closed := !mysg.success
gp.param = nil
if mysg.releasetime > 0 {
blockevent(mysg.releasetime-t0, 2)
}
mysg.c = nil
releaseSudog(mysg)
if closed {
if c.closed == 0 {
throw("chansend: spurious wakeup")
}
panic(plainError("send on closed channel"))
}
return true
}

Expand Down Expand Up @@ -333,6 +334,7 @@ func send(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
gp := sg.g
unlockf()
gp.param = unsafe.Pointer(sg)
sg.success = true
if sg.releasetime != 0 {
sg.releasetime = cputicks()
}
Expand Down Expand Up @@ -406,7 +408,8 @@ func closechan(c *hchan) {
sg.releasetime = cputicks()
}
gp := sg.g
gp.param = nil
gp.param = unsafe.Pointer(sg)
sg.success = false
if raceenabled {
raceacquireg(gp, c.raceaddr())
}
Expand All @@ -424,7 +427,8 @@ func closechan(c *hchan) {
sg.releasetime = cputicks()
}
gp := sg.g
gp.param = nil
gp.param = unsafe.Pointer(sg)
sg.success = false
if raceenabled {
raceacquireg(gp, c.raceaddr())
}
Expand Down Expand Up @@ -607,11 +611,11 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
if mysg.releasetime > 0 {
blockevent(mysg.releasetime-t0, 2)
}
closed := gp.param == nil
success := mysg.success
gp.param = nil
mysg.c = nil
releaseSudog(mysg)
return true, !closed
return true, success
}

// recv processes a receive operation on a full channel c.
Expand Down Expand Up @@ -664,6 +668,7 @@ func recv(c *hchan, sg *sudog, ep unsafe.Pointer, unlockf func(), skip int) {
gp := sg.g
unlockf()
gp.param = unsafe.Pointer(sg)
sg.success = true
if sg.releasetime != 0 {
sg.releasetime = cputicks()
}
Expand Down
6 changes: 6 additions & 0 deletions libgo/go/runtime/runtime2.go
Expand Up @@ -354,6 +354,12 @@ type sudog struct {
// g.selectDone must be CAS'd to win the wake-up race.
isSelect bool

// success indicates whether communication over channel c
// succeeded. It is true if the goroutine was awoken because a
// value was delivered over channel c, and false if awoken
// because c was closed.
success bool

parent *sudog // semaRoot binary tree
waitlink *sudog // g.waiting list or semaRoot
waittail *sudog // semaRoot
Expand Down
19 changes: 7 additions & 12 deletions libgo/go/runtime/select.go
Expand Up @@ -235,10 +235,10 @@ func selectgo(cas0 *scase, order0 *uint16, ncases int) (int, bool) {
nextp **sudog
)

loop:
// pass 1 - look for something already waiting
var casi int
var cas *scase
var caseSuccess bool
var caseReleaseTime int64 = -1
var recvOK bool
for _, casei := range pollorder {
Expand Down Expand Up @@ -335,6 +335,7 @@ loop:
// We singly-linked up the SudoGs in lock order.
casi = -1
cas = nil
caseSuccess = false
sglist = gp.waiting
// Clear all elem before unlinking from gp.waiting.
for sg1 := gp.waiting; sg1 != nil; sg1 = sg1.waitlink {
Expand All @@ -350,6 +351,7 @@ loop:
// sg has already been dequeued by the G that woke us up.
casi = int(casei)
cas = k
caseSuccess = sglist.success
if sglist.releasetime > 0 {
caseReleaseTime = sglist.releasetime
}
Expand All @@ -368,16 +370,7 @@ loop:
}

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

c = cas.c
Expand All @@ -387,7 +380,9 @@ loop:
}

if cas.kind == caseRecv {
recvOK = true
recvOK = caseSuccess
} else if cas.kind == caseSend && !caseSuccess {
goto sclose
}

selunlock(scases, lockorder)
Expand Down

0 comments on commit eca96e3

Please sign in to comment.