Skip to content

Commit

Permalink
internal/quic: deflake TestConnTestConn
Browse files Browse the repository at this point in the history
Sending a message to a connection returns an error when the
connection event loop had exited. This is unreliable, since
a sent to the conn's message channel can succeed after the
event loop exits, writing the message to the channel buffer.

Drop the error return from Conn.sendMsg; it isn't useful,
since it's always possible for the connection to exit with
messages still in the channel buffer.

Fixes golang/go#61485

Change-Id: Ic8351f984df827af881cf7b6d93d97031d2e615c
Reviewed-on: https://go-review.googlesource.com/c/net/+/511658
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
  • Loading branch information
neild authored and gopherbot committed Jul 21, 2023
1 parent d0912d4 commit dd5bc96
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 10 deletions.
11 changes: 4 additions & 7 deletions internal/quic/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,24 +176,21 @@ func (c *Conn) loop(now time.Time) {

// sendMsg sends a message to the conn's loop.
// It does not wait for the message to be processed.
func (c *Conn) sendMsg(m any) error {
// The conn may close before processing the message, in which case it is lost.
func (c *Conn) sendMsg(m any) {
select {
case c.msgc <- m:
case <-c.donec:
return errors.New("quic: connection closed")
}
return nil
}

// runOnLoop executes a function within the conn's loop goroutine.
func (c *Conn) runOnLoop(f func(now time.Time, c *Conn)) error {
donec := make(chan struct{})
if err := c.sendMsg(func(now time.Time, c *Conn) {
c.sendMsg(func(now time.Time, c *Conn) {
defer close(donec)
f(now, c)
}); err != nil {
return err
}
})
select {
case <-donec:
case <-c.donec:
Expand Down
3 changes: 0 additions & 3 deletions internal/quic/conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ func TestConnTestConn(t *testing.T) {
tc.wait()

tc.advanceToTimer()
if err := tc.conn.sendMsg(nil); err == nil {
t.Errorf("after advancing to idle timeout, sendMsg = nil, want error")
}
if !tc.conn.exited {
t.Errorf("after advancing to idle timeout, exited = false, want true")
}
Expand Down

0 comments on commit dd5bc96

Please sign in to comment.