Skip to content

Commit

Permalink
fix: ensure that pings don't get stuck behind writes
Browse files Browse the repository at this point in the history
This sends pings/pongs first before processing messages.

This ensures that:

1. RTT times are accurate.
2. We don't end up thinking the connection is dead because a ping keeps
   getting stuck behind a message.

I tried lowering the max message size to 4KiB, but that broke tests due
to a perf regression. Although I'm not sure how "real" that perf
regression is.
  • Loading branch information
Stebalien committed Jan 6, 2022
1 parent 959e044 commit 6c2bbc3
Showing 1 changed file with 39 additions and 28 deletions.
67 changes: 39 additions & 28 deletions session.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,11 +501,9 @@ func (s *Session) sendLoop() error {
default:
}

// Flushes at least once every 100 microseconds unless we're
// constantly writing.
var buf []byte
// Make sure to send any pings & pongs first so they don't get stuck behind writes.
select {
case buf = <-s.sendCh:
case pingID := <-s.pingCh:
buf = pool.Get(headerSize)
hdr := encode(typePing, flagSYN, 0, pingID)
Expand All @@ -514,31 +512,44 @@ func (s *Session) sendLoop() error {
buf = pool.Get(headerSize)
hdr := encode(typePing, flagACK, 0, pingID)
copy(buf, hdr[:])
case <-s.shutdownCh:
return nil
//default:
// select {
// case buf = <-s.sendCh:
// case <-s.shutdownCh:
// return nil
// case <-writeTimeoutCh:
// if err := writer.Flush(); err != nil {
// if os.IsTimeout(err) {
// err = ErrConnectionWriteTimeout
// }
// return err
// }

// select {
// case buf = <-s.sendCh:
// case <-s.shutdownCh:
// return nil
// }

// if writeTimeout != nil {
// writeTimeout.Reset(s.config.WriteCoalesceDelay)
// }
// }
default:
// Then send normal data.
select {
case buf = <-s.sendCh:
case pingID := <-s.pingCh:
buf = pool.Get(headerSize)
hdr := encode(typePing, flagSYN, 0, pingID)
copy(buf, hdr[:])
case pingID := <-s.pongCh:
buf = pool.Get(headerSize)
hdr := encode(typePing, flagACK, 0, pingID)
copy(buf, hdr[:])
case <-s.shutdownCh:
return nil
//default:
// select {
// case buf = <-s.sendCh:
// case <-s.shutdownCh:
// return nil
// case <-writeTimeoutCh:
// if err := writer.Flush(); err != nil {
// if os.IsTimeout(err) {
// err = ErrConnectionWriteTimeout
// }
// return err
// }

// select {
// case buf = <-s.sendCh:
// case <-s.shutdownCh:
// return nil
// }

// if writeTimeout != nil {
// writeTimeout.Reset(s.config.WriteCoalesceDelay)
// }
// }
}
}

if err := extendWriteDeadline(); err != nil {
Expand Down

0 comments on commit 6c2bbc3

Please sign in to comment.