Skip to content

fix: spool queued request bodies to break HTTP/2 upload deadlock#2541

Draft
dunglas wants to merge 1 commit into
mainfrom
fix/concurrent-uploads-http2-deadlock
Draft

fix: spool queued request bodies to break HTTP/2 upload deadlock#2541
dunglas wants to merge 1 commit into
mainfrom
fix/concurrent-uploads-http2-deadlock

Conversation

@dunglas

@dunglas dunglas commented Jul 20, 2026

Copy link
Copy Markdown
Member

Problem

Concurrent file uploads multiplexed on a single HTTP/2 connection hang indefinitely when there are more streams than PHP threads (#1074).

Root cause is an HTTP/2 flow-control deadlock, not thread-pool exhaustion:

  • Each stream's goroutine blocks in ServeHTTP until a PHP thread frees. A queued stream never reads its request body.
  • An undrained stream keeps its flow-control window open. Enough queued streams exhaust the connection-level receive window (~1 MiB default).
  • The client can no longer send body data on any stream, including the ones that already hold a thread. Those threads block in Body.Read forever. Circular wait.

This matches the reproduction: hang at default thread counts, never with separate connections (no multiplexing). With max_wait_time unset (infinite), it hangs forever with no error.

Fix

Drain a request body into a buffer right before it enters the thread queue, releasing the flow-control window so every stream is read by someone (a thread or the spooler).

  • New spoolRequestBody: buffers to memory, spills past 2 MiB to a temp file, honors request_body_timeout during the drain, cleans up the temp file when the request ends.
  • Called from the regular and worker queue paths only. Requests that get a thread immediately still stream their body live, so the happy path is unchanged.
  • Only bodies with a known, non-zero Content-Length are spooled; chunked / unknown-length streaming requests keep their live stream.
  • A body overrunning a request_body max_size limit (http.MaxBytesReader) is rejected with 413 instead of reaching PHP truncated.

Tests

  • TestConcurrentUploadsHTTP2: 30 concurrent uploads on one h2c connection with 2 threads. Deadlocks without the fix (verified: hangs to the timeout), passes with it.
  • Unit tests for the in-memory path, temp-file spill + cleanup, streaming skip, idempotency, and the 413 rejection.

Notes / trade-offs

  • Queued bodies are fully buffered instead of streamed to PHP; PHP already buffers uploads to disk, so real impact is small.
  • Large queued bodies spill to os.TempDir(), bounded by the client's Content-Length. Pair with request_body max_size to cap it.
  • The 2 MiB memory threshold is currently hardcoded; can be exposed as an option later if needed.

Closes #1074

Concurrent uploads multiplexed on one HTTP/2 connection hang when there
are more streams than PHP threads. A stream queued waiting for a thread
keeps its flow-control window open while no one reads the body; enough
queued streams exhaust the connection-level window and stall every stream
on the connection, including those a thread is already serving.

Drain a request body into a buffer (spilling past 2 MiB to a temp file)
before it enters the queue, releasing the window so every stream is read
by someone. Requests that get a thread immediately still stream live.

Bodies overrunning a request_body max_size limit are rejected with 413
instead of reaching PHP truncated.

Closes #1074
@AlliBalliBaba

Copy link
Copy Markdown
Contributor

Would it also be possible to just read a fixed amount of bytes ahead and prevent the deadlock like that? (maybe even based on MaxConcurrentStreams)

Reading the whole body seems like it might open up other potential issues regarding DOS and requires managing a separate max body size. Also will lead to files being buffered to disk essentially twice.

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.

Concurrent file uploads hang when using HTTPS

2 participants