Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion .github/workflows/build-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,56 @@ jobs:
cmake --build tests/build -j"$(nproc)"
ctest --test-dir tests/build --output-on-failure

# Fault regressions run against a separate optimized build. Its hits are
# added to the production line table; test-only code is never counted.
# Do not install it: later lanes keep using the shipping Release module.
- name: Run hooked delivery regressions with coverage
if: matrix.debug == false && success()
working-directory: http-server
run: |
set -euxo pipefail
sudo apt-get install -y --no-install-recommends libfiu-dev
project_dir="$PWD"
aux_build="$project_dir/coverage/hooks-build"
mkdir -p "$aux_build"
(
cd "$aux_build"
PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:${PKG_CONFIG_PATH:-} \
CFLAGS='-g -O2' CXXFLAGS='-g -O2' \
"$project_dir/configure" \
--with-php-config=/usr/local/bin/php-config \
--enable-http-server --enable-http2 --enable-http3 \
--with-openssl --with-nghttp2=/usr/local \
--with-nghttp3=/usr/local --with-ngtcp2=/usr/local \
--enable-coverage --enable-fault-injection \
--enable-http-server-test-hooks --enable-tas-test-hooks
make -j4
)
/usr/local/bin/php "$GITHUB_WORKSPACE/php-src/run-tests.php" \
-n -d "extension=$aux_build/modules/true_async_server.so" \
-d opcache.protect_memory=0 -q -j4 --offline --no-progress \
--show-diff --set-timeout 120 \
tests/phpt/server/reactor_pool \
tests/phpt/server/h3/08[2-9]-*.phpt \
tests/phpt/server/h3/09[0-2]-*.phpt \
tests/phpt/server/core/083-fault-injection-hook.phpt \
tests/phpt/server/core/087-pool-partial-submit-shutdown.phpt \
tests/phpt/server/core/088-pool-worker-start-bailout.phpt \
| tee coverage/hooks-phpt.log
/usr/local/bin/php -n .github/scripts/assert_no_new_retries.php \
.github/scripts/retry-baseline.txt coverage/hooks-phpt.log
python3 - <<'PY'
import re
from pathlib import Path
log = Path('coverage/hooks-phpt.log').read_text()
skipped = re.search(r'Tests skipped\s*:\s*(\d+)', log)
if skipped is None or int(skipped[1]) != 0:
raise SystemExit('hooked coverage tests must run without skips')
PY
lcov --capture --directory "$aux_build/src" \
--output-file coverage/hooks.info \
--rc geninfo_unexecuted_blocks=1 --quiet

- name: Capture lcov + extract JSON
if: matrix.debug == false && success()
working-directory: http-server
Expand All @@ -741,7 +791,10 @@ jobs:
--rc geninfo_unexecuted_blocks=1 --quiet
python3 scripts/lcov-add-hits.py \
coverage/extension.info coverage/unit.info -o coverage/raw.info
lcov --extract coverage/raw.info "$PWD/src/*" \
python3 scripts/lcov-add-hits.py \
coverage/raw.info coverage/hooks.info -o coverage/combined.info \
--require-source src/core/worker_dispatch.c
lcov --extract coverage/combined.info "$PWD/src/*" \
--output-file coverage/filtered.info --quiet
python3 scripts/coverage-report.py \
coverage/filtered.info -o coverage/current.json
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **A full reactor mailbox no longer silently loses pooled HTTP/3 responses (#351).** The sender coroutine waits for capacity while other PHP tasks keep running. The worker records the transport's final result rather than counting an enqueue as a delivered response. A failed response with no confirmed HTTP status uses `responses_undelivered_total` and `error.type=response_undelivered`. Deterministic tests cover pressure, delayed final ACK, render/submit failure, timeout, concurrent writers and bailout during a linked wait. Cancelling the pool parent now joins worker tasks before freeing shared transport; the previous order reproduced an AddressSanitizer use-after-free in both direct cancellation and cancellation during reload. A refused first gzip `tryWrite()` now restores the fresh codec and original headers for retry. A failed sendFile pump stops issuing reads and disposes an outstanding read before releasing its buffer. Hook-dependent regressions contribute hits to the shipping Release line table through an isolated optimized test build.

- **HTTP/2 conformance failed with nghttp2 1.70.0 (#409).** Invalid PRIORITY and closed-stream DATA now receive connection errors, and a stream WINDOW_UPDATE overflow resets that stream. h2spec passes 146 cases with 0 failures on nghttp2 1.63.0, 1.70.0 and development 1.70.90; before the fix, 1.70.0 passed 141 with 5 failures. The gate counts each failed case once and rejects incomplete reports.

- **Under the reactor pool a worker could spin forever returning an HTTP/3 slot to a reactor that had left its loop (#384).** When the worker could not defer the release, it retried the post with no bound, and a reactor that has stopped refuses every post. The worker now retries only while the reactor runs and drops the release otherwise, with one notice on stderr; the slot goes with the pool. Found by reading, not seen in a run: `h3/081` drives the drop through a fault point.
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ set(CORE_SUBSYSTEM_SOURCES
src/core/fault_hooks.c
# Flat response marshalling type (issue #80, D3): worker -> reactor response.
src/core/response_wire.c
src/core/response_delivery.c
# Worker-side request dispatch (issue #80, B1b/D7): request ptr -> handler -> response.
src/core/worker_dispatch.c
# Worker inbox (issue #80, B2): per-worker request mailbox + dispatch drain.
Expand Down
1 change: 1 addition & 0 deletions config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ if test "$PHP_HTTP_SERVER" != "no"; then
src/core/reactor_pool_test_hooks.c
src/core/fault_hooks.c
src/core/response_wire.c
src/core/response_delivery.c
src/core/worker_dispatch.c
src/core/worker_inbox.c
src/core/worker_registry.c
Expand Down
1 change: 1 addition & 0 deletions config.w32
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ if (PHP_TRUE_ASYNC_SERVER == "yes") {
"reactor_pool_test_hooks.c " +
"fault_hooks.c " +
"response_wire.c " +
"response_delivery.c " +
"worker_dispatch.c " +
"worker_inbox.c " +
"worker_registry.c " +
Expand Down
19 changes: 19 additions & 0 deletions dev/DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,22 @@ Architectural decisions, newest first. Workflow rules live in `dev/WORKFLOW.md`.
Why: a second runner would drive a separate server process; phpt already runs the
server in-process.
Rejected: Behat.

- 2026-10-06 Pooled HTTP/3 delivery (#351): FULL is coroutine backpressure, not
a discard policy. Enqueue transfers ownership but does not count a response.
The reactor selects one terminal outcome at clean QUIC stream close (data and
FIN acknowledged), or failure. The original worker generation consumes that
result and its immutable request snapshot once. No confirmed status is the
fifth, `responses_undelivered_total`, bucket; the five sum to `total_requests`.
Principle: P1.2, within P2.1's existing reactor/worker exception.
Why: forced refusal timed out the client while recording 200 and ten bytes,
with zero dropped wires. A parked wire was not delivered either.
Rejected: keep dropping FULL but count the drop; use HttpRequest lastref as
completion; put failure notifications in the same full mailbox; hold the PHP
coroutine/response until peer ACK.
Cost: a persistent delivery record and copied access identity per request;
capacity gates per worker/reactor pair; await resources only on pressure.
Command slots remain bounded, but an internal producer block may allocate:
moodycamel try_enqueue refused such a block even below logical capacity.
Enqueue under an explicit slot reservation distinguishes actual allocation
failure from FULL. Control/completion publication allocates nothing.
155 changes: 155 additions & 0 deletions docs/REACTOR_RESPONSE_DELIVERY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# Pooled HTTP/3 response delivery

Issue #351. Queue acceptance, HTTP/3 apply and terminal transport completion are
different events. In-thread transports retain their existing accounting paths.

```mermaid
flowchart TD
H[Handler result] --> S[Coroutine sender and per-response writer FIFO]
S --> Q{Try bounded data mailbox}
Q -->|FULL| W[Subscribe, recheck, suspend coroutine]
W -->|Wake| Q
Q -->|Accepted| A[Mailbox owns wire; no response accounting]
A --> R[Bounded reactor drain frees slots and wakes worker gates]
R --> T[HTTP/3 apply and QUIC send or sendFile pump]
T -->|Clean close: data and FIN acknowledged| OK[Terminal normal]
S -->|Allocation, cancellation, deadline| C[Preallocated control command]
Q -->|Stopped or allocation error| C
C --> F[Reactor failure and reset]
T -->|Submit, pump, reset or connection failure| F
OK --> D[Terminal plus snapshot ready plus zero wire objects]
F --> D
D --> P[Embedded completion node and worker wake]
P --> L[Original worker generation counts and logs once]
R -.-> W
```

## Ownership

The worker registers all capacity gates before publishing its inbox. Each
delivery record references that concrete endpoint, not a reusable registry slot.
The reactor creates the record before posting the request; the worker copies
request identity before fallible PHP object/coroutine construction.

The sender owns one wire during rendering and capacity waits. The record tracks
it across allocation bailout. Accepted enqueue clears worker ownership; a later
cancel does not return it. Wire references pin the record. Request-consumed
control may overtake data, but cannot release the stream's worker borrow while
wire objects remain. Retaining a request delays memory reclamation, not the
terminal result or logger shutdown.

The worker-only writer FIFO serializes HEADERS/CHUNK and byte-credit bookkeeping.
Finalization takes the same reentrant lock before codec finish and terminal
messages. Nonblocking refusal does not suspend; an unaccepted first commit can
be rolled back. Readiness probes wait for writer, capacity and credit with one
deadline, without reserving a writer between awaitWritable and tryWrite calls.

Stack waiters are protected through the entire registered interval, including
scheduler bailout during suspend. Cleanup unlinks them before rethrow. Active
completion removal and FIFO registration use intrusive links, not linear tail
searches or per-completion scans.

## Outcome

HTTP/3 submit chooses a status, not a successful delivery. A stream-data ACK
confirms a response has begun; clean QUIC close confirms the full send side.
These events do not prove the client's application consumed the response.
Application-body ACK callbacks accumulate the confirmed body size. A sendFile
apply starts the pump; pump failure remains a delivery failure.

The reactor chooses one outcome. Cancellation requested before that choice
excludes normal completion. Connection teardown finishes records for which
ngtcp2 emits no stream-close callback. Control callbacks run the transport flush
epilogue too: a reset does not need a later data post to reach the socket.

Publication requires the terminal outcome, the ready access snapshot and zero
wire objects. Early peer closure cannot expose an unfinished snapshot. The
worker owns all counter updates and access-log emission:

```
total_requests = responses_2xx_total + responses_3xx_total
+ responses_4xx_total + responses_5xx_total
+ responses_undelivered_total
```

No confirmed status omits http.response.status_code and reports
response_undelivered. Failure after status confirmation keeps that status and
reports response_aborted. Body size is acknowledged application-body bytes at
terminal selection (encoded bytes under compression), not prepared-buffer size.
Handler service sampling remains separate; delivery reporting can use its early
identity snapshot and reactor result even when a PHP bailout invalidates the
handler sample.

## Shutdown

Retirement unpublishes the inbox and acknowledges a reliable control admission
fence on every reactor. Full mailbox is never mistaken for a completed fence.
After the normal handler-scope drain, endpoint closure cancels remaining
deliveries, consumes completions and waits for local subscriptions to leave.
Gates are unregistered before logger/endpoint destruction.

The parent closes reload admission, sends STOP and joins the worker tasks while
its event loop continues to run. Cancelling the original wait is not worker
completion. The join uses the public `Async\protect` boundary and retains the
completion events until `pending == 0` and the current reload has released its
reservation. A partial initial submission stops and joins the accepted tasks.

The worker handler catches recoverable start bailout, then completes the clone
destructor before the task future can resolve. An arbitrary fatal/OOM inside
the destructor cannot be reported as successful quiescence; progress through
that failure is not guaranteed. Shared transport must not be freed on a false
completion acknowledgment.

Cold destructor cleanup does not suspend a PHP coroutine. It detaches stack
subscriptions, discards still worker-owned wires, then acknowledges both control
and data fences. Detached cleanup cannot touch retired gates. Reactor-owned H3
teardown is fenced after worker producers have quiesced. The old pending-wire
TTL and its potentially lost release queue are gone.

## Coverage

- h3/082: forced FULL yields to another task on the same worker; render/submit
failures terminate the client; retained request does not delay completion.
- h3/083: counters stay zero while final ACK is delayed, then become one.
- h3/084: plain/gzip refused first offers restore headers and codec; a scheduler
probe and time bound verify nonblocking behavior; queue timeout has a lower
and upper time bound and resets through control without data capacity.
- h3/085: concurrent plain/gzip writers and try/await loops stay ordered; peer
closure before sender completion preserves the access identity.
- h3/086: bailout after waiter registration retracts subscriptions and owned
wire; the next request still works.
- h3/087–088: repeated parent cancellation and cancellation during reload join
workers before transport teardown; both reproduced an ASAN use-after-free
before the parent join fix.
- core/087–088: partial initial submit and recoverable pre-start worker bailout.
- h3/089: bailout from start after inbox publication still completes cleanup.
- Existing overflow, sendFile, reload, stop and bailout tests cover integration.

- h3/090: forced FULL independently during HEADERS, CHUNK, END, ABORT and
SEND_FILE; exact body/trailers, preserved file options, same-worker progress,
once-only outcome. ABORT waits for acknowledged prefix bytes and verifies
`response_aborted`, the confirmed status and exact acknowledged body size.
- h3/091: failure of a real subsequent file read resets an incomplete transfer;
one failure counter and one access record, with classification based on ACK.
- h3/092: a refused encoded chunk after stream start is a terminal failure,
rather than retrying an encoder state the peer never received.
- reactor_pool/011: retiring a queued inbox releases every request exactly once
and invokes no handler; invalid admission and NULL cleanup are also checked.

## Coverage capture

The ordinary Release build and its PHPT suite remain unchanged. A separate
optimized build enables fault injection and test hooks, runs the selected
regressions without skips or retries, and adds its hits through
`lcov-add-hits.py`. The production trace determines the source files, line
coordinates and functions. Test-only entries do not expand the denominator;
source matching is checked to prevent a silently ignored auxiliary trace.
The one-percentage-point comparison threshold remains unchanged.

## Remaining acceptance checks

The audited tests do not prove every concurrency interleaving. Dedicated checks
for cancellation while waiting in the writer FIFO or on byte credit, and late
completion during worker-generation rotation remain to be added. A clean test
run or high line coverage is not proof of these scenarios. The unbounded work
in one control-drain callback is a separate architectural review item.
1 change: 1 addition & 0 deletions include/compression/http_compression_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ void http_compression_attach(struct _zend_object *response_obj,

/* Free state attached above. Called from the response object's free_obj. */
void http_compression_state_free(struct _zend_object *response_obj);
void http_compression_reset_unstarted(struct _zend_object *response_obj);

/* Mark this response as ineligible for compression (BREACH-sensitive
* endpoints, handler-controlled binary blobs, etc.). Idempotent. */
Expand Down
27 changes: 27 additions & 0 deletions include/core/atomic_pointer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* Copyright (c) TrueAsync. Licensed under the Apache License, Version 2.0. */
#ifndef HTTP_ATOMIC_POINTER_H
#define HTTP_ATOMIC_POINTER_H
#include "Zend/zend_atomic.h"
#include <stdint.h>
/* Zend exposes pointer load/store, but no pointer CAS. Its portable 64-bit
* atomics cover uintptr_t on every supported target (including Win64). */
typedef zend_atomic_int64 http_atomic_pointer_t;
static inline void *http_atomic_pointer_load(http_atomic_pointer_t *p)
{
return (void *)(uintptr_t)zend_atomic_int64_load_ex(p);
}
static inline void *http_atomic_pointer_exchange(http_atomic_pointer_t *p, void *v)
{
return (void *)(uintptr_t)zend_atomic_int64_exchange_ex(p, (int64_t)(uintptr_t)v);
}
static inline bool http_atomic_pointer_compare_exchange(http_atomic_pointer_t *p, void **expected,
void *v)
{
int64_t old = (int64_t)(uintptr_t)*expected;
bool ok = zend_atomic_int64_compare_exchange_ex(p, &old, (int64_t)(uintptr_t)v);
if (!ok) {
*expected = (void *)(uintptr_t)old;
}
return ok;
}
#endif
Loading
Loading