Skip to content

Commit ef47932

Browse files
committed
Drain inbound traffic before closing to avoid TCP RST data loss
Half-close with CloseWrite() and wait for Serve() to exit before calling conn.Close(), so late peer packets don't trigger RST. Also fix a pre-existing data race in Ping() where s.pingId was read outside pingLock. Signed-off-by: Andreas Erz <andreas.erz@sap.com>
1 parent 954f580 commit ef47932

1 file changed

Lines changed: 38 additions & 2 deletions

File tree

connection.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,8 @@ func NewConnectionWithOptions(conn net.Conn, server bool, opts ...spdy.FramerOpt
280280
// Ping sends a ping frame across the connection and
281281
// returns the response time
282282
func (s *Connection) Ping() (time.Duration, error) {
283-
pid := s.pingId
284283
s.pingLock.Lock()
284+
pid := s.pingId
285285
if s.pingId > 0x7ffffffe {
286286
s.pingId = s.pingId - 0x7ffffffe
287287
} else {
@@ -740,7 +740,43 @@ func (s *Connection) shutdown(closeTimeout time.Duration) {
740740
var err error
741741
select {
742742
case <-streamsClosed:
743-
// No active streams, close should be safe
743+
// No active streams; now drain inbound traffic before closing so the
744+
// kernel sees FIN, not RST. Background: If a peer packet (e.g. a SPDY
745+
// PING) arrives at our socket after Close(), the kernel responds with
746+
// RST and discards anything still queued in OUR kernel send buffer.
747+
//
748+
// Half-closing the write end of the connection triggers a FIN once the
749+
// send buffer is emptied and prevents new packets from being sent, but
750+
// still allows the receive buffer to be drained.
751+
func() {
752+
cw, ok := s.conn.(interface{ CloseWrite() error })
753+
if !ok {
754+
debugMessage("(%p) connection does not support half-close, skipping drain", s)
755+
return
756+
}
757+
if err := cw.CloseWrite(); err != nil {
758+
debugMessage("(%p) failed to half-close connection: %s", s, err)
759+
return
760+
}
761+
var drainTimeout <-chan time.Time
762+
if closeTimeout == time.Duration(0) {
763+
// no close timeout configured; use fixed drain timeout to avoid
764+
// hanging if peer does not respond or Serve() was not called
765+
drainTimer := time.NewTimer(10 * time.Second)
766+
defer drainTimer.Stop()
767+
drainTimeout = drainTimer.C
768+
}
769+
select {
770+
case <-s.closeChan:
771+
return
772+
case <-timeout:
773+
debugMessage("(%p) close timeout reached", s)
774+
return
775+
case <-drainTimeout:
776+
debugMessage("(%p) drain timeout reached", s)
777+
return
778+
}
779+
}()
744780
err = s.conn.Close()
745781
case <-timeout:
746782
// Force ungraceful close

0 commit comments

Comments
 (0)