mitchellh.com / tack
Stitch any CI into Tangled
151
fork

Configure Feed

Select the types of activity you want to include in your feed.

http: bound websocket writes with deadlines

The /logs and /events handlers wrote frames with conn.WriteMessage
and never set a write deadline. A client that stopped reading but
kept the TCP connection open could fill the kernel send buffer and
park the handler goroutine on a write forever, leaking the request
context, the broker subscription, and the log producer.

Add a wsWriteWait constant (10s) and call SetWriteDeadline before
every WriteMessage in the logs drain loop and in streamEvents. The
keep-alive ping and the closing close-frame already used WriteControl,
which takes a deadline argument directly; raise their bounds from 1s
to wsWriteWait for consistency. A stuck peer now fails the next write
within ~10s and the handler unwinds cleanly.

authored by

Mitchell Hashimoto and committed by
Tangled 7b4d240e 878b78ff

+39 -3
+39 -3
http.go
··· 34 34 "go.mitchellh.com/tack/internal/buildkite" 35 35 ) 36 36 37 + // wsWriteWait bounds how long any single websocket write (frame or 38 + // control) is allowed to block before we treat the peer as dead. A 39 + // client that stops reading but keeps the TCP connection open would 40 + // otherwise hang the handler indefinitely on a full kernel send buffer. 41 + // 10s is intentionally generous: real backpressure resolves in 42 + // milliseconds, so anything past that is a stuck peer we'd rather drop 43 + // than serve. 44 + const wsWriteWait = 10 * time.Second 45 + 37 46 // runHTTP starts the spindle's HTTP server and blocks until ctx is 38 47 // cancelled or the listener returns a fatal error. On ctx cancellation it 39 48 // performs a graceful shutdown with a bounded timeout. ··· 285 294 defer func() { 286 295 // Send a close frame on the way out so the appview proxy 287 296 // sees a clean shutdown. Mirrors upstream 288 - // spindle.(*Spindle).Logs. 297 + // spindle.(*Spindle).Logs. WriteControl honours the 298 + // deadline argument directly, so a stuck peer can't hang 299 + // us here. 289 300 _ = conn.WriteControl( 290 301 websocket.CloseMessage, 291 302 websocket.FormatCloseMessage( 292 303 websocket.CloseNormalClosure, "log stream complete", 293 304 ), 294 - time.Now().Add(time.Second), 305 + time.Now().Add(wsWriteWait), 295 306 ) 296 307 conn.Close() 297 308 }() ··· 327 338 // is a programmer bug. Log and bail rather than 328 339 // poison the stream with a half-frame. 329 340 logger.Error("marshal log line", 341 + "err", err, 342 + "knot", knot, 343 + "pipeline_rkey", pipelineRkey, 344 + "workflow", workflow, 345 + ) 346 + return 347 + } 348 + // Bound the write so a client that stopped reading 349 + // but kept the TCP connection open can't hang us on a 350 + // full kernel send buffer. WriteMessage doesn't take a 351 + // deadline argument the way WriteControl does — we 352 + // have to set it on the conn before each frame. 353 + if err := conn.SetWriteDeadline(time.Now().Add(wsWriteWait)); err != nil { 354 + logger.Debug("logs set write deadline failed", 330 355 "err", err, 331 356 "knot", knot, 332 357 "pipeline_rkey", pipelineRkey, ··· 440 465 return 441 466 } 442 467 case <-ticker.C: 468 + // WriteControl takes its own deadline argument, so 469 + // the ping itself can't hang us — but we still want a 470 + // generous-but-bounded ceiling to match the per-frame 471 + // write timeout. 443 472 if err := conn.WriteControl( 444 473 websocket.PingMessage, nil, 445 - time.Now().Add(time.Second), 474 + time.Now().Add(wsWriteWait), 446 475 ); err != nil { 447 476 logger.Debug("events ping failed", "err", err) 448 477 return ··· 474 503 }) 475 504 if err != nil { 476 505 return fmt.Errorf("marshal envelope: %w", err) 506 + } 507 + // Bound the per-frame write so a client that stopped reading 508 + // (but didn't close the TCP connection) can't hang the 509 + // handler on a full kernel send buffer. WriteMessage has no 510 + // deadline argument of its own — we set it on the conn. 511 + if err := conn.SetWriteDeadline(time.Now().Add(wsWriteWait)); err != nil { 512 + return fmt.Errorf("set write deadline: %w", err) 477 513 } 478 514 if err := conn.WriteMessage(websocket.TextMessage, frame); err != nil { 479 515 return fmt.Errorf("write frame: %w", err)