Skip to content

fix: serve HTTP server has no read/write/idle deadlines, so slow clients can pin handlers and block shutdown#875

Closed
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-3939400c
Closed

fix: serve HTTP server has no read/write/idle deadlines, so slow clients can pin handlers and block shutdown#875
sam-saffron-jarvis wants to merge 1 commit into
SamSaffron:mainfrom
sam-saffron-jarvis:feat/codereview-3939400c

Conversation

@sam-saffron-jarvis

Copy link
Copy Markdown
Contributor

What changed

  • Set ReadHeaderTimeout and IdleTimeout on the serve HTTP server in cmd/serve.go.
  • Kept server-wide WriteTimeout at zero so long-lived SSE endpoints do not get an artificial lifetime cap.
  • Added a small streamingResponseWriter wrapper that applies a per-write SetWriteDeadline around every streaming Write and Flush call.
  • Wrapped the serve streaming endpoints (/v1/chat/completions, /v1/messages, and response-run SSE streams) with that writer so slow or stalled clients cannot block a handler forever.
  • Added tests covering the server timeout configuration and the per-write deadline wrapper behavior.

Why this is high-value

Before this change, serveServer.Start created an http.Server with no read-header or idle deadlines, and the streaming handlers wrote directly to http.ResponseWriter/Flush with no per-connection write deadline. That meant:

  • slowloris-style header reads could keep connections open indefinitely,
  • idle keep-alive clients could sit around forever,
  • stalled SSE clients could block inside a socket write/flush indefinitely,
  • and blocked streaming handlers could delay or defeat graceful shutdown even though contextWithShutdown tries to make them exit promptly.

This fix closes those reliability holes without breaking long-lived streaming behavior.

Validation

  • gofmt -w cmd/serve.go cmd/serve_protocol.go cmd/serve_handlers_chat.go cmd/serve_handlers_anthropic.go cmd/serve_response_runs.go cmd/serve_test.go cmd/serve_protocol_test.go
  • go build ./...
  • go test ./...
  • git diff --check

@SamSaffron

Copy link
Copy Markdown
Owner

Implemented in local copy: serve HTTP server now has read-header/idle timeouts while keeping global WriteTimeout unset for SSE, and streaming endpoints use a per-write deadline wrapper. Verified with go build ./... and targeted tests.

@SamSaffron SamSaffron closed this Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants