Skip to content

fix(h3): preserve responses under reactor backpressure - #411

Merged
EdmondDantes merged 3 commits into
mainfrom
fix/351-response-backpressure
Oct 6, 2026
Merged

EdmondDantes merged 3 commits into
mainfrom
fix/351-response-backpressure

Conversation

@EdmondDantes

@EdmondDantes EdmondDantes commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

A full reactor mailbox could discard a pooled HTTP/3 response while the worker logged 200 and the prepared body size, leaving the client waiting. The sender now preserves the wire and suspends its coroutine until capacity is available. Queue acceptance transfers ownership; accounting waits for the terminal transport outcome.

The reactor publishes one completion to the originating worker generation after the request snapshot is ready and all wire objects are released. Clean completion requires data and FIN acknowledgement. Failures before a confirmed status use responses_undelivered_total and response_undelivered; failures after confirmation retain the status and report response_aborted. An independent preallocated control path handles cancellation, timeout and shutdown even when the data mailbox is full.

Per-response writer ordering, retryable nonblocking first writes, waiter bailout cleanup and shutdown fences replace the old pending-wire TTL. The parent closes reload admission, sends STOP and joins worker tasks before destroying shared transport. The join retains completion events, runs the event loop through public Async\protect, and waits for the current reload reservation to unwind. Partial worker submission stops and joins accepted tasks; recoverable worker start bailout completes clone cleanup before the task future can resolve. Arbitrary fatal/OOM inside a destructor does not acknowledge successful quiescence; progress through that failure is not guaranteed.

The flow and ownership rules are documented in docs/REACTOR_RESPONSE_DELIVERY.md.

Validation:

  • Full PHPT discovery: 570 tests; 543 passed immediately, 2 passed on retry, 25 skipped, 0 final failures. The two retries were tls/006 and tls/007. Those and the two TLS/static tests that retried in the preceding full run all passed on a separate run on both the old and updated normal modules. The full run is not claimed retry-free. Of the skips, 24 require test hooks absent from that build; one depends on kernel SO_REUSEPORT behavior.
  • After the final cancellation-origin adjustment, all 8 affected shutdown tests passed.
  • Unit suite: 22/22 passed.
  • AddressSanitizer: 11/11 targeted tests passed, including the original five delivery regressions, five new shutdown regressions, and H3 reload. Leak detection was disabled; this was not the full PHPT suite under ASAN.
  • Before the parent-join fix, both new parent-cancellation tests reproduced ASAN heap-use-after-free in reactor_pool_is_running; they now pass. The expanded ASAN attempt also crashed in three pre-existing core graceful-shutdown tests before the first server response. The same crashes reproduced on the previous PR module in the same ASAN environment; the three tests pass on the normal build.
  • Deterministic coverage: forced FULL yields to another task on the same worker; delayed final ACK prevents early accounting; render/submit failure, nonblocking retry, timeout, concurrent plain/gzip writers, early close and bailout during a registered wait. Shutdown tests add repeated parent cancellation with a busy worker, cancellation during reload, partial initial submit, pre-start bailout, and bailout after inbox publication.
  • Critic and subsequent Astra review approved the scoped parent-shutdown fix after finding and addressing the lifetime issue.

Remaining review items:

  • Control drain still processes a complete detached batch; bounding reversal and execution work per reactor callback remains separate work.
  • Coverage capture currently runs in Release without fault injection, so four original delivery regressions do not contribute to that report. The coverage decrease remains unresolved; no override was added.

Closes #351.

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Coverage

Total lines: 84.69% → 85.27% (+0.58 pp)

File Baseline Current Δ Touched
src/compression/http_compression_response.c 77.64% 83.62% +5.98 pp ●
src/core/reactor_pool.c 79.17% 86.75% +7.59 pp ●
src/core/reactor_pool_test_hooks.c 100.00% 100.00% +0.00 pp ●
src/core/response_delivery.c 0.00% 82.14% +82.14 pp ●
src/core/response_wire.c 99.42% 99.44% +0.02 pp ●
src/core/thread_mailbox.c 79.31% 81.51% +2.20 pp ●
src/core/thread_queue.cc 81.67% 80.33% -1.34 pp ●
src/core/worker_dispatch.c 83.45% 85.92% +2.47 pp ●
src/core/worker_inbox.c 93.10% 95.24% +2.13 pp ●
src/core/worker_registry.c 79.07% 94.19% +15.12 pp
src/http3/http3_callbacks.c 86.28% 87.05% +0.77 pp ●
src/http3/http3_connection.c 82.88% 86.39% +3.51 pp ●
src/http3/http3_dispatch.c 88.37% 88.84% +0.48 pp ●
src/http3/http3_listener.c 79.01% 79.16% +0.15 pp
src/http3/http3_static_response.c 76.67% 75.71% -0.96 pp ●
src/http3/http3_stream.c 88.29% 90.68% +2.39 pp ●
src/http_request.c 86.25% 88.71% +2.45 pp ●
src/http_response.c 89.92% 90.55% +0.63 pp ●
src/http_server_class.c 76.33% 78.19% +1.86 pp ●
src/log/http_log.c 70.45% 70.59% +0.14 pp ●
src/room/room_hub.c 66.25% 66.59% +0.34 pp

❌ Regression in touched files (> 1.0 pp drop)

  • src/core/thread_queue.cc dropped -1.34 pp

Add [coverage-drop-ok] to a commit message in this PR to override.

@EdmondDantes
EdmondDantes merged commit 19cfc24 into main Oct 6, 2026
8 checks passed
@EdmondDantes
EdmondDantes deleted the fix/351-response-backpressure branch October 6, 2026 21:54
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.

A response wire the reactor-pool worker cannot deliver leaves the peer waiting and is recorded as delivered

1 participant