Skip to content

fix: bound response writes so a stalled client can't wedge a thread forever - #2574

Open
dunglas wants to merge 1 commit into
mainfrom
fix-response-write-timeout
Open

fix: bound response writes so a stalled client can't wedge a thread forever#2574
dunglas wants to merge 1 commit into
mainfrom
fix-response-write-timeout

Conversation

@dunglas

@dunglas dunglas commented Jul 28, 2026

Copy link
Copy Markdown
Member

Context

Follow-up from review of #2570 and #2564 for #2553, and from #2573.

go_read_post already bounds a stalled request body read with a deadline (#2538). go_ub_write had no equivalent: a client that stops reading the response holds the handling thread in a blocking Write() indefinitely. frankenphp_force_kill_thread() can't rescue it either — its own doc comment says it interrupts the Zend VM "at the next opcode boundary" or wakes a real blocking syscall via EINTR, but this stall is neither: Go's net/http writes go through the non-blocking netpoller, so an unresponsive peer just parks the goroutine in Go's scheduler, invisible to a signal sent to the OS thread.

This is the same mechanism behind #2573's rebootAllThreads() fix, just hit from the request-handling side instead of the reboot side: a thread stuck here can wedge a reboot or shutdown exactly like the one #2573 addresses.

Fix

Mirror go_read_post's pattern: set an idle write deadline via http.ResponseController before each go_ub_write call, driven by a new opt-in fc.responseWriteTimeout. A deadline-exceeded write is treated as a genuine disconnect (php_handle_aborted_connection() fires), not silently retried.

Exposed as:

  • WithResponseWriteTimeout() at the library level (zero disables it, matching WithRequestBodyTimeout).
  • response_write_timeout in the Caddyfile, defaulting to 60s — same default and rationale as request_body_timeout (mirrors nginx's send_timeout).

Documented alongside the existing slow-POST mitigation in docs/security.md and docs/config.md.

Test

TestResponseWriteTimeout drives a raw TCP connection that sends a request and never reads the response, so the server's socket send buffer fills and the underlying Write() blocks. Verified it fails (blocks past 5s) on the code before this fix, and passes in ~0.3s with it.

Full go test -race ./... (root + internal/...) and the caddy package suite both pass; go vet/gofmt clean.

This narrows the real-world cases #2573's Abandoned-state fallback needs to handle — any stalled HTTP/1.1 or HTTP/2 write now self-heals within the timeout — but doesn't replace it: it says nothing about flock(), a slow DNS lookup, a hung stream wrapper, or any other blocking call outside go_ub_write, and can be configured off.

…orever

frankenphp_force_kill_thread() cannot interrupt a write parked in Go's
non-blocking netpoller: the stall isn't a blocking syscall, so there's no
opcode boundary or EINTR-able syscall for it to reach. A client that stops
reading holds the handling thread in go_ub_write indefinitely.

Add an idle write deadline via http.ResponseController, mirroring the
existing read-deadline handling in go_read_post (#2538). Exposed as
WithResponseWriteTimeout() and the response_write_timeout Caddyfile
directive, defaulting to 60s like request_body_timeout.
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.

1 participant