Skip to content

Commit

Permalink
http2: add support for server-originated pings
Browse files Browse the repository at this point in the history
Add configurable support for health-checking idle connections
accepted by the HTTP/2 server, following the same configuration
as the Transport.

Fixes golang/go#67812

Change-Id: Ia4014e691546b2c29db8dad3af5f39966d0ceb93
Reviewed-on: https://go-review.googlesource.com/c/net/+/601497
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
neild committed Sep 25, 2024
1 parent 541dbe5 commit 4790dc7
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 0 deletions.
60 changes: 60 additions & 0 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"bufio"
"bytes"
"context"
"crypto/rand"
"crypto/tls"
"errors"
"fmt"
Expand Down Expand Up @@ -127,6 +128,16 @@ type Server struct {
// If zero or negative, there is no timeout.
IdleTimeout time.Duration

// ReadIdleTimeout is the timeout after which a health check using a ping
// frame will be carried out if no frame is received on the connection.
// If zero, no health check is performed.
ReadIdleTimeout time.Duration

// PingTimeout is the timeout after which the connection will be closed
// if a response to a ping is not received.
// If zero, a default of 15 seconds is used.
PingTimeout time.Duration

// WriteByteTimeout is the timeout after which a connection will be
// closed if no data can be written to it. The timeout begins when data is
// available to write, and is extended whenever any bytes are written.
Expand Down Expand Up @@ -644,9 +655,12 @@ type serverConn struct {
inGoAway bool // we've started to or sent GOAWAY
inFrameScheduleLoop bool // whether we're in the scheduleFrameWrite loop
needToSendGoAway bool // we need to schedule a GOAWAY frame write
pingSent bool
sentPingData [8]byte
goAwayCode ErrCode
shutdownTimer timer // nil until used
idleTimer timer // nil if unused
readIdleTimer timer // nil if unused

// Owned by the writeFrameAsync goroutine:
headerWriteBuf bytes.Buffer
Expand Down Expand Up @@ -974,11 +988,17 @@ func (sc *serverConn) serve() {
defer sc.idleTimer.Stop()
}

if sc.srv.ReadIdleTimeout > 0 {
sc.readIdleTimer = sc.srv.afterFunc(sc.srv.ReadIdleTimeout, sc.onReadIdleTimer)
defer sc.readIdleTimer.Stop()
}

go sc.readFrames() // closed by defer sc.conn.Close above

settingsTimer := sc.srv.afterFunc(firstSettingsTimeout, sc.onSettingsTimer)
defer settingsTimer.Stop()

lastFrameTime := sc.srv.now()
loopNum := 0
for {
loopNum++
Expand All @@ -992,6 +1012,7 @@ func (sc *serverConn) serve() {
case res := <-sc.wroteFrameCh:
sc.wroteFrame(res)
case res := <-sc.readFrameCh:
lastFrameTime = sc.srv.now()
// Process any written frames before reading new frames from the client since a
// written frame could have triggered a new stream to be started.
if sc.writingFrameAsync {
Expand Down Expand Up @@ -1023,6 +1044,8 @@ func (sc *serverConn) serve() {
case idleTimerMsg:
sc.vlogf("connection is idle")
sc.goAway(ErrCodeNo)
case readIdleTimerMsg:
sc.handlePingTimer(lastFrameTime)
case shutdownTimerMsg:
sc.vlogf("GOAWAY close timer fired; closing conn from %v", sc.conn.RemoteAddr())
return
Expand Down Expand Up @@ -1061,19 +1084,51 @@ func (sc *serverConn) serve() {
}
}

func (sc *serverConn) handlePingTimer(lastFrameReadTime time.Time) {
if sc.pingSent {
sc.vlogf("timeout waiting for PING response")
sc.conn.Close()
return
}

pingAt := lastFrameReadTime.Add(sc.srv.ReadIdleTimeout)
now := sc.srv.now()
if pingAt.After(now) {
// We received frames since arming the ping timer.
// Reset it for the next possible timeout.
sc.readIdleTimer.Reset(pingAt.Sub(now))
return
}

sc.pingSent = true
// Ignore crypto/rand.Read errors: It generally can't fail, and worse case if it does
// is we send a PING frame containing 0s.
_, _ = rand.Read(sc.sentPingData[:])
sc.writeFrame(FrameWriteRequest{
write: &writePing{data: sc.sentPingData},
})
pingTimeout := sc.srv.PingTimeout
if pingTimeout <= 0 {
pingTimeout = 15 * time.Second
}
sc.readIdleTimer.Reset(pingTimeout)
}

type serverMessage int

// Message values sent to serveMsgCh.
var (
settingsTimerMsg = new(serverMessage)
idleTimerMsg = new(serverMessage)
readIdleTimerMsg = new(serverMessage)
shutdownTimerMsg = new(serverMessage)
gracefulShutdownMsg = new(serverMessage)
handlerDoneMsg = new(serverMessage)
)

func (sc *serverConn) onSettingsTimer() { sc.sendServeMsg(settingsTimerMsg) }
func (sc *serverConn) onIdleTimer() { sc.sendServeMsg(idleTimerMsg) }
func (sc *serverConn) onReadIdleTimer() { sc.sendServeMsg(readIdleTimerMsg) }
func (sc *serverConn) onShutdownTimer() { sc.sendServeMsg(shutdownTimerMsg) }

func (sc *serverConn) sendServeMsg(msg interface{}) {
Expand Down Expand Up @@ -1604,6 +1659,11 @@ func (sc *serverConn) processFrame(f Frame) error {
func (sc *serverConn) processPing(f *PingFrame) error {
sc.serveG.check()
if f.IsAck() {
if sc.pingSent && sc.sentPingData == f.Data {
// This is a response to a PING we sent.
sc.pingSent = false
sc.readIdleTimer.Reset(sc.srv.ReadIdleTimeout)
}
// 6.7 PING: " An endpoint MUST NOT respond to PING frames
// containing this flag."
return nil
Expand Down
43 changes: 43 additions & 0 deletions http2/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4706,3 +4706,46 @@ func TestServerWriteByteTimeout(t *testing.T) {
st.advance(1 * time.Second) // timeout after failing to write any more bytes
st.wantClosed()
}

func TestServerPingSent(t *testing.T) {
const readIdleTimeout = 15 * time.Second
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
}, func(s *Server) {
s.ReadIdleTimeout = readIdleTimeout
})
st.greet()

st.wantIdle()

st.advance(readIdleTimeout)
_ = readFrame[*PingFrame](t, st)
st.wantIdle()

st.advance(14 * time.Second)
st.wantIdle()
st.advance(1 * time.Second)
st.wantClosed()
}

func TestServerPingResponded(t *testing.T) {
const readIdleTimeout = 15 * time.Second
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
}, func(s *Server) {
s.ReadIdleTimeout = readIdleTimeout
})
st.greet()

st.wantIdle()

st.advance(readIdleTimeout)
pf := readFrame[*PingFrame](t, st)
st.wantIdle()

st.advance(14 * time.Second)
st.wantIdle()

st.writePing(true, pf.Data)

st.advance(2 * time.Second)
st.wantIdle()
}
10 changes: 10 additions & 0 deletions http2/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ func (se StreamError) writeFrame(ctx writeContext) error {

func (se StreamError) staysWithinBuffer(max int) bool { return frameHeaderLen+4 <= max }

type writePing struct {
data [8]byte
}

func (w writePing) writeFrame(ctx writeContext) error {
return ctx.Framer().WritePing(false, w.data)
}

func (w writePing) staysWithinBuffer(max int) bool { return frameHeaderLen+len(w.data) <= max }

type writePingAck struct{ pf *PingFrame }

func (w writePingAck) writeFrame(ctx writeContext) error {
Expand Down

0 comments on commit 4790dc7

Please sign in to comment.