fix: bound response writes so a stalled client can't wedge a thread forever - #2574
Open
dunglas wants to merge 1 commit into
Open
fix: bound response writes so a stalled client can't wedge a thread forever#2574dunglas wants to merge 1 commit into
dunglas wants to merge 1 commit into
Conversation
…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.
This was referenced Jul 28, 2026
dunglas
marked this pull request as ready for review
July 28, 2026 12:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Follow-up from review of #2570 and #2564 for #2553, and from #2573.
go_read_postalready bounds a stalled request body read with a deadline (#2538).go_ub_writehad no equivalent: a client that stops reading the response holds the handling thread in a blockingWrite()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'snet/httpwrites 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 viahttp.ResponseControllerbefore eachgo_ub_writecall, driven by a new opt-infc.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, matchingWithRequestBodyTimeout).response_write_timeoutin the Caddyfile, defaulting to 60s — same default and rationale asrequest_body_timeout(mirrors nginx'ssend_timeout).Documented alongside the existing slow-POST mitigation in
docs/security.mdanddocs/config.md.Test
TestResponseWriteTimeoutdrives a raw TCP connection that sends a request and never reads the response, so the server's socket send buffer fills and the underlyingWrite()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 thecaddypackage suite both pass;go vet/gofmtclean.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 aboutflock(), a slow DNS lookup, a hung stream wrapper, or any other blocking call outsidego_ub_write, and can be configured off.