From 140d828f22bc1b7fbf2fc69df9d2e10b8aa3791b Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:42:36 +0300 Subject: [PATCH 1/3] fix(h3): preserve responses under reactor backpressure (#351) --- CHANGELOG.md | 2 + CMakeLists.txt | 1 + config.m4 | 1 + config.w32 | 1 + dev/DECISIONS.md | 19 + docs/REACTOR_RESPONSE_DELIVERY.md | 108 +++ .../compression/http_compression_response.h | 1 + include/core/atomic_pointer.h | 27 + include/core/reactor_pool.h | 33 + include/core/response_delivery.h | 61 ++ include/core/response_wire.h | 3 + include/core/thread_mailbox.h | 3 + include/core/thread_queue.h | 15 +- include/core/worker_dispatch.h | 3 + include/core/worker_inbox.h | 5 + include/http1/http_parser.h | 2 + include/http3/http3_stream.h | 3 + include/php_http_server.h | 9 +- src/compression/http_compression_response.c | 22 + src/core/reactor_pool.c | 186 +++- src/core/response_delivery.c | 852 ++++++++++++++++++ src/core/response_wire.c | 13 + src/core/thread_mailbox.c | 23 +- src/core/thread_queue.cc | 18 +- src/core/worker_dispatch.c | 579 ++++++++++-- src/core/worker_inbox.c | 24 + src/http3/http3_callbacks.c | 17 +- src/http3/http3_connection.c | 3 +- src/http3/http3_dispatch.c | 78 +- src/http3/http3_stream.c | 33 +- src/http_request.c | 8 +- src/http_response.c | 15 + src/http_server_class.c | 330 +------ src/log/http_log.c | 6 +- tests/h3client/h3client.c | 72 +- .../050-h3-reactor-pool-mailbox-overflow.phpt | 10 +- ...53-h3-pool-sendfile-status-accounting.phpt | 9 +- .../h3/082-h3-delivery-backpressure.phpt | 123 +++ .../server/h3/083-h3-delivery-final-ack.phpt | 69 ++ .../084-h3-delivery-nonblocking-timeout.phpt | 93 ++ .../085-h3-delivery-writers-early-close.phpt | 107 +++ .../h3/086-h3-delivery-wait-bailout.phpt | 89 ++ .../telemetry/009-getstats-contract.phpt | 1 + tests/unit/CMakeLists.txt | 1 + tests/unit/common/delivery_stubs.c | 14 + 45 files changed, 2614 insertions(+), 478 deletions(-) create mode 100644 docs/REACTOR_RESPONSE_DELIVERY.md create mode 100644 include/core/atomic_pointer.h create mode 100644 include/core/response_delivery.h create mode 100644 src/core/response_delivery.c create mode 100644 tests/phpt/server/h3/082-h3-delivery-backpressure.phpt create mode 100644 tests/phpt/server/h3/083-h3-delivery-final-ack.phpt create mode 100644 tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt create mode 100644 tests/phpt/server/h3/085-h3-delivery-writers-early-close.phpt create mode 100644 tests/phpt/server/h3/086-h3-delivery-wait-bailout.phpt create mode 100644 tests/unit/common/delivery_stubs.c diff --git a/CHANGELOG.md b/CHANGELOG.md index e072f4f7..cc2a8784 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. + - **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. diff --git a/CMakeLists.txt b/CMakeLists.txt index e3af9a4b..ffce9706 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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. diff --git a/config.m4 b/config.m4 index 01ce3672..0c164b93 100644 --- a/config.m4 +++ b/config.m4 @@ -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 diff --git a/config.w32 b/config.w32 index 2525a776..850535fd 100644 --- a/config.w32 +++ b/config.w32 @@ -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 " + diff --git a/dev/DECISIONS.md b/dev/DECISIONS.md index 963b6fc2..3004aea0 100644 --- a/dev/DECISIONS.md +++ b/dev/DECISIONS.md @@ -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. diff --git a/docs/REACTOR_RESPONSE_DELIVERY.md b/docs/REACTOR_RESPONSE_DELIVERY.md new file mode 100644 index 00000000..145db6be --- /dev/null +++ b/docs/REACTOR_RESPONSE_DELIVERY.md @@ -0,0 +1,108 @@ +# 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. + +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: refused nonblocking first offer is retryable; queue deadline 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. +- Existing overflow, sendFile, reload, stop and bailout tests cover integration. diff --git a/include/compression/http_compression_response.h b/include/compression/http_compression_response.h index aa73c19e..adf85968 100644 --- a/include/compression/http_compression_response.h +++ b/include/compression/http_compression_response.h @@ -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. */ diff --git a/include/core/atomic_pointer.h b/include/core/atomic_pointer.h new file mode 100644 index 00000000..9f2324d0 --- /dev/null +++ b/include/core/atomic_pointer.h @@ -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 +/* 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 diff --git a/include/core/reactor_pool.h b/include/core/reactor_pool.h index 8125b3d3..901ec527 100644 --- a/include/core/reactor_pool.h +++ b/include/core/reactor_pool.h @@ -12,6 +12,7 @@ #include #include #include +#include "core/thread_queue.h" /* * Reactor thread pool (issue #80, design D1 — substrate). @@ -61,6 +62,34 @@ bool reactor_pool_post(reactor_pool_t *rp, int idx, void *item); /* A function run on a reactor's own thread by reactor_pool_exec. */ typedef void (*reactor_exec_fn)(void *arg); +/* Intrusive, preallocated control command. Unlike data commands, this path + * has no mailbox capacity and allocates nothing on post. A node may be posted + * only once at a time; it must live through callback execution. */ +typedef struct reactor_control_s { + struct reactor_control_s *next; + reactor_exec_fn fn; + void *arg; +} reactor_control_t; +bool reactor_pool_post_control(reactor_pool_t *rp, int idx, + reactor_control_t *node); +/* Cold startup/shutdown fence: guaranteed control path, no heap allocation. + * Producers must still hold the pool lifetime. Never use for response writes. */ +bool reactor_pool_control_exec(reactor_pool_t *rp, int idx, + reactor_exec_fn fn, void *arg); + +/* Register before publishing the producer; unregister after its deliveries + * have quiesced. The list and callbacks belong to the reactor. Used to wake + * coroutine senders after a bounded mailbox drain, without blocking a thread. */ +typedef struct reactor_capacity_gate_s { + struct reactor_capacity_gate_s *next; + reactor_exec_fn wake; + void *arg; +} reactor_capacity_gate_t; +bool reactor_pool_capacity_register(reactor_pool_t *rp, int idx, + reactor_capacity_gate_t *gate); +bool reactor_pool_capacity_unregister(reactor_pool_t *rp, int idx, + reactor_capacity_gate_t *gate); + /* Run fn(arg) on reactor `idx`'s own loop thread and block the caller until it * returns. The reactor executes it inline on its drain pass — this is how * transport that must be bound to the reactor's libuv loop (a uv handle, the @@ -80,11 +109,15 @@ bool reactor_pool_exec(reactor_pool_t *rp, int idx, reactor_exec_fn fn, void *ar * non-running reactor, or a full mailbox (backpressure — the caller keeps `arg` * and decides to drop/retry). Any thread; must not race destroy(). */ bool reactor_pool_post_exec(reactor_pool_t *rp, int idx, reactor_exec_fn fn, void *arg); +thread_queue_result_t reactor_pool_try_post_exec(reactor_pool_t *rp, int idx, + reactor_exec_fn fn, void *arg); /* Whether reactor `idx` is in its loop and takes posts. False for a bad index, * before the loop starts and once it has left; a reactor that left does not * come back. Any thread; must not race destroy(). */ bool reactor_pool_is_running(const reactor_pool_t *rp, int idx); +/* Advisory only; successful enqueue remains the sole ownership boundary. */ +bool reactor_pool_has_capacity(const reactor_pool_t *rp, int idx); /* Count of items reactor `idx` has drained from its inbound. Rises as the * reactor services work — "alive" == "draining". Returns 0 for a bad index. */ diff --git a/include/core/response_delivery.h b/include/core/response_delivery.h new file mode 100644 index 00000000..80c7917a --- /dev/null +++ b/include/core/response_delivery.h @@ -0,0 +1,61 @@ +/* Copyright (c) TrueAsync. Licensed under the Apache License, Version 2.0. */ +#ifndef RESPONSE_DELIVERY_H +#define RESPONSE_DELIVERY_H + +#include "php.h" +#include "Zend/zend_async_API.h" +#include "core/reactor_pool.h" + +typedef struct response_delivery_owner_s response_delivery_owner_t; +typedef struct response_delivery_s response_delivery_t; +struct http_request_t; +struct _http_server_object; +struct response_wire_s; + +/* One endpoint per worker generation, registered before publishing its inbox. + * close runs after the inbox fence and the handler-scope drain, before logger + * shutdown. It cancels unfinished transports and waits for their publications. */ +response_delivery_owner_t *response_delivery_owner_create(void *server, reactor_pool_t *pool); +void response_delivery_owner_close(response_delivery_owner_t *owner); +void response_delivery_owner_free(response_delivery_owner_t *owner); + +/* Reactor creates the record before dispatch. The worker owns the request; + * neither terminal reporting nor reset depends on the request's last ref. */ +response_delivery_t *response_delivery_create(response_delivery_owner_t *owner, int reactor, + void *stream, reactor_exec_fn reset, + reactor_exec_fn release_request, + reactor_exec_fn apply); +void response_delivery_abandon(response_delivery_t *d); +void response_delivery_addref(response_delivery_t *d); +void response_delivery_release(response_delivery_t *d); +void response_delivery_worker_begin(response_delivery_t *d, const struct http_request_t *req); +void response_delivery_sender_done(response_delivery_t *d, const struct http_request_t *req, + zend_object *response, bool report); +void response_delivery_sender_abandoned(response_delivery_t *d); +void response_delivery_own_wire(response_delivery_t *d, struct response_wire_s *wire); +void response_delivery_discard_owned(response_delivery_t *d); +void response_delivery_note_drop(response_delivery_t *d); +bool response_delivery_post(response_delivery_t *d, struct response_wire_s *wire, + uint32_t timeout_ms, bool nonblocking); +void response_delivery_cancel(response_delivery_t *d, const char *reason); +bool response_delivery_failed(const response_delivery_t *d); +bool response_delivery_sendable(response_delivery_t *d); +bool response_delivery_wait_writable(response_delivery_t *d, uint32_t timeout_ms); + +/* Reactor-only. finish and the sender's snapshot publish independently; + * completion is queued once only when both are ready. */ +void response_delivery_started(response_delivery_t *d, int status); +void response_delivery_ack(response_delivery_t *d, uint64_t bytes); +void response_delivery_ack_transport(response_delivery_t *d, uint64_t bytes); +void response_delivery_finish(response_delivery_t *d, bool clean, const char *reason); +void response_delivery_stream_gone(response_delivery_t *d); +void response_delivery_maybe_release_request(response_delivery_t *d); + +/* Worker-only: post request-consumed control to the reactor. */ +void response_delivery_request_consumed(response_delivery_t *d); + +/* Any thread holding a record reference: track wire lifetime. */ +void response_delivery_wire_acquire(response_delivery_t *d); +void response_delivery_wire_release(response_delivery_t *d); + +#endif diff --git a/include/core/response_wire.h b/include/core/response_wire.h index 530476ea..30ee70a1 100644 --- a/include/core/response_wire.h +++ b/include/core/response_wire.h @@ -36,6 +36,9 @@ */ typedef struct response_wire_s response_wire_t; +typedef struct response_delivery_s response_delivery_t; +void response_wire_set_delivery(response_wire_t *rw, response_delivery_t *delivery); +response_delivery_t *response_wire_delivery(const response_wire_t *rw); /* FULL = single-shot at dispose. Streamed responses go FIFO on one reactor * mailbox: one STREAM_HEADERS, N STREAM_CHUNKs, one STREAM_END (may carry diff --git a/include/core/thread_mailbox.h b/include/core/thread_mailbox.h index 48c10271..6882b638 100644 --- a/include/core/thread_mailbox.h +++ b/include/core/thread_mailbox.h @@ -11,6 +11,7 @@ #include #include +#include "core/thread_queue.h" /* * Reactor-integrated MPSC mailbox (issue #81): a bounded thread_mpsc_t wired to @@ -87,6 +88,8 @@ thread_cmd_mailbox_t *thread_cmd_mailbox_create(size_t capacity, size_t batch, void thread_cmd_mailbox_free(thread_cmd_mailbox_t *mb); /* Producer side — any thread. Copies *cmd in; false if full. */ bool thread_cmd_mailbox_post(thread_cmd_mailbox_t *mb, const reactor_cmd_t *cmd); +thread_queue_result_t thread_cmd_mailbox_try_post(thread_cmd_mailbox_t *mb, + const reactor_cmd_t *cmd); void thread_cmd_mailbox_keepalive(thread_cmd_mailbox_t *mb, bool enable); size_t thread_cmd_mailbox_count(const thread_cmd_mailbox_t *mb); diff --git a/include/core/thread_queue.h b/include/core/thread_queue.h index 4db3db5e..9854dfff 100644 --- a/include/core/thread_queue.h +++ b/include/core/thread_queue.h @@ -20,9 +20,10 @@ * thread_mpsc_t — many producers, one consumer (moodycamel ConcurrentQueue) * thread_spsc_t — one producer, one consumer (moodycamel ReaderWriterQueue) * - * Both are *bounded*: a capacity is fixed at creation and enqueue fails cleanly - * when full, giving deterministic backpressure with no allocation on the hot - * path (ties into the OOM-firewall philosophy). The bound is enforced by an + * Both are bounded: capacity limits live items, and a full queue refuses the + * offer without taking ownership. The pointer queues use allocation-free + * try_enqueue; command queues may acquire internal producer blocks and + * distinguish allocation failure from FULL. The bound is enforced by an * explicit atomic length counter on top of the underlying queue, which also * drives the empty->non-empty edge used for wakeup (see thread_mailbox.h). * @@ -39,6 +40,9 @@ extern "C" { /* ------------------------------------------------------------------ */ typedef struct thread_mpsc_s thread_mpsc_t; +typedef enum { + THREAD_QUEUE_ACCEPTED, THREAD_QUEUE_FULL, THREAD_QUEUE_ERROR, THREAD_QUEUE_STOPPED +} thread_queue_result_t; /* Create a bounded MPSC queue holding at most `capacity` items. * Returns NULL on allocation failure or capacity == 0. */ @@ -77,7 +81,8 @@ size_t thread_spsc_drain(thread_spsc_t *q, void **items, size_t max); size_t thread_spsc_count(const thread_spsc_t *q); /* MPSC carrying reactor_cmd_t by value — same bounded/lock-free contract as - * the void* MPSC, no per-message malloc. */ + * the void* MPSC. Commands travel by value without an allocated envelope; + * internal producer-block allocation may occur below the logical bound. */ typedef struct reactor_cmd_s reactor_cmd_t; /* core/reactor_cmd.h */ typedef struct thread_cmd_mpsc_s thread_cmd_mpsc_t; @@ -87,6 +92,8 @@ void thread_cmd_mpsc_free(thread_cmd_mpsc_t *q); /* Copies *cmd into the ring. Returns false when full. */ bool thread_cmd_mpsc_enqueue(thread_cmd_mpsc_t *q, const reactor_cmd_t *cmd); +thread_queue_result_t thread_cmd_mpsc_try_enqueue(thread_cmd_mpsc_t *q, + const reactor_cmd_t *cmd); /* Drains up to `max` commands into `out` (an array of `max` reactor_cmd_t). */ size_t thread_cmd_mpsc_drain(thread_cmd_mpsc_t *q, reactor_cmd_t *out, size_t max); size_t thread_cmd_mpsc_count(const thread_cmd_mpsc_t *q); diff --git a/include/core/worker_dispatch.h b/include/core/worker_dispatch.h index 3a13ccd3..1a38681d 100644 --- a/include/core/worker_dispatch.h +++ b/include/core/worker_dispatch.h @@ -43,6 +43,9 @@ typedef bool (*worker_response_sink_fn)(response_wire_t *rw, void *sink_arg); * to response_wire_free — every drop site must go through this so a new * owned field can't leak from a forgotten copy. */ void response_wire_discard(response_wire_t *rw); +/* Consume an inbox request during teardown/dispatch failure, publish its + * failure without trying to enqueue response data or await from a destructor. */ +void worker_dispatch_cancel_request(struct http_request_t *req); /* Take ownership of `req` (a persistent reactor-built or ZMM request, refcount * 1), wrap it in an HttpRequest on THIS (worker) thread, spawn the user handler diff --git a/include/core/worker_inbox.h b/include/core/worker_inbox.h index 9d49b3dd..a543f4c3 100644 --- a/include/core/worker_inbox.h +++ b/include/core/worker_inbox.h @@ -35,6 +35,11 @@ */ typedef struct worker_inbox_s worker_inbox_t; +typedef struct response_delivery_owner_s response_delivery_owner_t; +void worker_inbox_set_delivery_owner(worker_inbox_t *inbox, + response_delivery_owner_t *owner); +response_delivery_owner_t *worker_inbox_delivery_owner(worker_inbox_t *inbox); +void worker_inbox_cancel_pending(worker_inbox_t *inbox); /* Create a worker inbox on THIS (worker) thread. Dispatched requests run their * handler in `scope` (own_scope mirrors worker_dispatch_request) against diff --git a/include/http1/http_parser.h b/include/http1/http_parser.h index 3f346137..5eec99d5 100644 --- a/include/http1/http_parser.h +++ b/include/http1/http_parser.h @@ -27,6 +27,7 @@ /* Forward declaration (full type in http_connection.h) */ typedef struct _http_connection_t http_connection_t; +typedef struct response_delivery_s response_delivery_t; /* Parser-level error codes recorded on http1_parser_t::parse_error so * the connection layer can build an RFC-compliant 4xx response. @@ -185,6 +186,7 @@ struct http_request_t { * returns the slot to its pool. The base http_request_destroy has * already cleaned up every field declared in http_request_t. */ void (*release)(struct http_request_t *req); + response_delivery_t *delivery; /* pool dispatch only, handed to worker ctx */ /* W3C Trace Context. Populated by http_request_parse_trace_context * at headers-complete or dispatch iff server has telemetry enabled and the diff --git a/include/http3/http3_stream.h b/include/http3/http3_stream.h index f778782f..5a00b66d 100644 --- a/include/http3/http3_stream.h +++ b/include/http3/http3_stream.h @@ -17,6 +17,7 @@ #include "Zend/zend_async_API.h" /* zend_coroutine_t, zend_async_trigger_event_t */ #include "zend_smart_str.h" #include "http1/http_parser.h" /* http_request_t */ +#include "core/reactor_pool.h" #include #include @@ -179,6 +180,8 @@ struct _http3_stream_s { /* Lifecycle refcount. Starts at 1 (held by nghttp3 stream_user_data). * Once dispatch fires, the handler coroutine bumps to 2. */ unsigned refcount; + response_delivery_t *delivery; + reactor_control_t request_release_control; /* Flags. Written only by the stream's reactor thread: they share one * storage unit, so a second writer would tear the neighbours. */ diff --git a/include/php_http_server.h b/include/php_http_server.h index 1fd611d8..b7606a57 100644 --- a/include/php_http_server.h +++ b/include/php_http_server.h @@ -811,6 +811,9 @@ struct http_response_stream_ops_t { * below threshold. Returns NULL only on alloc failure — callers * must NULL-check. */ zend_async_event_t *(*get_wait_event)(void *ctx); + /* Optional: whether a first nonblocking offer has handed headers to the + * transport. False permits rolling back an unaccepted first commit. */ + bool (*is_started)(void *ctx); /* Protocol-owned static-file body delivery. * @@ -922,7 +925,7 @@ typedef struct http_server_counters_s { /* Final response status class, bumped exactly once per served request * alongside total_requests (see http_server_count_request). Every - * request lands in one bucket, so the four sum to total_requests. */ + * request lands in one status/no-status bucket; the five sum to total_requests. */ uint64_t responses_2xx_total; uint64_t responses_3xx_total; uint64_t responses_4xx_total; @@ -932,6 +935,9 @@ typedef struct http_server_counters_s { * Overlaps the four buckets rather than joining them: the status counted * above is the one the peer was told. */ uint64_t responses_aborted_total; + /* A pooled request ended before any HTTP status was confirmed. The five + * disjoint status/no-status buckets reconcile with total_requests. */ + uint64_t responses_undelivered_total; /* Active-connection gauge split by negotiated protocol (++ once the * protocol is detected / on H3 conn open, -- at conn close). Sum of @@ -1073,6 +1079,7 @@ typedef struct http_server_counters_s { X(tls_bytes_ciphertext_out_total, SUM) \ X(total_requests, SUM) \ X(responses_2xx_total, SUM) \ + X(responses_undelivered_total, SUM) \ X(responses_3xx_total, SUM) \ X(responses_4xx_total, SUM) \ X(responses_5xx_total, SUM) \ diff --git a/src/compression/http_compression_response.c b/src/compression/http_compression_response.c index e3a01fb3..c3a84bd2 100644 --- a/src/compression/http_compression_response.c +++ b/src/compression/http_compression_response.c @@ -800,6 +800,13 @@ static bool ws_is_alive(void *ctx_opaque) || w->underlying_ops->is_alive(w->underlying_ctx)); } +static bool ws_is_started(void *ctx) +{ + ws_ctx_t *w = ctx; + return w->underlying_ops->is_started == NULL + || w->underlying_ops->is_started(w->underlying_ctx); +} + static const http_response_stream_ops_t compressing_stream_ops = { .append_chunk = ws_append_chunk, .sendable = ws_sendable, @@ -808,6 +815,7 @@ static const http_response_stream_ops_t compressing_stream_ops = { .mark_ended = ws_mark_ended, .abort = ws_abort, .get_wait_event = ws_get_wait_event, + .is_started = ws_is_started, }; void http_compression_maybe_install_stream_wrapper(zend_object *response_obj) @@ -854,3 +862,17 @@ void http_compression_maybe_install_stream_wrapper(zend_object *response_obj) st->wrapper_ctx = w; st->wrapper_installed = true; } + +void http_compression_reset_unstarted(zend_object *obj) +{ + http_compression_state_t *st = state_of(obj); + if (st == NULL || !st->wrapper_installed) return; + ws_ctx_t *w = st->wrapper_ctx; + ZEND_ASSERT(!w->first_chunk_done); + http_response_replace_stream_ops(obj, w->underlying_ops, w->underlying_ctx); + if (st->encoder != NULL) http_compression_pool_release(st->encoder); + st->encoder = NULL; + st->wrapper_ctx = NULL; + st->wrapper_installed = false; + efree(w); +} diff --git a/src/core/reactor_pool.c b/src/core/reactor_pool.c index e5441978..2587b13a 100644 --- a/src/core/reactor_pool.c +++ b/src/core/reactor_pool.c @@ -16,6 +16,8 @@ #include "Zend/zend_async_API.h" #include "Zend/zend_atomic.h" #include "core/reactor_pool.h" +#include "core/atomic_pointer.h" +#include "fiu-local.h" #include "core/reactor_cmd.h" #include "core/thread_mailbox.h" @@ -47,8 +49,39 @@ typedef struct { zend_atomic_int phase; thread_cmd_mailbox_t *mailbox; zend_atomic_int64 processed; + http_atomic_pointer_t controls; + zend_async_trigger_event_t *control_wake; + reactor_capacity_gate_t *capacity_gates; bool stopping; } reactor_ctx_t; +static void reactor_epilogue(void); + +static void reactor_control_drain(reactor_ctx_t *rc) +{ + reactor_control_t *head = http_atomic_pointer_exchange(&rc->controls, NULL); + reactor_control_t *ordered = NULL; + while (head != NULL) { + reactor_control_t *next = head->next; + head->next = ordered; + ordered = head; + head = next; + } + while (ordered != NULL) { + reactor_control_t *node = ordered; + ordered = node->next; + node->fn(node->arg); /* callback may free the node */ + } +} + +static void reactor_control_signal(zend_async_event_t *event, + zend_async_event_callback_t *callback, + void *result, zend_object *exception) +{ + (void)callback; (void)result; (void)exception; + reactor_ctx_t *rc = *(reactor_ctx_t **)((char *)event + event->extra_offset); + reactor_control_drain(rc); + reactor_epilogue(); /* resets must flush even when no data post follows */ +} struct reactor_pool_s { zend_async_thread_pool_t *tp; @@ -61,6 +94,11 @@ struct reactor_pool_s { * before reactors run, read on each reactor thread at drain-batch end. */ static void (*g_drain_epilogue)(void) = NULL; +static void reactor_epilogue(void) +{ + if (g_drain_epilogue != NULL) g_drain_epilogue(); +} + void reactor_pool_set_drain_epilogue(void (*fn)(void)) { g_drain_epilogue = fn; @@ -84,6 +122,12 @@ static void reactor_drain(reactor_cmd_t *items, const size_t count, void *arg) reactor_ctx_t *const rc = (reactor_ctx_t *)arg; int64_t drained = 0; + /* dequeue already freed the ring slots; every subscribed worker may retry. + * Wakes are bounded by the number of registered worker/ reactor pairs. */ + for (reactor_capacity_gate_t *g = rc->capacity_gates; g != NULL; g = g->next) { + g->wake(g->arg); + } + for (size_t i = 0; i < count; i++) { reactor_cmd_t *const cmd = &items[i]; @@ -145,6 +189,29 @@ static void reactor_loop_handler(zend_async_event_t *event, void *vctx) thread_cmd_mailbox_keepalive(mb, true); rc->mailbox = mb; /* publish (plain) */ + rc->control_wake = ZEND_ASYNC_NEW_TRIGGER_EVENT_EX(sizeof(reactor_ctx_t *)); + if (rc->control_wake == NULL) { + thread_cmd_mailbox_free(mb); + rc->mailbox = NULL; + zend_atomic_int_store_ex(&rc->phase, REACTOR_PHASE_DONE); + return; + } + *(reactor_ctx_t **)((char *)&rc->control_wake->base + + rc->control_wake->base.extra_offset) = rc; + zend_async_event_callback_t *control_cb = + ZEND_ASYNC_EVENT_CALLBACK(reactor_control_signal); + if (control_cb == NULL + || !rc->control_wake->base.add_callback(&rc->control_wake->base, control_cb)) { + if (control_cb != NULL) { + ZEND_ASYNC_EVENT_CALLBACK_RELEASE(control_cb); + } + rc->control_wake->base.dispose(&rc->control_wake->base); + rc->control_wake = NULL; + thread_cmd_mailbox_free(mb); + rc->mailbox = NULL; + zend_atomic_int_store_ex(&rc->phase, REACTOR_PHASE_DONE); + return; + } zend_atomic_int_store_ex(&rc->phase, REACTOR_PHASE_RUN); /* release */ while (!rc->stopping) { @@ -152,6 +219,9 @@ static void reactor_loop_handler(zend_async_event_t *event, void *vctx) } thread_cmd_mailbox_keepalive(mb, false); + reactor_control_drain(rc); + rc->control_wake->base.dispose(&rc->control_wake->base); + rc->control_wake = NULL; thread_cmd_mailbox_free(mb); /* consumer-thread */ rc->mailbox = NULL; @@ -198,6 +268,7 @@ reactor_pool_t *reactor_pool_create(const int reactors, const size_t mailbox_cap rp->ctx[i].stopping = false; ZEND_ATOMIC_INT_INIT(&rp->ctx[i].phase, REACTOR_PHASE_SPAWN); ZEND_ATOMIC_INT64_INIT(&rp->ctx[i].processed, 0); + ZEND_ATOMIC_INT64_INIT(&rp->ctx[i].controls, 0); zend_async_event_t *const evt = tp->submit_internal(tp, reactor_loop_handler, &rp->ctx[i]); @@ -302,35 +373,118 @@ bool reactor_pool_exec(reactor_pool_t *rp, const int idx, const reactor_exec_fn return true; } -bool reactor_pool_post_exec(reactor_pool_t *rp, const int idx, - const reactor_exec_fn fn, void *arg) +bool reactor_pool_post_exec(reactor_pool_t *rp, int idx, reactor_exec_fn fn, void *arg) { - if (UNEXPECTED(rp == NULL || idx < 0 || idx >= rp->count || fn == NULL)) { + return reactor_pool_try_post_exec(rp, idx, fn, arg) == THREAD_QUEUE_ACCEPTED; +} +thread_queue_result_t reactor_pool_try_post_exec(reactor_pool_t *rp, int idx, + reactor_exec_fn fn, void *arg) +{ + if (!reactor_pool_is_running(rp, idx) || fn == NULL) return THREAD_QUEUE_STOPPED; + /* Fault seam: force FULL and model a consumer drain with a NOOP. The + * notifier still runs on the reactor, exercising subscribe/recheck/wake. */ + fiu_do_on("h3/reverse/queue_full", { + (void)reactor_pool_post(rp, idx, NULL); + return THREAD_QUEUE_FULL; + }); + reactor_ctx_t *rc = &rp->ctx[idx]; + reactor_cmd_t cmd = {0}; + cmd.kind = REACTOR_CMD_POST; cmd.fn = fn; cmd.arg = arg; + return thread_cmd_mailbox_try_post(rc->mailbox, &cmd); +} + +bool reactor_pool_is_running(const reactor_pool_t *rp, const int idx) +{ + if (UNEXPECTED(rp == NULL || idx < 0 || idx >= rp->count)) { return false; } - reactor_ctx_t *const rc = &rp->ctx[idx]; + return zend_atomic_int_load_ex(&rp->ctx[idx].phase) == REACTOR_PHASE_RUN; +} - if (UNEXPECTED(zend_atomic_int_load_ex(&rc->phase) != REACTOR_PHASE_RUN)) { +bool reactor_pool_has_capacity(const reactor_pool_t *rp, int idx) +{ + return reactor_pool_is_running(rp, idx) + && thread_cmd_mailbox_count(rp->ctx[idx].mailbox) < rp->mailbox_capacity; +} + +bool reactor_pool_post_control(reactor_pool_t *rp, const int idx, + reactor_control_t *node) +{ + if (!reactor_pool_is_running(rp, idx) || node == NULL || node->fn == NULL) { return false; } + reactor_ctx_t *rc = &rp->ctx[idx]; + void *head = http_atomic_pointer_load(&rc->controls); + do { + node->next = head; + } while (!http_atomic_pointer_compare_exchange(&rc->controls, &head, node)); + rc->control_wake->trigger(rc->control_wake); + return true; +} - /* Fire-and-forget: the value rides the ring, no `done` ack, nothing to free. */ - reactor_cmd_t cmd = {0}; - cmd.kind = REACTOR_CMD_POST; - cmd.fn = fn; - cmd.arg = arg; +typedef struct { + reactor_ctx_t *rc; + reactor_capacity_gate_t *gate; +} capacity_registration_t; - return thread_cmd_mailbox_post(rc->mailbox, &cmd); +typedef struct { + reactor_control_t node; + reactor_exec_fn fn; + void *arg; + zend_atomic_int done; +} control_fence_t; + +static void control_fence_apply(void *arg) +{ + control_fence_t *f = arg; + f->fn(f->arg); + zend_atomic_int_store_ex(&f->done, 1); } -bool reactor_pool_is_running(const reactor_pool_t *rp, const int idx) +bool reactor_pool_control_exec(reactor_pool_t *rp, int idx, + reactor_exec_fn fn, void *arg) { - if (UNEXPECTED(rp == NULL || idx < 0 || idx >= rp->count)) { - return false; - } + control_fence_t f = {0}; + f.fn = fn; f.arg = arg; + f.node.fn = control_fence_apply; f.node.arg = &f; + ZEND_ATOMIC_INT_INIT(&f.done, 0); + if (!reactor_pool_post_control(rp, idx, &f.node)) return false; + /* A posted stack node cannot be abandoned on cancellation. This cold + * fence waits for its acknowledgement, not for data-mailbox capacity. */ + while (zend_atomic_int_load_ex(&f.done) == 0) reactor_pool_msleep(); + return true; +} - return zend_atomic_int_load_ex(&rp->ctx[idx].phase) == REACTOR_PHASE_RUN; +static void capacity_register(void *arg) +{ + capacity_registration_t *r = arg; + r->gate->next = r->rc->capacity_gates; + r->rc->capacity_gates = r->gate; +} + +static void capacity_unregister(void *arg) +{ + capacity_registration_t *r = arg; + reactor_capacity_gate_t **p = &r->rc->capacity_gates; + while (*p != NULL && *p != r->gate) p = &(*p)->next; + if (*p != NULL) *p = r->gate->next; +} + +bool reactor_pool_capacity_register(reactor_pool_t *rp, int idx, + reactor_capacity_gate_t *gate) +{ + if (!reactor_pool_is_running(rp, idx) || gate == NULL) return false; + capacity_registration_t r = { &rp->ctx[idx], gate }; + return reactor_pool_control_exec(rp, idx, capacity_register, &r); +} + +bool reactor_pool_capacity_unregister(reactor_pool_t *rp, int idx, + reactor_capacity_gate_t *gate) +{ + if (!reactor_pool_is_running(rp, idx)) return true; + capacity_registration_t r = { &rp->ctx[idx], gate }; + return reactor_pool_control_exec(rp, idx, capacity_unregister, &r); } uint64_t reactor_pool_processed(const reactor_pool_t *rp, const int idx) diff --git a/src/core/response_delivery.c b/src/core/response_delivery.c new file mode 100644 index 00000000..5e0ba592 --- /dev/null +++ b/src/core/response_delivery.c @@ -0,0 +1,852 @@ +/* Copyright (c) TrueAsync. Licensed under the Apache License, Version 2.0. */ +#ifdef HAVE_CONFIG_H +#include +#endif +#include "core/response_delivery.h" +#include "core/response_wire.h" +#include "core/atomic_pointer.h" +#include "core/bailout_guard.h" +#include "core/worker_dispatch.h" +#include "core/async_plain_event.h" +#include "php_http_server.h" +#include "http1/http_parser.h" +#include "log/http_log.h" +#include "zend_exceptions.h" +#include "Zend/zend_hrtime.h" +#include "fiu-local.h" +#include +#include + +enum +{ + DELIVERY_PENDING, + DELIVERY_CANCEL_REQUESTED, + DELIVERY_OK, + DELIVERY_ERROR +}; +enum +{ + SNAPSHOT_READY = 1, + TERMINAL_READY = 2 +}; + +typedef struct delivery_waiter_s +{ + struct delivery_waiter_s *next; + struct delivery_waiter_s *prev; + zend_async_event_t *event; + response_delivery_t *delivery; + volatile bool detached; +} delivery_waiter_t; + +typedef struct +{ + reactor_capacity_gate_t gate; + response_delivery_owner_t *owner; + zend_atomic_bool waiting; + delivery_waiter_t *waiters; /* worker-only: reactor signals the gate */ + delivery_waiter_t *tail; +} delivery_gate_t; + +struct response_delivery_owner_s +{ + void *server; + reactor_pool_t *pool; + zend_async_trigger_event_t *wake; + http_atomic_pointer_t completed; + zend_atomic_int outstanding; + delivery_gate_t *gates; + int reactors; + response_delivery_t *active; /* worker-only */ + bool closing; + unsigned waiters; +}; + +struct response_delivery_s +{ + zend_atomic_int refs; + zend_atomic_int state; + zend_atomic_int ready; + zend_atomic_int wires; + zend_atomic_int dropped_wires; + zend_atomic_bool published; + response_delivery_owner_t *owner; + reactor_pool_t *pool; /* remains valid through producer quiescence */ + int reactor; + void *stream; /* reactor-only */ + reactor_exec_fn reset; + reactor_exec_fn apply; + reactor_exec_fn release_request; + reactor_control_t cancel_node; + reactor_control_t consumed_node; + response_delivery_t *completed_next; + response_delivery_t *active_next; + response_delivery_t **active_link; + bool request_consumed; /* reactor-only */ + bool request_released; + int status; /* reactor-only: accepted by HTTP/3 */ + int submitted_status; + uint64_t acked_body; + const char *reason; /* fixed string, no allocation on cancellation */ + const char *cancel_reason; /* published by CAS state */ + http_access_rec_t access; /* worker writes before SNAPSHOT_READY */ + char *method, *path, *query, *address; + uint8_t trace_id[16], span_id[8]; + bool report; + bool worker_begun; + response_wire_t *owned_wire; /* worker-only, including bailout recovery */ + bool owned_ready; +}; + +static int delivery_ready(response_delivery_t *d, int bit) +{ + int old = zend_atomic_int_load_ex(&d->ready); + while (!zend_atomic_int_compare_exchange_ex(&d->ready, &old, old | bit)) { + } + return old; +} + +static void delivery_publish(response_delivery_t *d, int bit) +{ + const int old = delivery_ready(d, bit); + if ((old | bit) != (SNAPSHOT_READY | TERMINAL_READY) || + zend_atomic_int_load_ex(&d->wires) != 0 || + zend_atomic_bool_exchange_ex(&d->published, true)) { + return; + } + response_delivery_owner_t *o = d->owner; + void *head = http_atomic_pointer_load(&o->completed); + do { + d->completed_next = head; + } while (!http_atomic_pointer_compare_exchange(&o->completed, &head, d)); + o->wake->trigger(o->wake); +} + +void response_delivery_addref(response_delivery_t *d) +{ + if (d != NULL) { + zend_atomic_int_inc(&d->refs); + } +} + +void response_delivery_release(response_delivery_t *d) +{ + if (d == NULL || zend_atomic_int_fetch_sub(&d->refs, 1) != 1) { + return; + } + free(d->method); + free(d->path); + free(d->query); + free(d->address); + free(d); +} + +static void owner_drain(response_delivery_owner_t *o) +{ + response_delivery_t *head = http_atomic_pointer_exchange(&o->completed, NULL); + while (head != NULL) { + response_delivery_t *d = head; + head = d->completed_next; + http_server_object *server = o->server; + http_server_counters_t *c = http_server_counters(server); + if (d->report) { + c->worker_wire_dropped_total += (uint64_t)zend_atomic_int_load_ex(&d->dropped_wires); + const bool failed = zend_atomic_int_load_ex(&d->state) == DELIVERY_ERROR; + if (d->status != 0) { + http_server_count_request(c, d->status); + if (failed) { + c->responses_aborted_total++; + } + } else { + c->total_requests++; + c->responses_undelivered_total++; + } + d->access.status = d->status; + d->access.response_size = d->acked_body; + d->access.error_type = + failed ? (d->status != 0 ? "response_aborted" : "response_undelivered") : NULL; + http_log_emit_access(http_server_get_log_state(server), &d->access); + } + *d->active_link = d->active_next; + if (d->active_next != NULL) { + d->active_next->active_link = d->active_link; + } + zend_atomic_int_dec(&o->outstanding); + d->owner = NULL; /* later request reclamation does not retain a logger */ + response_delivery_release(d); /* worker's pending-completion ref */ + } +} + +static void owner_signal(zend_async_event_t *event, zend_async_event_callback_t *callback, + void *result, zend_object *exception) +{ + (void)callback; + (void)result; + (void)exception; + response_delivery_owner_t *o = + *(response_delivery_owner_t **)((char *)event + event->extra_offset); + for (int i = 0; i < o->reactors; i++) { + for (delivery_waiter_t *w = o->gates[i].waiters; w != NULL; w = w->next) { + if (w == o->gates[i].waiters || o->closing || + zend_atomic_int_load_ex(&w->delivery->state) != DELIVERY_PENDING) { + async_plain_event_fire(w->event); + } + } + } + owner_drain(o); +} + +static void capacity_wake(void *arg) +{ + delivery_gate_t *g = arg; + if (zend_atomic_bool_load_ex(&g->waiting)) { + g->owner->wake->trigger(g->owner->wake); + } +} + +static void gate_add(delivery_gate_t *g, delivery_waiter_t *w) +{ + w->prev = g->tail; + w->next = NULL; + if (g->tail != NULL) { + g->tail->next = w; + } else { + g->waiters = w; + } + g->tail = w; + g->owner->waiters++; + zend_atomic_bool_store_ex(&g->waiting, true); +} + +static void gate_remove(delivery_gate_t *g, delivery_waiter_t *w, int reactor) +{ + if (w->prev != NULL) { + w->prev->next = w->next; + } else { + g->waiters = w->next; + } + if (w->next != NULL) { + w->next->prev = w->prev; + } else { + g->tail = w->prev; + } + g->owner->waiters--; + zend_atomic_bool_store_ex(&g->waiting, g->waiters != NULL); + /* An awakened sender may cancel instead of taking the available slot. */ + if (g->waiters != NULL && + (g->owner->closing || + zend_atomic_int_load_ex(&g->waiters->delivery->state) != DELIVERY_PENDING || + reactor_pool_has_capacity(g->owner->pool, reactor))) { + async_plain_event_fire(g->waiters->event); + } + if (g->owner->closing) { + g->owner->wake->trigger(g->owner->wake); + } +} + +/* Prepare every fallible resource before publishing a stack waiter. A local + * bailout firewall retracts subscriptions before the C frame disappears. */ +static bool wait_prepare(delivery_waiter_t *w, zend_coroutine_t *co, uint64_t deadline) +{ + volatile bool ok = false; + volatile bool bailout = false; + zend_async_timer_event_t *volatile timer = NULL; + volatile bool timer_subscribed = false; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try + { + w->event = async_plain_event_new(); + if (w->event != NULL && ZEND_ASYNC_WAKER_NEW(co) != NULL) { + zend_async_resume_when(co, w->event, false, zend_async_waker_callback_resolve, NULL); + if (deadline != 0) { + uint64_t now = zend_hrtime(); + timer = ZEND_ASYNC_NEW_TIMER_EVENT( + now < deadline ? (deadline - now + 999999) / 1000000 : 1, false); + if (timer != NULL) { + zend_async_resume_when(co, &timer->base, true, + zend_async_waker_callback_timeout, NULL); + timer_subscribed = true; + ok = true; + } + } else { + ok = true; + } + } + } + zend_catch + { + http_bailout_state_restore(&state); + bailout = true; + } + zend_end_try(); + if (!ok) { + ZEND_ASYNC_WAKER_DESTROY(co); + if (timer != NULL && !timer_subscribed) { + timer->base.dispose(&timer->base); + } + if (w->event != NULL) { + w->event->dispose(w->event); + } + w->event = NULL; + } + if (bailout) { + zend_bailout(); + } + return ok; +} + +static bool wait_finish(delivery_gate_t *g, delivery_waiter_t *w, zend_coroutine_t *co, int reactor, + bool suspend) +{ + volatile bool bailout = false; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try + { + if (suspend) { + fiu_do_on("h3/delivery/wait_bailout", { zend_bailout(); }); + ZEND_ASYNC_SUSPEND(); + } + } + zend_catch + { + http_bailout_state_restore(&state); + bailout = true; + } + zend_end_try(); + ZEND_ASYNC_WAKER_DESTROY(co); + bool attached = !w->detached; + if (attached) { + gate_remove(g, w, reactor); + } + w->event->dispose(w->event); + if (bailout) { + zend_bailout(); + } + return attached; +} + +response_delivery_owner_t *response_delivery_owner_create(void *server, reactor_pool_t *pool) +{ + if (server == NULL || pool == NULL) { + return NULL; + } + response_delivery_owner_t *o = calloc(1, sizeof(*o)); + if (o == NULL) { + return NULL; + } + o->server = server; + o->pool = pool; + o->reactors = reactor_pool_count(pool); + ZEND_ATOMIC_INT64_INIT(&o->completed, 0); + ZEND_ATOMIC_INT_INIT(&o->outstanding, 0); + o->gates = calloc((size_t)o->reactors, sizeof(*o->gates)); + o->wake = ZEND_ASYNC_NEW_TRIGGER_EVENT_EX(sizeof(response_delivery_owner_t *)); + if (o->gates == NULL || o->wake == NULL) { + goto fail; + } + *(response_delivery_owner_t **)((char *)&o->wake->base + o->wake->base.extra_offset) = o; + zend_async_event_callback_t *cb = ZEND_ASYNC_EVENT_CALLBACK(owner_signal); + if (cb == NULL || !o->wake->base.add_callback(&o->wake->base, cb)) { + if (cb != NULL) { + ZEND_ASYNC_EVENT_CALLBACK_RELEASE(cb); + } + goto fail; + } + int registered = 0; + for (int i = 0; i < o->reactors; i++) { + delivery_gate_t *g = &o->gates[i]; + g->owner = o; + g->gate.wake = capacity_wake; + g->gate.arg = g; + ZEND_ATOMIC_BOOL_INIT(&g->waiting, false); + if (!reactor_pool_capacity_register(pool, i, &g->gate)) { + for (int j = 0; j < registered; j++) { + reactor_pool_capacity_unregister(pool, j, &o->gates[j].gate); + } + goto fail; + } + registered++; + } + return o; +fail: + if (o->wake != NULL) { + o->wake->base.dispose(&o->wake->base); + } + free(o->gates); + free(o); + return NULL; +} + +static void cancel_apply(void *arg) +{ + response_delivery_t *d = arg; + response_delivery_finish(d, false, d->cancel_reason); + if (d->stream != NULL && d->reset != NULL) { + d->reset(d->stream); + } + response_delivery_release(d); /* control command */ +} + +static void consumed_apply(void *arg) +{ + response_delivery_t *d = arg; + d->request_consumed = true; + response_delivery_maybe_release_request(d); + response_delivery_release(d); +} + +response_delivery_t *response_delivery_create(response_delivery_owner_t *o, int reactor, + void *stream, reactor_exec_fn reset, + reactor_exec_fn release_request, + reactor_exec_fn apply) +{ + response_delivery_t *d = calloc(1, sizeof(*d)); + if (d == NULL) { + return NULL; + } + d->owner = o; + d->pool = o->pool; + d->reactor = reactor; + d->stream = stream; + d->reset = reset; + d->release_request = release_request; + d->apply = apply; + d->cancel_node.fn = cancel_apply; + d->cancel_node.arg = d; + d->consumed_node.fn = consumed_apply; + d->consumed_node.arg = d; + ZEND_ATOMIC_INT_INIT(&d->refs, 2); /* reactor stream + worker completion */ + ZEND_ATOMIC_INT_INIT(&d->state, DELIVERY_PENDING); + ZEND_ATOMIC_INT_INIT(&d->ready, 0); + ZEND_ATOMIC_INT_INIT(&d->wires, 0); + ZEND_ATOMIC_INT_INIT(&d->dropped_wires, 0); + ZEND_ATOMIC_BOOL_INIT(&d->published, false); + zend_atomic_int_inc(&o->outstanding); + return d; +} + +void response_delivery_abandon(response_delivery_t *d) +{ + zend_atomic_int_dec(&d->owner->outstanding); + response_delivery_release(d); + response_delivery_release(d); +} + + +static char *copy_string(const char *s) +{ + if (s == NULL) { + return NULL; + } + size_t n = strlen(s) + 1; + char *p = malloc(n); + if (p != NULL) { + memcpy(p, s, n); + } + return p; +} + +static void update_string(char **copy, const char *value) +{ + if (*copy != NULL && value != NULL && strcmp(*copy, value) == 0) { + return; + } + char *next = copy_string(value); + free(*copy); + *copy = next; +} + +static void delivery_snapshot(response_delivery_t *d, const http_request_t *req, + zend_object *response, bool report) +{ + char ip[INET6_ADDRSTRLEN]; + http_request_fill_access_rec(req, response, &d->access, ip, sizeof(ip)); + /* Request identity is immutable. Reuse its early snapshot; only tracing + * and service stamps become available after the handler has run. */ + update_string(&d->method, d->access.method); + update_string(&d->path, d->access.url_path); + update_string(&d->query, d->access.url_query); + update_string(&d->address, d->access.client_address); + d->access.method = d->method; + d->access.url_path = d->path; + d->access.url_query = d->query; + d->access.client_address = d->address; + if (req != NULL && req->has_trace) { + memcpy(d->trace_id, req->trace_id, sizeof(d->trace_id)); + memcpy(d->span_id, req->span_id, sizeof(d->span_id)); + d->access.trace_id = d->trace_id; + d->access.span_id = d->span_id; + } + d->report = report; +} + +void response_delivery_worker_begin(response_delivery_t *d, const http_request_t *req) +{ + if (d->worker_begun) { + return; + } + d->worker_begun = true; + d->active_next = d->owner->active; + d->active_link = &d->owner->active; + if (d->active_next != NULL) { + d->active_next->active_link = &d->active_next; + } + d->owner->active = d; + /* Teardown can finish without resuming a cancelled handler. Preserve + * immutable request identity while the fields still belong to the worker. */ + delivery_snapshot(d, req, NULL, true); +} + +void response_delivery_sender_abandoned(response_delivery_t *d) +{ + response_delivery_cancel(d, "dispatch_failed"); + delivery_publish(d, SNAPSHOT_READY); /* early identity snapshot is intact */ +} + +void response_delivery_sender_done(response_delivery_t *d, const http_request_t *req, + zend_object *response, bool report) +{ + if ((zend_atomic_int_load_ex(&d->ready) & SNAPSHOT_READY) != 0) { + return; + } + delivery_snapshot(d, req, response, report); + delivery_publish(d, SNAPSHOT_READY); +} + +bool response_delivery_failed(const response_delivery_t *d) +{ + int state = zend_atomic_int_load_ex(&((response_delivery_t *)d)->state); + return state == DELIVERY_CANCEL_REQUESTED || state == DELIVERY_ERROR; +} + +void response_delivery_cancel(response_delivery_t *d, const char *reason) +{ + int expected = DELIVERY_PENDING; + if (zend_atomic_int_load_ex(&d->state) != DELIVERY_PENDING) { + return; + } + /* Only the worker calls cancel: publish the bounded reason before state. */ + d->cancel_reason = reason; + if (!zend_atomic_int_compare_exchange_ex(&d->state, &expected, DELIVERY_CANCEL_REQUESTED)) { + return; + } + response_delivery_addref(d); + if (!reactor_pool_post_control(d->pool, d->reactor, &d->cancel_node)) { + /* A stopped reactor has no writer left. It cannot deliver a response. */ + response_delivery_finish(d, false, reason); + response_delivery_release(d); + } +} + +bool response_delivery_post(response_delivery_t *d, response_wire_t *wire, uint32_t timeout_ms, + bool nonblocking) +{ + if (d->owned_wire == NULL) { + response_delivery_own_wire(d, wire); + } + ZEND_ASSERT(d->owned_wire == wire); + d->owned_ready = true; + response_delivery_owner_t *o = d->owner; + if (o == NULL) { + response_delivery_discard_owned(d); + return false; + } + delivery_gate_t *g = &o->gates[d->reactor]; + zend_coroutine_t *co = ZEND_ASYNC_CURRENT_COROUTINE; + const uint64_t deadline = timeout_ms != 0 ? zend_hrtime() + (uint64_t)timeout_ms * 1000000 : 0; + for (;;) { + if (response_delivery_failed(d) || o->closing || + !reactor_pool_is_running(o->pool, d->reactor)) { + break; + } + thread_queue_result_t result = + reactor_pool_try_post_exec(o->pool, d->reactor, d->apply, wire); + if (result == THREAD_QUEUE_ACCEPTED) { + d->owned_wire = NULL; + return true; + } + if (result != THREAD_QUEUE_FULL) { + break; + } + if (nonblocking) { + d->owned_ready = false; /* retryable offer, not a mandatory lost wire */ + response_delivery_discard_owned(d); + return false; /* retryable offer, not a lost mandatory fragment */ + } + if (co == NULL || ZEND_ASYNC_IS_SCHEDULER_CONTEXT) { + break; + } + if (deadline != 0 && zend_hrtime() >= deadline) { + break; + } + + delivery_waiter_t w = {.delivery = d}; + if (!wait_prepare(&w, co, deadline)) { + break; + } + gate_add(g, &w); + /* Subscription is live before the second enqueue: a wake before + * SUSPEND resolves its waker instead of disappearing. */ + result = !response_delivery_failed(d) && !o->closing + ? reactor_pool_try_post_exec(o->pool, d->reactor, d->apply, wire) + : THREAD_QUEUE_STOPPED; + bool posted = result == THREAD_QUEUE_ACCEPTED; + if (!wait_finish(g, &w, co, d->reactor, + result == THREAD_QUEUE_FULL && + reactor_pool_is_running(o->pool, d->reactor) && + !response_delivery_failed(d))) { + return false; + } + if (posted) { + d->owned_wire = NULL; + return true; + } + if (result != THREAD_QUEUE_FULL) { + break; + } + if (EG(exception) != NULL) { + break; + } + } + response_delivery_cancel(d, "reverse_post_failed"); + response_delivery_discard_owned(d); + return false; +} + +void response_delivery_started(response_delivery_t *d, int status) +{ + if (d != NULL && zend_atomic_int_load_ex(&d->state) < DELIVERY_OK) { + d->submitted_status = status > 0 ? status : 200; + } +} + +bool response_delivery_sendable(response_delivery_t *d) +{ + return d != NULL && d->owner != NULL && !d->owner->closing && + zend_atomic_int_load_ex(&d->state) == DELIVERY_PENDING && + reactor_pool_has_capacity(d->pool, d->reactor); +} + +bool response_delivery_wait_writable(response_delivery_t *d, uint32_t timeout_ms) +{ + if (response_delivery_sendable(d)) { + return true; + } + if (d->owner == NULL) { + return false; + } + response_delivery_owner_t *o = d->owner; + delivery_gate_t *g = &o->gates[d->reactor]; + zend_coroutine_t *co = ZEND_ASYNC_CURRENT_COROUTINE; + if (co == NULL || ZEND_ASYNC_IS_SCHEDULER_CONTEXT) { + return false; + } + const uint64_t deadline = timeout_ms != 0 ? zend_hrtime() + (uint64_t)timeout_ms * 1000000 : 0; + while (!response_delivery_sendable(d)) { + if (zend_atomic_int_load_ex(&d->state) != DELIVERY_PENDING || o->closing || + !reactor_pool_is_running(d->pool, d->reactor)) { + return false; + } + if (deadline != 0 && zend_hrtime() >= deadline) { + return false; + } + delivery_waiter_t w = {.delivery = d}; + if (!wait_prepare(&w, co, deadline)) { + return false; + } + gate_add(g, &w); + if (!wait_finish(g, &w, co, d->reactor, !response_delivery_sendable(d))) { + return false; + } + if (EG(exception) != NULL) { + return false; + } + } + return true; +} + +void response_delivery_ack_transport(response_delivery_t *d, uint64_t bytes) +{ + if (d != NULL && bytes != 0 && zend_atomic_int_load_ex(&d->state) < DELIVERY_OK) { + d->status = d->submitted_status; + } +} + +void response_delivery_ack(response_delivery_t *d, uint64_t bytes) +{ + if (d != NULL && zend_atomic_int_load_ex(&d->state) < DELIVERY_OK) { + d->acked_body += bytes; + } +} + +void response_delivery_finish(response_delivery_t *d, bool clean, const char *reason) +{ + if (d == NULL) { + return; + } + int state = zend_atomic_int_load_ex(&d->state); + for (;;) { + if (state == DELIVERY_OK || state == DELIVERY_ERROR) { + return; + } + if (clean) { + d->status = d->submitted_status; /* clean close confirms headers too */ + } + int desired = + clean && state == DELIVERY_PENDING && d->status != 0 ? DELIVERY_OK : DELIVERY_ERROR; + if (zend_atomic_int_compare_exchange_ex(&d->state, &state, desired)) { + break; + } + } + d->reason = reason; + response_delivery_owner_t *o = d->owner; + delivery_publish(d, TERMINAL_READY); + o->wake->trigger(o->wake); /* wake capacity waiters on peer close */ +} + +void response_delivery_wire_acquire(response_delivery_t *d) +{ + response_delivery_addref(d); + zend_atomic_int_inc(&d->wires); +} + +void response_delivery_own_wire(response_delivery_t *d, response_wire_t *wire) +{ + ZEND_ASSERT(d->owned_wire == NULL); + d->owned_wire = wire; + d->owned_ready = false; + if (response_wire_delivery(wire) == NULL) { + response_wire_set_delivery(wire, d); + } +} +void response_delivery_discard_owned(response_delivery_t *d) +{ + response_wire_t *wire = d->owned_wire; + d->owned_wire = NULL; + if (wire != NULL) { + if (d->owned_ready) { + response_delivery_note_drop(d); + } + response_wire_discard(wire); + } +} +void response_delivery_wire_release(response_delivery_t *d) +{ + if (zend_atomic_int_fetch_sub(&d->wires, 1) == 1) { + delivery_publish(d, 0); + } + response_delivery_release(d); +} + +void response_delivery_request_consumed(response_delivery_t *d) +{ + response_delivery_addref(d); + if (!reactor_pool_post_control(d->pool, d->reactor, &d->consumed_node)) { + response_delivery_release(d); + } +} + +void response_delivery_maybe_release_request(response_delivery_t *d) +{ + if (d->stream != NULL && d->request_consumed && !d->request_released && + zend_atomic_int_load_ex(&d->wires) == 0) { + d->request_released = true; + d->release_request(d->stream); + } +} + +void response_delivery_stream_gone(response_delivery_t *d) +{ + if (d == NULL) { + return; + } + response_delivery_finish(d, false, "connection_closed"); + d->stream = NULL; + response_delivery_release(d); /* reactor stream */ +} + +void response_delivery_owner_close(response_delivery_owner_t *o) +{ + if (o == NULL || o->closing) { + return; + } + o->closing = true; + zend_coroutine_t *co = ZEND_ASYNC_CURRENT_COROUTINE; + const bool can_suspend = co != NULL && !ZEND_ASYNC_IS_SCHEDULER_CONTEXT; + for (response_delivery_t *d = o->active; d != NULL; d = d->active_next) { + response_delivery_cancel(d, "worker_stopped"); + if (!can_suspend) { + response_delivery_discard_owned(d); + delivery_publish(d, SNAPSHOT_READY); + } + } + if (!can_suspend) { + /* No PHP coroutine can be awaited from a destructor/scheduler stack. + * Detach local stack subscriptions; their resumed cleanup must not + * touch an endpoint which has already been fenced and destroyed. */ + for (int i = 0; i < o->reactors; i++) { + delivery_waiter_t *w = o->gates[i].waiters; + o->gates[i].waiters = NULL; + o->gates[i].tail = NULL; + zend_atomic_bool_store_ex(&o->gates[i].waiting, false); + while (w != NULL) { + delivery_waiter_t *next = w->next; + w->detached = true; + async_plain_event_fire(w->event); + w = next; + } + } + o->waiters = 0; + /* Scope teardown may be running on the scheduler stack. Cold fences + * acknowledge cancellations without trying to suspend a dead coroutine. */ + for (int i = 0; i < o->reactors; i++) { + reactor_pool_control_exec(o->pool, i, capacity_wake, &o->gates[i]); + } + /* Data fences run after accepted wires of this producer. A control + * fence alone can overtake those wires and cannot prove reclamation. */ + for (int i = 0; i < o->reactors; i++) { + reactor_pool_exec(o->pool, i, capacity_wake, &o->gates[i]); + } + owner_drain(o); + ZEND_ASSERT(zend_atomic_int_load_ex(&o->outstanding) == 0); + } + while (zend_atomic_int_load_ex(&o->outstanding) != 0 || o->waiters != 0) { + owner_drain(o); + if (zend_atomic_int_load_ex(&o->outstanding) == 0 && o->waiters == 0) { + break; + } + ZEND_ASYNC_WAKER_NEW(co); + zend_async_resume_when(co, &o->wake->base, false, zend_async_waker_callback_resolve, NULL); + if (http_atomic_pointer_load(&o->completed) == NULL) { + ZEND_ASYNC_SUSPEND(); + } + ZEND_ASYNC_WAKER_DESTROY(co); + if (EG(exception) != NULL) { + zend_clear_exception(); + } + } + for (int i = 0; i < o->reactors; i++) { + reactor_pool_capacity_unregister(o->pool, i, &o->gates[i].gate); + } +} + +void response_delivery_owner_free(response_delivery_owner_t *o) +{ + if (o == NULL) { + return; + } + ZEND_ASSERT(o->closing && zend_atomic_int_load_ex(&o->outstanding) == 0); + ZEND_ASYNC_EVENT_SET_CLOSED(&o->wake->base); + o->wake->base.dispose(&o->wake->base); + free(o->gates); + free(o); +} + +void response_delivery_note_drop(response_delivery_t *d) +{ + if (d != NULL) { + zend_atomic_int_inc(&d->dropped_wires); + } +} diff --git a/src/core/response_wire.c b/src/core/response_wire.c index e2cd78ec..b11620ae 100644 --- a/src/core/response_wire.c +++ b/src/core/response_wire.c @@ -18,6 +18,7 @@ #include "php.h" /* zend_string — persistent body carrier */ #include "core/response_wire.h" +#include "core/response_delivery.h" #include #include @@ -30,6 +31,7 @@ typedef struct { } wire_header_t; struct response_wire_s { + response_delivery_t *delivery; uint32_t reactor_id; int64_t stream_id; void *conn; @@ -82,6 +84,16 @@ struct response_wire_s { } sf; }; +void response_wire_set_delivery(response_wire_t *rw, response_delivery_t *d) +{ + rw->delivery = d; + if (d != NULL) response_delivery_wire_acquire(d); +} +response_delivery_t *response_wire_delivery(const response_wire_t *rw) +{ + return rw->delivery; +} + /* Copy `len` bytes into the arena, growing it as needed. Returns the byte * offset of the copy, or SIZE_MAX on overflow / allocation failure. A zero * length appends nothing and returns the current end offset. */ @@ -447,5 +459,6 @@ void response_wire_free(response_wire_t *rw) free(rw->arena); free(rw->headers); free(rw->trailers); + if (rw->delivery != NULL) response_delivery_wire_release(rw->delivery); free(rw); } diff --git a/src/core/thread_mailbox.c b/src/core/thread_mailbox.c index 164fc53b..b2b4306b 100644 --- a/src/core/thread_mailbox.c +++ b/src/core/thread_mailbox.c @@ -186,13 +186,18 @@ static void cmd_mailbox_on_signal(zend_async_event_t *event, zend_async_event_ca thread_cmd_mailbox_t *mb = *(thread_cmd_mailbox_t **) ((char *) event + event->extra_offset); - for (;;) { + /* One batch per callback: producers waking on capacity must not keep this + * callback running forever and starve QUIC ACKs/timers. */ + { const size_t n = thread_cmd_mpsc_drain(mb->queue, mb->batch_buf, mb->batch); if (n == 0) { - break; + return; } mb->on_drain(mb->batch_buf, n, mb->arg); + if (thread_cmd_mpsc_count(mb->queue) != 0) { + mb->trigger->trigger(mb->trigger); + } } } @@ -260,13 +265,15 @@ void thread_cmd_mailbox_free(thread_cmd_mailbox_t *mb) bool thread_cmd_mailbox_post(thread_cmd_mailbox_t *mb, const reactor_cmd_t *cmd) { - if (UNEXPECTED(!thread_cmd_mpsc_enqueue(mb->queue, cmd))) { - return false; - } - + return thread_cmd_mailbox_try_post(mb, cmd) == THREAD_QUEUE_ACCEPTED; +} +thread_queue_result_t thread_cmd_mailbox_try_post(thread_cmd_mailbox_t *mb, + const reactor_cmd_t *cmd) +{ + thread_queue_result_t result = thread_cmd_mpsc_try_enqueue(mb->queue, cmd); + if (result != THREAD_QUEUE_ACCEPTED) return result; mb->trigger->trigger(mb->trigger); - - return true; + return THREAD_QUEUE_ACCEPTED; } size_t thread_cmd_mailbox_count(const thread_cmd_mailbox_t *mb) diff --git a/src/core/thread_queue.cc b/src/core/thread_queue.cc index 0a46b715..1be883fc 100644 --- a/src/core/thread_queue.cc +++ b/src/core/thread_queue.cc @@ -305,18 +305,28 @@ void thread_cmd_mpsc_free(thread_cmd_mpsc_t *q) } bool thread_cmd_mpsc_enqueue(thread_cmd_mpsc_t *q, const reactor_cmd_t *cmd) +{ + return thread_cmd_mpsc_try_enqueue(q, cmd) == THREAD_QUEUE_ACCEPTED; +} + +thread_queue_result_t thread_cmd_mpsc_try_enqueue(thread_cmd_mpsc_t *q, + const reactor_cmd_t *cmd) { size_t prev; if (!cap_reserve(q->count, q->capacity, prev)) { - return false; + return THREAD_QUEUE_FULL; } - if (!q->q.try_enqueue(*cmd)) { + /* count reserves the bounded logical slot. try_enqueue also refuses when + * a producer needs a new internal block (even with free logical slots), + * which is not observable mailbox backpressure. Let enqueue acquire that + * block; its false result is an actual allocation failure. */ + if (!q->q.enqueue(*cmd)) { q->count.fetch_sub(1, std::memory_order_release); - return false; + return THREAD_QUEUE_ERROR; } - return true; + return THREAD_QUEUE_ACCEPTED; } size_t thread_cmd_mpsc_drain(thread_cmd_mpsc_t *q, reactor_cmd_t *out, const size_t max) diff --git a/src/core/worker_dispatch.c b/src/core/worker_dispatch.c index 716ff290..fc2eb499 100644 --- a/src/core/worker_dispatch.c +++ b/src/core/worker_dispatch.c @@ -14,6 +14,8 @@ #endif #include "core/worker_dispatch.h" +#include "core/response_delivery.h" +#include "core/async_plain_event.h" #include "php_http_server.h" /* http_server_object accessors, response API */ #include "core/http_connection.h" /* http_request_handler_coroutine_new */ #include "core/bailout_guard.h" @@ -27,6 +29,7 @@ #include "grpc/grpc_call.h" /* call lifecycle policy (init/status/finish) */ #include "Zend/zend_hrtime.h" /* zend_hrtime — request-service sampling */ #include "log/http_log.h" /* access-log emit */ +#include "fiu-local.h" #ifdef HAVE_HTTP_COMPRESSION #include "compression/http_compression_request.h" /* request body decode */ #include "compression/http_compression_response.h" /* response encode */ @@ -40,8 +43,16 @@ * an HttpRequest zval, taking ownership of the request's single reference. */ extern zval *http_request_create_from_parsed(http_request_t *req); +typedef struct worker_writer_waiter_s { + struct worker_writer_waiter_s *volatile next; + struct worker_writer_waiter_s *volatile prev; + zend_async_event_t *volatile event; + volatile bool probe; +} worker_writer_waiter_t; + /* Per-request worker-side dispatch state. Lives from worker_dispatch_request - * until the handler coroutine's dispose; ecalloc/efree on the worker thread. */ + * until the handler and all active stream operations release their references; + * ecalloc/efree on the worker thread. */ typedef struct { http_server_object *server; http_server_counters_t *counters; /* worker's real counters */ @@ -55,9 +66,22 @@ typedef struct { uint64_t start_ns; bool stamps; - /* The user handler died in a zend_bailout: dispose still renders a 500 but - * collects no telemetry for the request, same as H1/H2/H3. */ + /* The user handler died in a zend_bailout: the sender tail renders a 500. + * Legacy handler telemetry is skipped; delivery accounting remains active. */ bool handler_bailout; + zend_object *handler_exception; + response_delivery_t *delivery; + bool sender_done; + unsigned refs; + bool disposed; + bool writer_busy; + zend_coroutine_t *writer_owner; + unsigned writer_depth; + worker_writer_waiter_t *writer_waiters; + worker_writer_waiter_t *writer_tail; + worker_writer_waiter_t *idle_waiters; + worker_writer_waiter_t *idle_tail; + response_wire_t *render_wire; /* isolated/test dispatch without a record */ /* Routing echoed from the request onto the response_wire so the reactor * can resolve which QUIC stream to emit on. */ @@ -85,6 +109,144 @@ typedef struct { zend_async_trigger_event_t *credit_wake; /* worker-owned; reactor signals it */ uint64_t posted_bytes; } worker_dispatch_ctx_t; +static const http_response_stream_ops_t worker_stream_ops; +static void worker_ctx_unref(worker_dispatch_ctx_t *ctx); + +static void writer_remove(worker_dispatch_ctx_t *ctx, worker_writer_waiter_t *w) +{ + worker_writer_waiter_t **head = w->probe ? &ctx->idle_waiters : &ctx->writer_waiters; + worker_writer_waiter_t **tail = w->probe ? &ctx->idle_tail : &ctx->writer_tail; + if (w->prev != NULL) w->prev->next = w->next; + else *head = w->next; + if (w->next != NULL) w->next->prev = w->prev; + else *tail = w->prev; + if (!w->probe && !ctx->writer_busy && ctx->writer_waiters != NULL) + async_plain_event_fire(ctx->writer_waiters->event); + if (!w->probe && !ctx->writer_busy && ctx->writer_waiters == NULL) + for (worker_writer_waiter_t *p = ctx->idle_waiters; p != NULL; p = p->next) + async_plain_event_fire(p->event); +} + +/* The worker alone owns this FIFO. No second coroutine may open HEADERS or + * pass an END while the preceding sender is parked on mailbox capacity. */ +static bool writer_enter_until(worker_dispatch_ctx_t *ctx, bool nonblocking, + uint64_t deadline, bool acquire) +{ + zend_coroutine_t *co = ZEND_ASYNC_CURRENT_COROUTINE; + if (ctx->disposed) return false; + if (ctx->writer_busy && ctx->writer_owner == co) { + if (acquire) ctx->writer_depth++; + return true; + } + if (!ctx->writer_busy && ctx->writer_waiters == NULL) { + if (acquire) { + ctx->writer_busy = true; ctx->writer_owner = co; ctx->writer_depth = 1; + } + return true; + } + if (nonblocking || co == NULL || ZEND_ASYNC_IS_SCHEDULER_CONTEXT) return false; + worker_writer_waiter_t w = { .probe = !acquire }; + volatile bool linked = false, bailout = false, acquired = false; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + w.event = async_plain_event_new(); + if (w.event != NULL) { + worker_writer_waiter_t *tail = acquire ? ctx->writer_tail : ctx->idle_tail; + w.prev = tail; + if (tail == NULL) { + if (acquire) ctx->writer_waiters = &w; + else ctx->idle_waiters = &w; + } + else { + tail->next = &w; + } + if (acquire) ctx->writer_tail = &w; + else ctx->idle_tail = &w; + linked = true; + for (;;) { + if (ctx->disposed || EG(exception) != NULL) break; + if (deadline != 0 && zend_hrtime() >= deadline) break; + if (!ctx->writer_busy && (acquire ? ctx->writer_waiters == &w + : ctx->writer_waiters == NULL)) { + if (acquire) { + ctx->writer_busy = true; ctx->writer_owner = co; ctx->writer_depth = 1; + } + acquired = true; + break; + } + ZEND_ASYNC_WAKER_NEW(co); + zend_async_resume_when(co, w.event, false, + zend_async_waker_callback_resolve, NULL); + if (deadline != 0) { + uint64_t now = zend_hrtime(); + zend_async_timer_event_t *timer = ZEND_ASYNC_NEW_TIMER_EVENT( + now < deadline ? (deadline - now + 999999) / 1000000 : 1, false); + if (timer == NULL) break; + zend_async_resume_when(co, &timer->base, true, + zend_async_waker_callback_timeout, NULL); + } + if (ctx->writer_busy || (acquire ? ctx->writer_waiters != &w + : ctx->writer_waiters != NULL)) ZEND_ASYNC_SUSPEND(); + ZEND_ASYNC_WAKER_DESTROY(co); + } + } + } zend_catch { + http_bailout_state_restore(&state); + bailout = true; + } zend_end_try(); + ZEND_ASYNC_WAKER_DESTROY(co); + if (linked) writer_remove(ctx, &w); + if (w.event != NULL) w.event->dispose(w.event); + if (bailout) zend_bailout(); + return acquired; +} + +static bool writer_enter(worker_dispatch_ctx_t *ctx, bool nonblocking) +{ + return writer_enter_until(ctx, nonblocking, 0, true); +} + +static void writer_leave(worker_dispatch_ctx_t *ctx) +{ + ZEND_ASSERT(ctx->writer_busy && ctx->writer_depth != 0); + if (--ctx->writer_depth != 0) return; + ctx->writer_busy = false; ctx->writer_owner = NULL; + if (ctx->writer_waiters != NULL) async_plain_event_fire(ctx->writer_waiters->event); + else for (worker_writer_waiter_t *w = ctx->idle_waiters; w != NULL; w = w->next) + async_plain_event_fire(w->event); +} + +static response_wire_t *worker_wire_create(worker_dispatch_ctx_t *ctx) +{ + response_wire_t *rw = response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + if (rw != NULL) { + if (ctx->delivery != NULL) response_delivery_own_wire(ctx->delivery, rw); + else ctx->render_wire = rw; + } + return rw; +} + +static void worker_discard_owned(worker_dispatch_ctx_t *ctx) +{ + if (ctx->delivery != NULL) response_delivery_discard_owned(ctx->delivery); + if (ctx->render_wire != NULL) { + response_wire_discard(ctx->render_wire); + ctx->render_wire = NULL; + } +} + +static void worker_encoded_offer_refused(worker_dispatch_ctx_t *ctx) +{ + if (ctx->delivery != NULL && !ctx->stream_failed + && http_response_from_obj(Z_OBJ(ctx->response_zv))->stream_ops != &worker_stream_ops) { + /* The codec has consumed input. A later queue refusal cannot be + * retried from that encoder state or rendered as an empty gzip FULL. */ + ctx->stream_failed = true; + response_delivery_cancel(ctx->delivery, "codec_post_failed"); + response_delivery_note_drop(ctx->delivery); + } +} #ifdef HAVE_HTTP_COMPRESSION /* Decodes a Content-Encoding request body in place before the handler, as the @@ -117,13 +279,13 @@ static bool worker_decode_request_body(worker_dispatch_ctx_t *ctx) #endif /* Handler coroutine body: run the registered user handler with (request, response). */ -static void worker_dispatch_entry(void) +static void worker_dispatch_handler(void) { const zend_coroutine_t *const co = ZEND_ASYNC_CURRENT_COROUTINE; worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)co->extended_data; ZEND_ASSERT(ctx != NULL); - /* Synthetic 404 (no handler): dispose reports it like any other response. */ + /* Synthetic 404 (no handler): the sender tail sends it like any other response. */ if (ctx->skip_handler) { return; } @@ -205,7 +367,7 @@ static void worker_dispatch_entry(void) * keep_content_length says whether the table's field survives the copy. The * reactor submits the wire as it stands, so a name dropped here cannot be put * back downstream. */ -static void worker_wire_copy_head(response_wire_t *rw, zend_object *resp, +static bool worker_wire_copy_head(response_wire_t *rw, zend_object *resp, const char *cl, const size_t cl_len, const bool keep_content_length) { @@ -218,14 +380,14 @@ static void worker_wire_copy_head(response_wire_t *rw, zend_object *resp, response_wire_set_status(rw, status); if (cl_len != 0) { - response_wire_add_header(rw, "content-length", sizeof("content-length") - 1, - cl, cl_len); + if (!response_wire_add_header(rw, "content-length", sizeof("content-length") - 1, + cl, cl_len)) return false; } HashTable *const headers = http_response_get_headers(resp); if (headers == NULL) { - return; + return true; } zend_string *name; @@ -241,8 +403,8 @@ static void worker_wire_copy_head(response_wire_t *rw, zend_object *resp, } if (EXPECTED(Z_TYPE_P(values) == IS_STRING)) { - response_wire_add_header(rw, ZSTR_VAL(name), ZSTR_LEN(name), - Z_STRVAL_P(values), Z_STRLEN_P(values)); + if (!response_wire_add_header(rw, ZSTR_VAL(name), ZSTR_LEN(name), + Z_STRVAL_P(values), Z_STRLEN_P(values))) return false; } else if (Z_TYPE_P(values) == IS_ARRAY) { zval *v; ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(values), v) { @@ -250,20 +412,21 @@ static void worker_wire_copy_head(response_wire_t *rw, zend_object *resp, continue; } - response_wire_add_header(rw, ZSTR_VAL(name), ZSTR_LEN(name), - Z_STRVAL_P(v), Z_STRLEN_P(v)); + if (!response_wire_add_header(rw, ZSTR_VAL(name), ZSTR_LEN(name), + Z_STRVAL_P(v), Z_STRLEN_P(v))) return false; } ZEND_HASH_FOREACH_END(); } } ZEND_HASH_FOREACH_END(); + return true; } /* Flatten the response trailer map onto the wire. */ -static void worker_wire_copy_trailers(response_wire_t *rw, zend_object *resp) +static bool worker_wire_copy_trailers(response_wire_t *rw, zend_object *resp) { HashTable *const trailers = http_response_get_trailers(resp); if (trailers == NULL) { - return; + return true; } zend_string *name; @@ -273,9 +436,10 @@ static void worker_wire_copy_trailers(response_wire_t *rw, zend_object *resp) continue; } - response_wire_add_trailer(rw, ZSTR_VAL(name), ZSTR_LEN(name), - Z_STRVAL_P(val), Z_STRLEN_P(val)); + if (!response_wire_add_trailer(rw, ZSTR_VAL(name), ZSTR_LEN(name), + Z_STRVAL_P(val), Z_STRLEN_P(val))) return false; } ZEND_HASH_FOREACH_END(); + return true; } void response_wire_discard(response_wire_t *rw) @@ -295,8 +459,18 @@ void response_wire_discard(response_wire_t *rw) } /* The sink owns the wire in every outcome. */ -static bool worker_wire_post(worker_dispatch_ctx_t *ctx, response_wire_t *rw) +static bool worker_wire_post(worker_dispatch_ctx_t *ctx, response_wire_t *rw, + bool nonblocking) { + if (ctx->delivery != NULL) { + if (response_delivery_post(ctx->delivery, rw, + http_server_get_write_timeout_s(ctx->server) * 1000u, + nonblocking)) return true; + if (!response_delivery_failed(ctx->delivery)) return false; + ctx->stream_failed = true; + return false; + } + if (ctx->render_wire == rw) ctx->render_wire = NULL; const response_wire_kind_t kind = response_wire_kind(rw); bool delivered = false; @@ -318,8 +492,9 @@ static bool worker_wire_post(worker_dispatch_ctx_t *ctx, response_wire_t *rw) * reactor signals per ack. @p bound_ms 0 takes the configured write deadline, * which is what bounds a peer that stops ACKing. */ static bool worker_stream_wait_credit(worker_dispatch_ctx_t *ctx, - const uint32_t bound_ms) + const uint32_t bound_ms, uint64_t deadline) { + if (ctx->disposed) return false; if (ctx->credit == NULL) { return true; } @@ -327,9 +502,12 @@ static bool worker_stream_wait_credit(worker_dispatch_ctx_t *ctx, const uint32_t timeout_ms = bound_ms != 0 ? bound_ms : http_server_get_write_timeout_s(ctx->server) * 1000u; + if (deadline == 0 && timeout_ms != 0) + deadline = zend_hrtime() + (uint64_t)timeout_ms * 1000000; while (ctx->posted_bytes - stream_credit_acked(ctx->credit) >= WORKER_STREAM_INFLIGHT_CAP) { + if (deadline != 0 && zend_hrtime() >= deadline) return false; if (stream_credit_is_dead(ctx->credit)) { return false; } @@ -359,16 +537,20 @@ static bool worker_stream_wait_credit(worker_dispatch_ctx_t *ctx, zend_async_resume_when(co, &ctx->credit_wake->base, false, zend_async_waker_callback_resolve, NULL); - if (timeout_ms > 0) { - zend_async_event_t *const timer = - &ZEND_ASYNC_NEW_TIMER_EVENT((zend_ulong)timeout_ms, false)->base; - zend_async_resume_when(co, timer, true, + if (deadline != 0) { + uint64_t now = zend_hrtime(); + zend_async_timer_event_t *timer = ZEND_ASYNC_NEW_TIMER_EVENT( + now < deadline ? (deadline - now + 999999) / 1000000 : 1, false); + if (timer == NULL) return false; + zend_async_resume_when(co, &timer->base, true, zend_async_waker_callback_timeout, NULL); } ZEND_ASYNC_SUSPEND(); zend_async_waker_clean(co); + if (ctx->disposed || ctx->credit == NULL) return false; + if (EG(exception) != NULL) { return false; /* write timeout or cancelled while parked */ } @@ -377,14 +559,13 @@ static bool worker_stream_wait_credit(worker_dispatch_ctx_t *ctx, return true; } -static int worker_stream_append_chunk(void *vctx, zend_string *chunk, - const bool nonblocking) +static int worker_stream_append_impl(worker_dispatch_ctx_t *ctx, zend_string *chunk, + const bool nonblocking, volatile bool *owned) { - worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)vctx; if (UNEXPECTED(ctx->stream_ended || ctx->stream_failed) || (ctx->credit != NULL && stream_credit_is_dead(ctx->credit))) { - zend_string_release(chunk); + *owned = false; zend_string_release(chunk); return HTTP_STREAM_APPEND_STREAM_DEAD; } @@ -395,17 +576,18 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, if (nonblocking && ctx->credit != NULL && ctx->posted_bytes - stream_credit_acked(ctx->credit) >= WORKER_STREAM_INFLIGHT_CAP) { - zend_string_release(chunk); + *owned = false; zend_string_release(chunk); return HTTP_STREAM_APPEND_BACKPRESSURE; } /* first write(): open the stream; the reactor adopts one credit ref */ if (!ctx->stream_started) { response_wire_t *const hw = - response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + worker_wire_create(ctx); - if (UNEXPECTED(hw == NULL)) { - zend_string_release(chunk); + if (UNEXPECTED(hw == NULL)) { + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + *owned = false; zend_string_release(chunk); return HTTP_STREAM_APPEND_STREAM_DEAD; } @@ -414,15 +596,26 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, response_wire_set_kind(hw, RESPONSE_WIRE_STREAM_HEADERS); response_wire_set_credit(hw, ctx->credit); - worker_wire_copy_head( - hw, Z_OBJ(ctx->response_zv), NULL, 0, - http_response_keeps_declared_length(Z_OBJ(ctx->response_zv))); + if (!worker_wire_copy_head(hw, Z_OBJ(ctx->response_zv), NULL, 0, + http_response_keeps_declared_length(Z_OBJ(ctx->response_zv)))) { + worker_discard_owned(ctx); + ctx->stream_failed = true; + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + *owned = false; zend_string_release(chunk); + return HTTP_STREAM_APPEND_STREAM_DEAD; + } /* headers undeliverable → the stream never opened; don't copy and * post a chunk wire the reactor would only throw away */ - if (UNEXPECTED(!worker_wire_post(ctx, hw))) { - zend_string_release(chunk); - return HTTP_STREAM_APPEND_STREAM_DEAD; + if (UNEXPECTED(!worker_wire_post(ctx, hw, nonblocking))) { + if (nonblocking) worker_encoded_offer_refused(ctx); + if (!ctx->stream_failed) { + stream_credit_release(ctx->credit); + ctx->credit = NULL; + } + *owned = false; zend_string_release(chunk); + return ctx->stream_failed ? HTTP_STREAM_APPEND_STREAM_DEAD + : HTTP_STREAM_APPEND_BACKPRESSURE; } /* Set only once the headers wire is away: this flag is what the @@ -433,10 +626,11 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, } response_wire_t *const cw = - response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + worker_wire_create(ctx); if (UNEXPECTED(cw == NULL)) { - zend_string_release(chunk); + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + *owned = false; zend_string_release(chunk); return HTTP_STREAM_APPEND_STREAM_DEAD; } @@ -453,12 +647,14 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, /* A refused wire is a dropped chunk: the sink exhausted its retries and * worker_wire_post has already marked the stream failed. Reporting OK here * would tell the handler it wrote bytes the peer will never see. */ - if (UNEXPECTED(!worker_wire_post(ctx, cw))) { - zend_string_release(chunk); - return HTTP_STREAM_APPEND_STREAM_DEAD; + if (UNEXPECTED(!worker_wire_post(ctx, cw, nonblocking))) { + if (nonblocking) worker_encoded_offer_refused(ctx); + *owned = false; zend_string_release(chunk); + return ctx->stream_failed ? HTTP_STREAM_APPEND_STREAM_DEAD + : HTTP_STREAM_APPEND_BACKPRESSURE; } - zend_string_release(chunk); /* bytes copied into the wire arena */ + *owned = false; zend_string_release(chunk); /* bytes copied into the wire arena */ ctx->posted_bytes += chunk_len; @@ -466,7 +662,8 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, return HTTP_STREAM_APPEND_OK; /* room was checked above; never parks */ } - if (!worker_stream_wait_credit(ctx, 0)) { + if (ctx->disposed) return HTTP_STREAM_APPEND_STREAM_DEAD; + if (!worker_stream_wait_credit(ctx, 0, 0)) { ctx->stream_failed = true; /* credit timeout / cancelled while parked */ return HTTP_STREAM_APPEND_STREAM_DEAD; } @@ -474,15 +671,46 @@ static int worker_stream_append_chunk(void *vctx, zend_string *chunk, return HTTP_STREAM_APPEND_OK; } +static int worker_stream_append_chunk(void *arg, zend_string *chunk, bool nonblocking) +{ + worker_dispatch_ctx_t *ctx = arg; + ctx->refs++; + volatile bool locked = false, owned = true, bailout = false; + volatile int rc = HTTP_STREAM_APPEND_STREAM_DEAD; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + locked = writer_enter(ctx, nonblocking); + if (locked) rc = worker_stream_append_impl(ctx, chunk, nonblocking, &owned); + else if (!ctx->disposed && !ctx->stream_failed && nonblocking) + rc = HTTP_STREAM_APPEND_BACKPRESSURE; + } zend_catch { + http_bailout_state_restore(&state); + bailout = true; + worker_discard_owned(ctx); + ctx->stream_failed = true; + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + } zend_end_try(); + if (owned) zend_string_release(chunk); + if (locked) writer_leave(ctx); + worker_ctx_unref(ctx); + if (bailout) zend_bailout(); + return rc; +} + /* sendable() advisory: true while append_chunk would not park. */ static bool worker_stream_sendable(void *vctx) { worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)vctx; - if (ctx->stream_ended || ctx->stream_failed) { + if (ctx->disposed || ctx->stream_ended || ctx->stream_failed + || (ctx->writer_busy && ctx->writer_owner != ZEND_ASYNC_CURRENT_COROUTINE) + || ctx->writer_waiters != NULL) { return false; } + if (ctx->delivery != NULL && !response_delivery_sendable(ctx->delivery)) return false; + if (ctx->credit == NULL) { return true; } @@ -498,16 +726,15 @@ static bool worker_stream_is_alive(void *vctx) { const worker_dispatch_ctx_t *const ctx = (const worker_dispatch_ctx_t *)vctx; - if (ctx->stream_ended || ctx->stream_failed) { + if (ctx->disposed || ctx->stream_ended || ctx->stream_failed) { return false; } return ctx->credit == NULL || !stream_credit_is_dead(ctx->credit); } -static void worker_stream_mark_ended(void *vctx) +static void worker_stream_mark_ended_impl(worker_dispatch_ctx_t *ctx) { - worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)vctx; if (!ctx->stream_started || ctx->stream_ended) { return; @@ -516,9 +743,10 @@ static void worker_stream_mark_ended(void *vctx) ctx->stream_ended = true; response_wire_t *const ew = - response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + worker_wire_create(ctx); if (UNEXPECTED(ew == NULL)) { + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); return; } @@ -527,10 +755,35 @@ static void worker_stream_mark_ended(void *vctx) response_wire_set_abort_code(ew, ctx->abort_code); } else { response_wire_set_kind(ew, RESPONSE_WIRE_STREAM_END); - worker_wire_copy_trailers(ew, Z_OBJ(ctx->response_zv)); + if (!worker_wire_copy_trailers(ew, Z_OBJ(ctx->response_zv))) { + worker_discard_owned(ctx); + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + return; + } } - worker_wire_post(ctx, ew); + worker_wire_post(ctx, ew, false); +} + +static void worker_stream_mark_ended(void *arg) +{ + worker_dispatch_ctx_t *ctx = arg; + ctx->refs++; + volatile bool locked = false, bailout = false; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + locked = writer_enter(ctx, false); + if (locked) worker_stream_mark_ended_impl(ctx); + } zend_catch { + http_bailout_state_restore(&state); + bailout = true; + worker_discard_owned(ctx); + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + } zend_end_try(); + if (locked) writer_leave(ctx); + worker_ctx_unref(ctx); + if (bailout) zend_bailout(); } /* The terminal wire above is already the abort one when stream_failed is set, @@ -555,11 +808,60 @@ static bool worker_stream_abort(void *vctx, const int64_t error_code) /* The credit wait the blocking path takes, offered to a non-blocking caller * that asked to be told when room comes back. */ -static bool worker_stream_wait_writable(void *vctx, const uint32_t timeout_ms) +static uint32_t writer_remaining_ms(uint64_t deadline) { - worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)vctx; + if (deadline == 0) return 0; + uint64_t now = zend_hrtime(); + return now < deadline ? (uint32_t)((deadline - now + 999999) / 1000000) : 1; +} + +static bool worker_stream_wait_writable_impl(worker_dispatch_ctx_t *ctx, uint64_t deadline) +{ + if (ctx->disposed) return false; + + if (ctx->delivery != NULL && !response_delivery_wait_writable(ctx->delivery, + writer_remaining_ms(deadline))) + return false; - return worker_stream_wait_credit(ctx, timeout_ms); + if (deadline != 0 && zend_hrtime() >= deadline) return false; + return worker_stream_wait_credit(ctx, writer_remaining_ms(deadline), deadline); +} + +static bool worker_stream_wait_writable(void *arg, uint32_t timeout_ms) +{ + worker_dispatch_ctx_t *ctx = arg; + ctx->refs++; + volatile bool result = false, bailout = false; + uint32_t bound = ctx->disposed ? 0 : (timeout_ms != 0 ? timeout_ms + : http_server_get_write_timeout_s(ctx->server) * 1000u); + uint64_t deadline = bound != 0 ? zend_hrtime() + (uint64_t)bound * 1000000 : 0; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + while (!ctx->disposed && !ctx->stream_ended && !ctx->stream_failed) { + /* Readiness waits do not reserve the writer between API calls: + * otherwise two tryWrite/awaitWritable loops can pass a lock back + * and forth forever without either offering its chunk. */ + if (!writer_enter_until(ctx, false, deadline, false)) break; + if (!worker_stream_wait_writable_impl(ctx, deadline)) break; + if (worker_stream_sendable(ctx)) { result = true; break; } + if (deadline != 0 && zend_hrtime() >= deadline) break; + } + } + zend_catch { + http_bailout_state_restore(&state); + bailout = true; + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "wait_failed"); + } zend_end_try(); + worker_ctx_unref(ctx); + if (bailout) zend_bailout(); + return result; +} + +static bool worker_stream_is_started(void *vctx) +{ + worker_dispatch_ctx_t *ctx = vctx; + return ctx->stream_started || ctx->writer_busy || ctx->writer_waiters != NULL; } static const http_response_stream_ops_t worker_stream_ops = { @@ -570,6 +872,7 @@ static const http_response_stream_ops_t worker_stream_ops = { .mark_ended = worker_stream_mark_ended, .abort = worker_stream_abort, .get_wait_event = NULL, /* the wait above is the one to take */ + .is_started = worker_stream_is_started, }; /* grpc-web in-body trailer frame; consumes the ref. */ @@ -595,7 +898,7 @@ static void worker_grpc_end_stream(void *vctx) worker_stream_mark_ended(vctx); } -/* Trailers-Only: dispose posts the FULL wire right after this returns. */ +/* Trailers-Only: the sender tail posts the FULL wire after this returns. */ static void worker_grpc_commit(void *vctx) { (void)vctx; @@ -609,12 +912,13 @@ static const grpc_finish_ops_t worker_grpc_finish_ops = { /* Flatten the committed HttpResponse into a response_wire. Buffered only. * Returns NULL on allocation failure. */ -static response_wire_t *worker_render_response(const worker_dispatch_ctx_t *ctx) +static response_wire_t *worker_render_response(worker_dispatch_ctx_t *ctx) { + fiu_return_on("h3/delivery/render_failed", NULL); zend_object *const resp = Z_OBJ(ctx->response_zv); response_wire_t *const rw = - response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + worker_wire_create(ctx); if (rw == NULL) { return NULL; @@ -630,11 +934,15 @@ static response_wire_t *worker_render_response(const worker_dispatch_ctx_t *ctx) bool keep_cl; const size_t cl_len = http_response_wire_content_length(resp, cl, &keep_cl); - worker_wire_copy_head(rw, resp, cl, cl_len, keep_cl); - worker_wire_copy_trailers(rw, resp); + if (!worker_wire_copy_head(rw, resp, cl, cl_len, keep_cl) + || !worker_wire_copy_trailers(rw, resp)) { + worker_discard_owned(ctx); + return NULL; + } /* http_response_get_body_str returns a borrowed reference; the bytes are - * copied into the arena, so nothing to release. HEAD carries the headers + * copied into a persistent wire-owned string, so nothing to release here. + * HEAD carries the headers * but no body (RFC 9110 §9.3.2). */ if (!ctx->is_head) { zend_string *const body = http_response_get_body_str(resp); @@ -665,11 +973,11 @@ static const char *worker_req_header(const http_request_t *req, const char *name return v != NULL ? ZSTR_VAL(v) : NULL; } -static response_wire_t *worker_render_send_file(const worker_dispatch_ctx_t *ctx, +static response_wire_t *worker_render_send_file(worker_dispatch_ctx_t *ctx, const http_send_file_request_t *sf) { response_wire_t *const rw = - response_wire_create(ctx->reactor_id, ctx->stream_id, ctx->conn); + worker_wire_create(ctx); if (rw == NULL) { return NULL; @@ -720,26 +1028,19 @@ static response_wire_t *worker_render_send_file(const worker_dispatch_ctx_t *ctx } if (!response_wire_set_send_file(rw, &wsf)) { - response_wire_free(rw); + worker_discard_owned(ctx); return NULL; } return rw; } -/* Coroutine dispose: commit the response (or derive a 500 from an unhandled - * exception), render it into a response_wire, hand it to the sink, and drop the - * per-request state. */ -static void worker_dispatch_dispose(zend_coroutine_t *coroutine) +/* Coroutine sender tail: commit the response (or derive a 500 from an + * unhandled exception), render a response_wire, and hand it to the sink. + * This path may suspend; disposal releases the per-request state separately. */ +static void worker_dispatch_finalize(worker_dispatch_ctx_t *ctx, + zend_coroutine_t *coroutine) { - worker_dispatch_ctx_t *const ctx = (worker_dispatch_ctx_t *)coroutine->extended_data; - ZEND_ASSERT(ctx != NULL); - - coroutine->extended_data = NULL; - - /* Un-bracket the in-flight request (--active), paired with the - * on_request_dispatch in worker_dispatch_request. */ - http_server_on_request_dispose(ctx->counters); /* A thrown handler exception becomes a response (derived 500 below, or * a grpc-status / aborted stream) — mark it consumed on both escalation @@ -756,14 +1057,14 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) zend_object *const resp = Z_OBJ(ctx->response_zv); /* gRPC maps exceptions to grpc-status, not HTTP 500 */ - if (coroutine->exception != NULL && !ctx->is_grpc + if (ctx->handler_exception != NULL && !ctx->is_grpc && !http_response_is_committed(resp)) { http_response_reset_to_error(resp, 500, "Internal Server Error"); } if (ctx->is_grpc) { grpc_call_ensure_status(resp, - coroutine->exception != NULL || ctx->handler_bailout); + ctx->handler_exception != NULL || ctx->handler_bailout); } else { http_response_reset_after_bailout(resp, ctx->handler_bailout); } @@ -776,7 +1077,7 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) grpc_call_finish(resp, &worker_grpc_finish_ops, ctx); } else if (http_response_is_streaming(resp)) { (void)http_response_finish_stream( - resp, http_handler_failed(coroutine, ctx->handler_bailout), -1); + resp, ctx->handler_exception != NULL || ctx->handler_bailout, -1); } /* a started stream must always get a terminal wire */ @@ -784,7 +1085,8 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) worker_stream_mark_ended(ctx); } - if (!ctx->stream_started) { + if (!ctx->stream_started && !ctx->stream_failed + && (ctx->delivery == NULL || !response_delivery_failed(ctx->delivery))) { /* sendFile() seals the response: marshal path + opts to the * reactor, which opens the file and runs the sendfile engine. * Falls through to the buffered render when absent (#105). */ @@ -802,18 +1104,20 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) } if (rw != NULL) { - /* Only a sendFile the reactor actually received is accounted - * there; one that never made it off this thread still has to be - * reported here, or it is counted by nobody. */ + /* Legacy sendFile accounting belongs to the reactor only after + * acceptance. A live delivery record reports its final outcome + * on the originating worker for every response kind. */ marshalled_send_file = - worker_wire_post(ctx, rw) && is_send_file; /* sink owns rw now */ + worker_wire_post(ctx, rw, false) && is_send_file; /* sink owns rw now */ + } else if (ctx->delivery != NULL) { + response_delivery_cancel(ctx->delivery, "render_failed"); } } - /* Collect telemetry here, where the response is final — except for a - * marshalled sendFile: its status is only stamped once the reactor has - * run the engine, so the reactor's cleanup reports that one instead. */ - if (EXPECTED(!ctx->handler_bailout) && !marshalled_send_file) { + /* Legacy telemetry without a delivery record: a marshalled sendFile + * obtains its final status in reactor cleanup. Live records defer all + * accounting until the transport outcome and sender snapshot are ready. */ + if (ctx->delivery == NULL && EXPECTED(!ctx->handler_bailout) && !marshalled_send_file) { http_request_telemetry(Z_ISUNDEF(ctx->request_zv) ? NULL : http_request_from_zobj(Z_OBJ(ctx->request_zv)), @@ -821,9 +1125,72 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) http_server_get_log_state(ctx->server)); } - /* ctx dies below; a late write() on a kept $response must throw, not UAF */ + /* Detach operations before cleanup: a late write() on a retained + * response must throw rather than access a released context. */ http_response_replace_stream_ops(resp, NULL, NULL); } +} + +/* A coroutine tail, not a destructor: queue capacity may suspend this sender. + * Every handler outcome, including synthetic responses, reaches this tail. */ +static void worker_dispatch_entry(void) +{ + zend_coroutine_t *co = ZEND_ASYNC_CURRENT_COROUTINE; + worker_dispatch_ctx_t *ctx = co->extended_data; + worker_dispatch_handler(); + if (EG(exception) != NULL) { + ctx->handler_exception = EG(exception); + GC_ADDREF(ctx->handler_exception); + zend_clear_exception(); + } + http_bailout_state_t state; + http_bailout_state_save(&state); + volatile bool locked = false; + zend_try { + locked = writer_enter(ctx, false); + if (locked) worker_dispatch_finalize(ctx, co); + else if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "sender_cancelled"); + } zend_catch { + http_bailout_state_restore(&state); + ctx->handler_bailout = true; + worker_discard_owned(ctx); + if (ctx->delivery != NULL) response_delivery_cancel(ctx->delivery, "render_failed"); + } zend_end_try(); + if (locked) writer_leave(ctx); + if (ctx->delivery != NULL) { + response_delivery_sender_done(ctx->delivery, + http_request_from_zobj(Z_OBJ(ctx->request_zv)), Z_OBJ(ctx->response_zv), + true); + } + ctx->sender_done = true; + if (EG(exception) != NULL) zend_clear_exception(); +} + +static void worker_dispatch_dispose(zend_coroutine_t *coroutine) +{ + worker_dispatch_ctx_t *ctx = coroutine->extended_data; + coroutine->extended_data = NULL; + http_server_on_request_dispose(ctx->counters); + if (!ctx->sender_done && ctx->delivery != NULL) { + response_delivery_cancel(ctx->delivery, "sender_cancelled"); + response_delivery_sender_done(ctx->delivery, + http_request_from_zobj(Z_OBJ(ctx->request_zv)), Z_OBJ(ctx->response_zv), + true); + } + if (coroutine->exception != NULL) { + ZEND_COROUTINE_SET_EXCEPTION_HANDLED(coroutine); + ZEND_ASYNC_EVENT_SET_EXC_CAUGHT(&coroutine->event); + } + ctx->disposed = true; + if (!Z_ISUNDEF(ctx->response_zv)) + http_response_replace_stream_ops(Z_OBJ(ctx->response_zv), NULL, NULL); + for (worker_writer_waiter_t *w = ctx->writer_waiters; w != NULL; w = w->next) + async_plain_event_fire(w->event); + for (worker_writer_waiter_t *w = ctx->idle_waiters; w != NULL; w = w->next) + async_plain_event_fire(w->event); + worker_discard_owned(ctx); + if (ctx->handler_exception != NULL) OBJ_RELEASE(ctx->handler_exception); + if (ctx->delivery != NULL) response_delivery_release(ctx->delivery); if (ctx->credit != NULL) { if (ctx->credit_wake != NULL) { @@ -837,11 +1204,19 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) } if (ctx->credit_wake != NULL) { + async_plain_event_fire(&ctx->credit_wake->base); ZEND_ASYNC_EVENT_SET_CLOSED(&ctx->credit_wake->base); ctx->credit_wake->base.dispose(&ctx->credit_wake->base); ctx->credit_wake = NULL; } + worker_ctx_unref(ctx); +} + +static void worker_ctx_unref(worker_dispatch_ctx_t *ctx) +{ + ZEND_ASSERT(ctx->refs != 0); + if (--ctx->refs != 0) return; if (!Z_ISUNDEF(ctx->request_zv)) { zval_ptr_dtor(&ctx->request_zv); ZVAL_UNDEF(&ctx->request_zv); @@ -855,6 +1230,19 @@ static void worker_dispatch_dispose(zend_coroutine_t *coroutine) efree(ctx); } +void worker_dispatch_cancel_request(http_request_t *req) +{ + if (req == NULL) return; + response_delivery_t *d = req->delivery; + req->delivery = NULL; + if (d != NULL) { + response_delivery_worker_begin(d, req); + response_delivery_cancel(d, "dispatch_failed"); + response_delivery_sender_done(d, req, NULL, true); + } + http_request_destroy(req); +} + bool worker_dispatch_request(http_server_object *server, zend_async_scope_t *scope, http_request_t *req, @@ -863,7 +1251,7 @@ bool worker_dispatch_request(http_server_object *server, { if (UNEXPECTED(server == NULL || scope == NULL || req == NULL)) { if (req != NULL) { - http_request_destroy(req); /* we own it; nothing else can free it */ + worker_dispatch_cancel_request(req); } return false; @@ -880,15 +1268,19 @@ bool worker_dispatch_request(http_server_object *server, const grpc_mode_t grpc_mode = grpc_classify(req, handlers); const bool is_grpc = grpc_mode != GRPC_MODE_NONE; + if (req->delivery != NULL) response_delivery_worker_begin(req->delivery, req); + zval *const req_obj = http_request_create_from_parsed(req); if (UNEXPECTED(req_obj == NULL)) { - http_request_destroy(req); /* nothing took the ref yet */ + worker_dispatch_cancel_request(req); return false; } worker_dispatch_ctx_t *const ctx = ecalloc(1, sizeof(*ctx)); + ctx->refs = 1; ctx->server = server; + ctx->delivery = req->delivery; ctx->counters = http_server_counters(server); ctx->stamps = http_server_sample_stamps_enabled(http_server_view(server)); ctx->reactor_id = reactor_id; @@ -950,12 +1342,19 @@ bool worker_dispatch_request(http_server_object *server, scope, worker_dispatch_entry, ctx, worker_dispatch_dispose, own_scope); if (UNEXPECTED(co == NULL)) { + if (ctx->delivery != NULL) { + response_delivery_cancel(ctx->delivery, "spawn_failed"); + response_delivery_sender_done(ctx->delivery, req, Z_OBJ(ctx->response_zv), true); + } zval_ptr_dtor(&ctx->request_zv); zval_ptr_dtor(&ctx->response_zv); efree(ctx); return false; } + if (ctx->delivery != NULL) response_delivery_addref(ctx->delivery); /* coroutine ctx */ + req->delivery = NULL; /* ctx now owns the dispatch-failure path */ + /* Bracket the in-flight request on the worker's counters (++active), * paired with on_request_dispose at coroutine dispose. */ http_server_on_request_dispatch(ctx->counters); diff --git a/src/core/worker_inbox.c b/src/core/worker_inbox.c index 3d8b444f..7b8cb0dc 100644 --- a/src/core/worker_inbox.c +++ b/src/core/worker_inbox.c @@ -15,6 +15,7 @@ #include "core/worker_inbox.h" #include "core/thread_mailbox.h" +#include "core/response_delivery.h" /* Bounded so a backed-up worker backpressures the reactor (post returns false) * rather than growing the queue without limit. */ @@ -28,6 +29,7 @@ struct worker_inbox_s { worker_response_sink_fn sink; void *sink_arg; bool own_scope; + response_delivery_owner_t *delivery_owner; }; /* Runs on the worker's reactor thread when requests are queued. Each item is an @@ -43,6 +45,10 @@ static void worker_inbox_drain(void **items, const size_t count, void *arg) http_request_t *const req = (http_request_t *)items[i]; ZEND_ASSERT(req != NULL); + if (inbox->scope == NULL) { + worker_dispatch_cancel_request(req); + continue; + } worker_dispatch_request(inbox->server, inbox->scope, req, inbox->own_scope, inbox->sink, inbox->sink_arg); } @@ -96,5 +102,23 @@ void worker_inbox_free(worker_inbox_t *inbox) } thread_mailbox_free(inbox->mb); + response_delivery_owner_free(inbox->delivery_owner); pefree(inbox, 0); } + +void worker_inbox_set_delivery_owner(worker_inbox_t *inbox, + response_delivery_owner_t *owner) +{ + inbox->delivery_owner = owner; +} +response_delivery_owner_t *worker_inbox_delivery_owner(worker_inbox_t *inbox) +{ + return inbox != NULL ? inbox->delivery_owner : NULL; +} + +void worker_inbox_cancel_pending(worker_inbox_t *inbox) +{ + if (inbox == NULL) return; + inbox->scope = NULL; + thread_mailbox_drain_pending(inbox->mb); +} diff --git a/src/http3/http3_callbacks.c b/src/http3/http3_callbacks.c index ffa5705c..667e273e 100644 --- a/src/http3/http3_callbacks.c +++ b/src/http3/http3_callbacks.c @@ -38,6 +38,8 @@ #include "core/bailout_guard.h" /* zend_try around body appends */ #include "http3_listener.h" /* http3_listener_server_obj etc. */ #include "http3_packet.h" /* http3_packet_compute_sr_token */ +#include "core/response_delivery.h" +#include "fiu-local.h" #include "http3_steer.h" /* CID steering encode */ #include "core/response_wire.h" /* response_wire_* (reverse path) */ #include "core/stream_credit.h" /* reverse-path flow control */ @@ -1017,6 +1019,7 @@ bool http3_stream_submit_response(http3_connection_t *c, http3_packet_stats_t *const stats = http3_listener_packet_stats(c->listener); if (rv == 0) { + response_delivery_started(s->delivery, http_response_get_status(resp_obj)); if (stats != NULL) stats->h3_response_submitted++; return true; } @@ -1081,6 +1084,7 @@ void http3_stream_adopt_wire_trailers(http3_stream_t *s, const response_wire_t * bool http3_stream_submit_response_wire(http3_connection_t *c, http3_stream_t *s, response_wire_t *rw) { + fiu_return_on("h3/delivery/submit_failed", false); if (c == NULL || s == NULL || rw == NULL || c->nghttp3_conn == NULL) { return false; } @@ -1136,6 +1140,7 @@ bool http3_stream_submit_response_wire(http3_connection_t *c, http3_stream_t *s, h3_chunk_queue_init(s); /* adopt the worker's credit ref; released at teardown */ s->wire_credit = response_wire_credit(rw); + response_wire_set_credit(rw, NULL); /* adopted: the wire no longer owns this ref */ } else { /* Adopt the worker-built persistent body — the data reader walks it * in place; released (flag-aware) in http3_stream_release. */ @@ -1163,6 +1168,7 @@ bool http3_stream_submit_response_wire(http3_connection_t *c, http3_stream_t *s, http3_packet_stats_t *const stats = http3_listener_packet_stats(c->listener); if (rv == 0) { + response_delivery_started(s->delivery, status); if (stats != NULL) stats->h3_response_submitted++; if (streaming) { @@ -1778,6 +1784,8 @@ static int h3_acked_stream_data_cb(nghttp3_conn *conn, int64_t stream_id, (void)conn; (void)stream_id; (void)user_data; http3_stream_t *const s = (http3_stream_t *)stream_user_data; + if (s != NULL) response_delivery_ack(s->delivery, datalen); + if (s == NULL || s->chunk_queue == NULL) { return 0; } @@ -2309,6 +2317,7 @@ static int acked_stream_data_offset_cb(ngtcp2_conn *conn, int64_t stream_id, (void)conn; (void)offset; http3_connection_t *const c = (http3_connection_t *)user_data; http3_stream_t *s = (http3_stream_t *)stream_user_data; + if (s != NULL) response_delivery_ack_transport(s->delivery, datalen); if (c == NULL) { return 0; @@ -2364,6 +2373,13 @@ static int stream_close_cb(ngtcp2_conn *conn, uint32_t flags, if (c == NULL) { return 0; } + if (stream_user_data != NULL) { + http3_stream_t *s = stream_user_data; + response_delivery_finish(s->delivery, + (flags & NGTCP2_STREAM_CLOSE_FLAG_APP_ERROR_CODE_SET) == 0 + && !s->local_aborted, + "stream_closed"); + } /* ngtcp2 never auto-extends MAX_STREAMS on close, so without this each * connection caps at initial_max_streams_bidi. id&3==0 = client bidi. @@ -2478,4 +2494,3 @@ const ngtcp2_callbacks HTTP3_NGTCP2_CALLBACKS = { .stream_stop_sending = stream_stop_sending_cb, .extend_max_remote_streams_bidi = extend_max_remote_streams_bidi_cb, }; - diff --git a/src/http3/http3_connection.c b/src/http3/http3_connection.c index 393256a1..b44fde69 100644 --- a/src/http3/http3_connection.c +++ b/src/http3/http3_connection.c @@ -26,6 +26,7 @@ * ngtcp2 + nghttp3 + openssl/ssl.h + * http3_connection.h + php_http_server.h */ #include "Zend/zend_hrtime.h" /* zend_hrtime — drain stamps */ +#include "core/response_delivery.h" #include "http3_listener.h" /* listener accessors */ #include "http3_packet.h" /* version_negotiation / stateless_reset */ #include "http3_steer.h" /* CID steering encode */ @@ -1027,6 +1028,7 @@ void http3_connection_free(http3_connection_t *conn) conn->streams_head = NULL; while (s != NULL) { http3_stream_t *next = s->list_next; + response_delivery_finish(s->delivery, false, "connection_closed"); /* Cancel before clearing s->conn: the pump is a callback FSM with * no scope to cancel it, and it holds the file io + a stream pin. */ @@ -1103,4 +1105,3 @@ void http3_connection_free(http3_connection_t *conn) OPENSSL_cleanse(conn, sizeof(*conn)); efree(conn); } - diff --git a/src/http3/http3_dispatch.c b/src/http3/http3_dispatch.c index 5ad49d28..accf5fab 100644 --- a/src/http3/http3_dispatch.c +++ b/src/http3/http3_dispatch.c @@ -31,6 +31,7 @@ #include "static/static_handler.h" /* http_static_try_serve / count */ #include "core/response_wire.h" /* response_wire_* (reverse path) */ #include "core/worker_dispatch.h" /* response_wire_discard */ +#include "core/response_delivery.h" #ifdef HAVE_HTTP_COMPRESSION #include "compression/http_compression_request.h" /* request body decode */ #include "compression/http_compression_response.h" /* response encode */ @@ -136,6 +137,27 @@ static const http_static_dispatch_cbs_t h3_static_dispatch_cbs = { * channel. The reactor keeps the stream alive via a worker-borrow ref until the * consumed arrives. No request-service stats here — those are the * worker's job (handler runs there). */ +static void h3_delivery_release_request(void *arg) +{ + http3_stream_release(arg); +} + +static void h3_delivery_reset(void *arg) +{ + http3_stream_t *s = arg; + http3_connection_t *c = s->conn; + if (c == NULL || c->closed || s->peer_closed) return; + s->local_aborted = true; + s->streaming_ended = true; + if (c->nghttp3_conn != NULL) + (void)nghttp3_conn_shutdown_stream_write(c->nghttp3_conn, s->stream_id); + if (c->ngtcp2_conn != NULL) + (void)ngtcp2_conn_shutdown_stream_write(c->ngtcp2_conn, 0, s->stream_id, + NGHTTP3_H3_INTERNAL_ERROR); + http3_listener_mark_flush(c->listener, c); + http3_listener_queue_epilogue_flush(c->listener); +} + static void http3_stream_dispatch_to_worker(http3_connection_t *c, http3_stream_t *s, const http3_reactor_ctx_t *rctx) { @@ -187,6 +209,17 @@ static void http3_stream_dispatch_to_worker(http3_connection_t *c, http3_stream_ s->conn = c; + response_delivery_owner_t *owner = worker_inbox_delivery_owner(inbox); + if (owner == NULL) return; + s->delivery = response_delivery_create(owner, rctx->reactor_id, s, + h3_delivery_reset, h3_delivery_release_request, + http3_reactor_apply_response); + if (s->delivery == NULL) { + h3_delivery_reset(s); + return; + } + s->request->delivery = s->delivery; + /* Routing for the reverse path. reactor_id selects the reverse channel; * reactor_conn carries the raw stream pointer (kept alive by the * worker-borrow ref below until consumed, so it is valid when the response @@ -204,6 +237,9 @@ static void http3_stream_dispatch_to_worker(http3_connection_t *c, http3_stream_ * H3_REQUEST_REJECTED so the client can retry instead of hanging */ s->refcount--; s->dispatched = false; + s->request->delivery = NULL; + response_delivery_abandon(s->delivery); + s->delivery = NULL; if (c->ngtcp2_conn != NULL) { (void)ngtcp2_conn_shutdown_stream_write( @@ -317,12 +353,10 @@ static http_request_t *h3_reactor_request_from_wire(http3_connection_t *c, * which needs response_zv UNDEF — so dtor it here. */ static void h3_reactor_sendfile_cleanup(http3_connection_t *c, http3_stream_t *s) { - /* Pool sendFile: the worker marshalled the send and left the delivery to - * us, so the reactor is the only place the final status exists. c->log_state - * is the OFF default here — this counts, and never touches a worker's sink. - * The stand-in request carries the method/URI; the worker's own request is - * off-limits on this thread. */ - if (c != NULL && !Z_ISUNDEF(s->response_zv)) { + /* A live delivery record reports the transport outcome on its worker. + * Without a record, legacy sendFile telemetry uses this reactor-owned + * stand-in request; the worker request and log sink are off-limits here. */ + if (s->delivery == NULL && c != NULL && !Z_ISUNDEF(s->response_zv)) { http_request_telemetry(s->sf_request, Z_OBJ(s->response_zv), c->counters, c->log_state); } @@ -347,12 +381,16 @@ static void h3_reactor_sendfile_cleanup(http3_connection_t *c, http3_stream_t *s * base (nghttp3) + worker-borrow refs already cover its lifetime. */ static void h3_reactor_sendfile_on_done(void *user, int status) { - (void)status; h3_sendfile_user_t *const u = (h3_sendfile_user_t *)user; http3_connection_t *const c = u->conn; http3_stream_t *const s = u->stream; efree(u); + if (status < 0 && s->delivery != NULL) { + response_delivery_finish(s->delivery, false, "sendfile_failed"); + h3_delivery_reset(s); + } + h3_reactor_sendfile_cleanup(c, s); } @@ -405,11 +443,21 @@ void http3_reactor_apply_response(void *arg) } http3_stream_t *const s = (http3_stream_t *)response_wire_conn(rw); + response_delivery_t *d = response_wire_delivery(rw); + bool discard_wire = false; + response_delivery_addref(d); /* protects reclamation after freeing the wire */ http3_connection_t *const c = (s != NULL) ? s->conn : NULL; - if (c == NULL || c->closed || c->nghttp3_conn == NULL) { + if (c == NULL || c->closed || c->nghttp3_conn == NULL + || (d != NULL && response_delivery_failed(d))) { /* stream gone: abandon credit / release chunk — unblock the producer */ + response_delivery_note_drop(d); response_wire_discard(rw); + if (d != NULL) { + response_delivery_finish(d, false, "stream_closed"); + response_delivery_maybe_release_request(d); + response_delivery_release(d); + } return; } @@ -420,6 +468,11 @@ void http3_reactor_apply_response(void *arg) if (http3_stream_submit_response_wire(c, s, rw)) { http3_listener_mark_flush(c->listener, c); http3_listener_queue_epilogue_flush(c->listener); + } else if (d != NULL) { + discard_wire = true; + response_delivery_note_drop(d); + response_delivery_finish(d, false, "submit_failed"); + h3_delivery_reset(s); } break; @@ -480,6 +533,7 @@ void http3_reactor_apply_response(void *arg) s->streaming_ended = true; s->local_aborted = true; + response_delivery_finish(d, false, "response_aborted"); /* Stops nghttp3 asking the data reader for more of a body that is * not coming; the RESET_STREAM below is what the peer sees. Same @@ -506,7 +560,12 @@ void http3_reactor_apply_response(void *arg) break; } - response_wire_free(rw); + if (discard_wire) response_wire_discard(rw); + else response_wire_free(rw); + if (d != NULL) { + response_delivery_maybe_release_request(d); + response_delivery_release(d); + } } /* Reactor-side static serving: serve files @@ -1100,4 +1159,3 @@ static void h3_handler_coroutine_dispose(zend_coroutine_t *coroutine) h3_dispose_tail(c, s); } - diff --git a/src/http3/http3_stream.c b/src/http3/http3_stream.c index e6be20c7..34cda817 100644 --- a/src/http3/http3_stream.c +++ b/src/http3/http3_stream.c @@ -23,6 +23,7 @@ #include "http_response_internal.h" /* http_response_replace_stream_ops */ #include "http3_listener.h" /* http3_listener_stream_pool */ #include "fiu-local.h" +#include "core/response_delivery.h" /* Static-delivery memory accounting (http3_static_response.c). Declared here * rather than via the heavy http3_internal.h — teardown only needs the debit. */ @@ -143,23 +144,24 @@ static void http3_stream_release_via_request(http_request_t *req) return; } - /* Hand the release back via the worker's ordered FIFO — no busy-spin, - * stays behind any parked wire of this stream. The fallback only fires - * when no timer can be armed (stopping loop), where the FIFO is empty; - * it spins while the reactor's mailbox is full, which its loop drains, - * and stops when the reactor leaves that loop. */ - if (!http_worker_reactor_post_release(s->req_reactor_id, - http3_reactor_consumed_apply, s)) { - while (!reactor_pool_post_exec(s->req_reactor_pool, s->req_reactor_id, - http3_reactor_consumed_apply, s)) { - if (http3_stream_slot_reactor_gone(s)) { - http3_stream_slot_release_drop(s); - break; - } + if (s->delivery != NULL) { + if (req->delivery != NULL) { + response_delivery_sender_abandoned(req->delivery); + req->delivery = NULL; } + response_delivery_request_consumed(s->delivery); + return; } + /* A preallocated control node cannot be refused because data is full. + * Dispatched delivery records separately pin their outstanding wires. */ + s->request_release_control.fn = http3_reactor_consumed_apply; + s->request_release_control.arg = s; + (void)reactor_pool_post_control(s->req_reactor_pool, s->req_reactor_id, + &s->request_release_control); return; + + } http3_stream_pool_free(s->pool, s); @@ -174,6 +176,10 @@ void http3_stream_release(http3_stream_t *s) if (--s->refcount > 0) { return; } + if (s->delivery != NULL) { + response_delivery_stream_gone(s->delivery); + s->delivery = NULL; + } /* Reactor mode: the worker owns the request's lifetime; the slab slot is * ours to reclaim. Read the stable flag, not s->conn — teardown nulls @@ -327,4 +333,3 @@ void http3_stream_release(http3_stream_t *s) * the callback fires later. */ http_request_destroy(s->request); } - diff --git a/src/http_request.c b/src/http_request.c index 29499da9..4b819eaa 100644 --- a/src/http_request.c +++ b/src/http_request.c @@ -943,7 +943,7 @@ void http_request_fill_access_rec(const http_request_t *req, { memset(rec, 0, sizeof *rec); - if (req == NULL || response_obj == NULL) { + if (req == NULL) { return; } @@ -958,13 +958,13 @@ void http_request_fill_access_rec(const http_request_t *req, rec->url_query = (q != NULL && q[1] != '\0') ? q + 1 : NULL; } - rec->status = http_response_get_status(response_obj); - rec->response_size = http_response_get_sent_body_size(response_obj); + rec->status = response_obj != NULL ? http_response_get_status(response_obj) : 0; + rec->response_size = response_obj != NULL ? http_response_get_sent_body_size(response_obj) : 0; /* OTel forbids error.type on a request that completed, so it is where the * record says the body stopped short while the status stays what the peer * was told. */ - if (UNEXPECTED(http_response_is_aborted(response_obj))) { + if (response_obj != NULL && UNEXPECTED(http_response_is_aborted(response_obj))) { rec->error_type = "response_aborted"; } diff --git a/src/http_response.c b/src/http_response.c index 92beb9cf..bbd45efe 100644 --- a/src/http_response.c +++ b/src/http_response.c @@ -1399,6 +1399,19 @@ static void http_response_stream_commit_once(zend_object *obj, #endif } +static void response_rollback_unstarted(zend_object *obj, http_response_object *response) +{ + if (response->stream_ops->is_started == NULL + || response->stream_ops->is_started(response->stream_ctx)) return; +#ifdef HAVE_HTTP_COMPRESSION + http_compression_reset_unstarted(obj); +#endif + response->streaming = false; + response->headers_sent = false; + response->committed = false; + response->declared_length = -1; +} + /* {{{ proto HttpResponse::write(string $chunk): static * * Streaming response — append a chunk to the outbound queue. First @@ -1540,6 +1553,7 @@ ZEND_METHOD(TrueAsync_HttpResponse, tryWrite) /* Nothing was queued, so the bytes reserved above are free again — the * handler is expected to offer the same chunk once there is room. */ response_release_declared_length(response, ZSTR_LEN(chunk)); + response_rollback_unstarted(Z_OBJ_P(ZEND_THIS), response); RETURN_FALSE; } @@ -1854,6 +1868,7 @@ ZEND_METHOD(TrueAsync_HttpResponse, tryWriteMessage) /* Nothing was queued: the offer can be made again, so the bytes are * not spent. */ response_release_declared_length(response, frame_len); + response_rollback_unstarted(Z_OBJ_P(ZEND_THIS), response); RETURN_FALSE; } diff --git a/src/http_server_class.c b/src/http_server_class.c index dea81814..339450ac 100644 --- a/src/http_server_class.c +++ b/src/http_server_class.c @@ -41,6 +41,7 @@ #endif #include "core/stats_registry.h" #include "core/response_wire.h" +#include "core/response_delivery.h" #include "core/stream_credit.h" #include "core/async_plain_event.h" /* async_coroutine_sleep_ms */ #include "log/http_log.h" @@ -3042,7 +3043,7 @@ static void http_server_reactor_h3_teardown(http_server_object *server) for (size_t i = 0; i < server->reactor_h3_listener_count; i++) { if (server->reactor_pool != NULL && server->reactor_h3_listeners[i].listener != NULL) { - reactor_pool_exec(server->reactor_pool, + reactor_pool_control_exec(server->reactor_pool, server->reactor_h3_listeners[i].reactor_id, reactor_h3_destroy_fn, server->reactor_h3_listeners[i].listener); @@ -3124,6 +3125,7 @@ static void http_server_reactor_pool_up(http_server_object *server, const int wo * SO_REUSEPORT onto another reactor routes back to its owner. Active only * with >1 reactor (the id is one byte, so cap at 256). */ const int real_reactors = reactor_pool_count(server->reactor_pool); + reactor_pool_set_drain_epilogue(http3_reactor_steer_flush_epilogue); if (http3_steer_init()) { http3_steer_set_active(real_reactors > 1 && real_reactors <= 256); @@ -3189,227 +3191,20 @@ static void http_server_reactor_pool_down(http_server_object *server) #endif } -#ifdef HAVE_HTTP_SERVER_HTTP3 -/* Per-worker FIFO of reverse-path posts that didn't fit the reactor mailbox; a - * 1 ms timer retries in order so the worker never blocks and a slot release - * can't overtake a wire of its stream. discard runs on give-up (NULL = drop); - * deadline_ns 0 = retry while the pool lives (releases), else a ~100 ms TTL. */ -typedef struct pending_post_s { - reactor_exec_fn fn; - void *arg; - void (*discard)(void *arg); - uint64_t deadline_ns; - int reactor; - struct pending_post_s *next; -} pending_post_t; - -ZEND_TLS pending_post_t *pending_post_head = NULL; -ZEND_TLS pending_post_t *pending_post_tail = NULL; -ZEND_TLS bool pending_post_armed = false; - -#define PENDING_WIRE_RETRY_MS 1u -#define PENDING_WIRE_TTL_NS (100ull * 1000000ull) - -static bool pending_post_arm_timer(void); - -static void pending_discard_wire(void *arg) -{ - response_wire_discard((response_wire_t *)arg); -} - -static void pending_post_flush(void) -{ - const uint64_t now = zend_hrtime(); - - while (pending_post_head != NULL) { - pending_post_t *const n = pending_post_head; - - const bool posted = g_reactor_pool != NULL - && reactor_pool_post_exec(g_reactor_pool, n->reactor, n->fn, n->arg); - - if (!posted) { - if (g_reactor_pool != NULL - && (n->deadline_ns == 0 || now < n->deadline_ns)) { - break; /* mailbox still full — keep order, retry next tick */ - } - if (n->discard != NULL) { - n->discard(n->arg); /* expired / pool gone → drop */ - } - } - - pending_post_head = n->next; - - if (pending_post_head == NULL) { - pending_post_tail = NULL; - } - - pefree(n, 1); - } - - if (pending_post_head != NULL && !pending_post_arm_timer()) { - /* can't schedule another tick — fail the whole queue now */ - while (pending_post_head != NULL) { - pending_post_t *const n = pending_post_head; - if (n->discard != NULL) { - n->discard(n->arg); - } - pending_post_head = n->next; - pefree(n, 1); - } - pending_post_tail = NULL; - } -} - -static void pending_post_timer_fn(zend_async_event_t *event, - zend_async_event_callback_t *callback, - void *result, zend_object *exception) -{ - (void)event; (void)callback; (void)result; - - pending_post_armed = false; /* one-shot: fired (auto-closes) */ - - if (exception != NULL) { - return; /* loop teardown — flush would re-arm on a dying reactor */ - } - - pending_post_flush(); -} - -static bool pending_post_arm_timer(void) -{ - if (pending_post_armed) { - return true; - } - - zend_async_timer_event_t *const t = - ZEND_ASYNC_NEW_TIMER_EVENT(PENDING_WIRE_RETRY_MS, /*periodic*/ false); - - if (UNEXPECTED(t == NULL)) { - zend_clear_exception(); - return false; - } - - zend_async_event_callback_t *const cb = - ZEND_ASYNC_EVENT_CALLBACK(pending_post_timer_fn); - - if (UNEXPECTED(cb == NULL || !t->base.add_callback(&t->base, cb))) { - if (cb != NULL) { - ZEND_ASYNC_EVENT_CALLBACK_RELEASE(cb); - } - t->base.dispose(&t->base); - return false; - } - - if (UNEXPECTED(!t->base.start(&t->base))) { - zend_async_callbacks_remove(&t->base, cb); - t->base.dispose(&t->base); - return false; - } - - pending_post_armed = true; - return true; -} - -static bool pending_post_defer(reactor_exec_fn fn, void *arg, - void (*discard)(void *arg), int reactor, - uint64_t deadline_ns) -{ - pending_post_t *const n = pemalloc(sizeof(*n), 1); - - n->fn = fn; - n->arg = arg; - n->discard = discard; - n->reactor = reactor; - n->deadline_ns = deadline_ns; - n->next = NULL; - - if (pending_post_tail != NULL) { - pending_post_tail->next = n; - } else { - pending_post_head = n; - } - pending_post_tail = n; - - if (!pending_post_arm_timer()) { - /* unlink what we just queued; caller handles the drop */ - if (pending_post_head == n) { - pending_post_head = NULL; - pending_post_tail = NULL; - } else { - pending_post_t *p = pending_post_head; - while (p->next != n) { - p = p->next; - } - p->next = NULL; - pending_post_tail = p; - } - pefree(n, 1); - return false; - } - - return true; -} -#endif /* HAVE_HTTP_SERVER_HTTP3 */ - -/* Worker response sink: post the rendered response back to the originating - * reactor for nghttp3 encode + send. Runs on the worker thread (handler - * coroutine or its dispose). reactor_id (echoed on the wire) selects the - * reverse channel; ownership of `rw` transfers to the reactor apply on - * success. A FULL wire that doesn't fit is dropped (the client times out); - * STREAM_* wires are ordered fragments, so they defer to the hidden retry - * timer instead — never blocking the worker thread. */ +/* Legacy/test sink. Live pool requests use response_delivery_post, whose + * sender coroutine waits for capacity before handing ownership to the reactor. */ static bool http_server_worker_response_sink(response_wire_t *rw, void *arg) { (void)arg; - #ifdef HAVE_HTTP_SERVER_HTTP3 - if (g_reactor_pool != NULL) { - const int reactor = (int)response_wire_reactor_id(rw); - const bool is_stream = response_wire_kind(rw) != RESPONSE_WIRE_FULL; - - /* Once anything is deferred, stream wires queue behind it so - * fragments never overtake each other. FULL wires are whole - * responses on their own streams — always try directly. */ - if ((!is_stream || pending_post_head == NULL) - && reactor_pool_post_exec(g_reactor_pool, reactor, - http3_reactor_apply_response, rw)) { - return true; /* the reactor owns rw now */ - } - - if (is_stream && pending_post_defer(http3_reactor_apply_response, rw, - pending_discard_wire, reactor, - zend_hrtime() + PENDING_WIRE_TTL_NS)) { - return true; /* parked; retried by the timer, ~100 ms TTL */ - } - } + if (g_reactor_pool != NULL + && reactor_pool_post_exec(g_reactor_pool, (int)response_wire_reactor_id(rw), + http3_reactor_apply_response, rw)) return true; #endif - - /* undeliverable: abandoning the credit unblocks the producer */ response_wire_discard(rw); return false; } -/* Post a non-wire reactor op (H3 slot release) via the same ordered FIFO, no - * TTL. False only if no retry timer can be armed, so the caller can fall back. */ -bool http_worker_reactor_post_release(int reactor, void (*fn)(void *arg), void *arg) -{ -#ifdef HAVE_HTTP_SERVER_HTTP3 - if (g_reactor_pool == NULL) { - return false; - } - - if (pending_post_head == NULL - && reactor_pool_post_exec(g_reactor_pool, reactor, fn, arg)) { - return true; - } - - return pending_post_defer(fn, arg, /*discard*/ NULL, reactor, /*deadline*/ 0); -#else - (void)reactor; (void)fn; (void)arg; - return false; -#endif -} - /* Suspend the current coroutine for `ms` on a one-shot timer; lets the worker * loop keep draining (mailbox, scheduler) while we wait. */ static void hot_reload_sleep_ms(zend_coroutine_t *co, const zend_ulong ms) @@ -3421,49 +3216,10 @@ static void hot_reload_sleep_ms(zend_coroutine_t *co, const zend_ulong ms) } } -#ifdef HAVE_HTTP_SERVER_HTTP3 -/* Flush deferred reverse posts before the worker loop dies — a parked slot - * release must land or the reactor's slab keeps a live slot. Bounded. */ -static void pending_post_drain(zend_coroutine_t *co) -{ - for (int spin = 0; spin < 400 && pending_post_head != NULL; spin++) { - pending_post_flush(); +/* Reliable admission fence: completion means the reactor passed the callback, + * never merely that its data mailbox refused a post. Cold shutdown only. */ +static void http_server_inbox_fence_cb(void *arg) { (void)arg; } - if (pending_post_head != NULL && co != NULL) { - hot_reload_sleep_ms(co, 5); - } - } -} -#else -static void pending_post_drain(zend_coroutine_t *co) { (void)co; } -#endif - -/* Reactor fence bookkeeping (#93): the decrement that reaches zero fires the - * retiring worker's trigger — from a reactor thread, or from the worker itself - * when a post could not be delivered. */ -typedef struct { - zend_atomic_int remaining; - zend_async_trigger_event_t *done; -} inbox_fence_t; - -static void http_server_inbox_fence_cb(void *arg) -{ - inbox_fence_t *const fence = arg; - - if (zend_atomic_int_fetch_sub(&fence->remaining, 1) == 1) { - fence->done->trigger(fence->done); - } -} - -/* Unpublish this worker's inbox before the worker dies (#93 reload): - * 1. retire the registry slot — no NEW pick can return the inbox; - * 2. fence every reactor — a dispatch that loaded the pointer just before the - * retire has finished by the time its reactor runs the fence (dispatch is - * synchronous on the reactor loop, the fence queues behind it); - * 3. wait out the residual mailbox — items already posted drain on our own - * loop into server_scope, where the normal stop-path drain awaits them. - * After this the clone's worker_inbox_free really does run with quiesced - * producers, which is the contract it always assumed. */ static void http_server_worker_inbox_retire(http_server_object *server) { if (server->worker_inbox == NULL || g_worker_registry == NULL @@ -3488,40 +3244,9 @@ static void http_server_worker_inbox_retire(http_server_object *server) const int n = g_reactor_pool != NULL ? reactor_pool_count(g_reactor_pool) : 0; - if (n > 0 && co != NULL) { - zend_async_trigger_event_t *done = ZEND_ASYNC_NEW_TRIGGER_EVENT(); - - if (done != NULL) { - inbox_fence_t *fence = pemalloc(sizeof(*fence), 1); - fence->done = done; - ZEND_ATOMIC_INT_INIT(&fence->remaining, n); - - /* Subscribe BEFORE posting: a fence that fires instantly resolves - * the waker and the suspend below returns immediately. */ - zend_async_resume_when(co, &done->base, false, - zend_async_waker_callback_resolve, NULL); - - for (int i = 0; i < n; i++) { - if (!reactor_pool_post_exec(g_reactor_pool, i, - http_server_inbox_fence_cb, fence)) { - /* Undeliverable (reactor gone / mailbox full): take over its - * decrement; fire ourselves on the zero transition. */ - if (zend_atomic_int_fetch_sub(&fence->remaining, 1) == 1) { - done->trigger(done); - } - } - } - - ZEND_ASYNC_SUSPEND(); - ZEND_ASYNC_WAKER_DESTROY(co); - - if (EG(exception)) { - zend_clear_exception(); - } - - done->base.dispose(&done->base); - pefree(fence, 1); - } + for (int i = 0; i < n; i++) { + (void)reactor_pool_control_exec(g_reactor_pool, i, + http_server_inbox_fence_cb, NULL); } /* Residual items already in the mailbox drain on our loop while we sleep; @@ -3533,7 +3258,7 @@ static void http_server_worker_inbox_retire(http_server_object *server) } /* land reverse posts those residual requests deferred (esp. slot releases) */ - pending_post_drain(co); + http_logf_info(&server->log_state, "reload.inbox retired depth=%zu", worker_inbox_depth(server->worker_inbox)); @@ -3641,9 +3366,18 @@ static void http_server_worker_inbox_up(http_server_object *server) return; } + response_delivery_owner_t *owner = response_delivery_owner_create(server, g_reactor_pool); + if (owner == NULL) { + worker_inbox_free(server->worker_inbox); + server->worker_inbox = NULL; + return; + } + worker_inbox_set_delivery_owner(server->worker_inbox, owner); + const int slot = worker_registry_add(g_worker_registry, server->worker_inbox); if (slot < 0) { + response_delivery_owner_close(owner); worker_inbox_free(server->worker_inbox); server->worker_inbox = NULL; return; @@ -4907,6 +4641,8 @@ static void http_server_start_run(INTERNAL_FUNCTION_PARAMETERS) * on the start() coroutine and can suspend, so server_scope is empty when * the object is freed (issue #74). On bailout we already longjmp'd above. */ http_server_drain_scope(server); + response_delivery_owner_close(worker_inbox_delivery_owner(server->worker_inbox)); + http_log_server_stop(&server->log_state); /* do_stop retired the connections already; one it left busy may be idle * now that the drain has ended the handlers, and closes here. */ @@ -5017,7 +4753,8 @@ static void http_server_do_stop(http_server_object *server, const char *reason) http_logf_info(&server->log_state, "server.stop"); } - http_log_server_stop(&server->log_state); + if (worker_inbox_delivery_owner(server->worker_inbox) == NULL) + http_log_server_stop(&server->log_state); /* Resolve wait event to wake up start(). The event is released by the * server coroutine's waker_clean on resume — do NOT dispose here, that @@ -6422,9 +6159,11 @@ static void http_server_free(zend_object *obj) pool_ctl_release(server->pool_ctl); server->pool_ctl = NULL; - /* Defensive on the path where the user dropped the server without - * calling stop(); idempotent otherwise. */ - http_log_server_stop(&server->log_state); + if (server->worker_inbox != NULL) http_server_worker_inbox_retire(server); + + /* Pool delivery logging is drained after cancellation of the scope below. */ + if (worker_inbox_delivery_owner(server->worker_inbox) == NULL) + http_log_server_stop(&server->log_state); #ifdef HAVE_HTTP_SERVER_HTTP3 /* alt_svc_header_value is allocated in start() and normally released @@ -6490,6 +6229,9 @@ static void http_server_free(zend_object *obj) } #endif + worker_inbox_cancel_pending(server->worker_inbox); + response_delivery_owner_close(worker_inbox_delivery_owner(server->worker_inbox)); + http_log_server_stop(&server->log_state); zval_ptr_dtor(&server->config); /* Reactor pool. Normally torn down in the start_pool cleanup path; this is diff --git a/src/log/http_log.c b/src/log/http_log.c index f65b8d19..4841b31f 100644 --- a/src/log/http_log.c +++ b/src/log/http_log.c @@ -2286,7 +2286,7 @@ void http_log_emit_access(http_log_state_t *state, const http_access_rec_t *ar) .v.s = ar->url_query }; } - attrs[n++] = (http_log_attr_t){ .key = "http.response.status_code", + if (ar->status != 0) attrs[n++] = (http_log_attr_t){ .key = "http.response.status_code", .type = HTTP_LOG_ATTR_I64, .v.i64 = ar->status }; @@ -2326,7 +2326,9 @@ void http_log_emit_access(http_log_state_t *state, const http_access_rec_t *ar) } char body[512]; - int blen = snprintf(body, sizeof body, "%s %s %d", method, path, ar->status); + int blen = ar->status != 0 + ? snprintf(body, sizeof body, "%s %s %d", method, path, ar->status) + : snprintf(body, sizeof body, "%s %s", method, path); if (blen < 0) { return; diff --git a/tests/h3client/h3client.c b/tests/h3client/h3client.c index addbc5d2..6d5c3cc0 100644 --- a/tests/h3client/h3client.c +++ b/tests/h3client/h3client.c @@ -579,6 +579,26 @@ static int rotate_dcid(h3c_t *c) { /* ----- main ----- */ +static void close_client(h3c_t *c) +{ + if (getenv("H3CLIENT_NO_CLOSE") == NULL) { + ngtcp2_ccerr ccerr; + ngtcp2_ccerr_set_application_error(&ccerr, /* H3_NO_ERROR */ 0x100, NULL, 0); + ngtcp2_path_storage ps2 = {0}; + ngtcp2_path_storage_init(&ps2, + (struct sockaddr *)&c->local, c->local_len, + (struct sockaddr *)&c->remote, c->remote_len, NULL); + ngtcp2_pkt_info pi2 = {0}; + uint8_t cbuf[1280]; + ngtcp2_ssize cn = ngtcp2_conn_write_connection_close( + c->qc, &ps2.path, &pi2, cbuf, sizeof(cbuf), &ccerr, now_ns()); + if (cn > 0) { + (void)sendto(c->fd, cbuf, (size_t)cn, 0, + (struct sockaddr *)&c->remote, c->remote_len); + } + } +} + int main(int argc, char **argv) { if (argc < 4) { fprintf(stderr, "usage: %s [ []]\n", argv[0]); @@ -798,6 +818,10 @@ int main(int argc, char **argv) { unsigned long completed = 0; bool sent = false; + bool cancelled = false; + uint64_t request_sent_ns = 0; + const char *cancel_env = getenv("H3CLIENT_CANCEL_AFTER_MS"); + unsigned long cancel_ms = cancel_env != NULL ? strtoul(cancel_env, NULL, 10) : 0; uint64_t deadline_ns = now_ns() + deadline_ms * 1000000ull; while (completed < request_count && now_ns() < deadline_ns) { @@ -815,10 +839,18 @@ int main(int argc, char **argv) { fprintf(stderr, "submit_request failed\n"); return 1; } sent = true; + request_sent_ns = now_ns(); c.req_body_offset = 0; /* re-read body for next request */ continue; } + if (sent && cancel_ms > 0 && now_ns() - request_sent_ns >= cancel_ms * 1000000ull) { + close_client(&c); + fprintf(stderr, "EARLY_CLOSE\n"); + cancelled = true; + break; + } + /* If the current request finished, snapshot it, reset per-request * state, and either move to the next request or break out. */ if (c.response_done) { @@ -921,9 +953,20 @@ int main(int argc, char **argv) { } } if (conn_done) break; + if (c.response_done) { + const char *delay_env = getenv("H3CLIENT_DELAY_FINAL_ACK_MS"); + if (delay_env != NULL) { + const unsigned long delay_ms = strtoul(delay_env, NULL, 10); + if (delay_ms > 0 && delay_ms <= 10000) { + fprintf(stderr, "ACK_WAITING\n"); + (void)poll(NULL, 0, (int)delay_ms); + (void)ngtcp2_conn_handle_expiry(c.qc, now_ns()); + } + } + } } - if (completed < request_count) { + if (completed < request_count && !cancelled) { fprintf(stderr, "h3client: timeout (completed=%lu of %lu)\n", completed, request_count); return 1; @@ -933,6 +976,16 @@ int main(int argc, char **argv) { fprintf(stderr, "COMPLETED=%lu\n", completed); } + /* response_done is receive EOF, not "our final delayed ACK was sent". + * Closing immediately can abandon an intact response without acknowledging + * its FIN. Service the delayed-ACK timer before graceful CONNECTION_CLOSE. + * NO_FINAL_ACK deliberately exercises that failure in delivery tests. */ + if (!cancelled && getenv("H3CLIENT_NO_FINAL_ACK") == NULL) { + (void)poll(NULL, 0, 30); + (void)ngtcp2_conn_handle_expiry(c.qc, now_ns()); + (void)drain_out(&c); + } + /* Step 6a — emit a graceful application-error CONNECTION_CLOSE so * the server-side reaper has something to react to (otherwise the * conn would only be torn down by the 30s idle timer). Best-effort: @@ -940,22 +993,7 @@ int main(int argc, char **argv) { * * `H3CLIENT_NO_CLOSE=1` skips the emit so test harnesses can drive * the server's idle-timeout path (Step 6e). */ - if (getenv("H3CLIENT_NO_CLOSE") == NULL) { - ngtcp2_ccerr ccerr; - ngtcp2_ccerr_set_application_error(&ccerr, /* H3_NO_ERROR */ 0x100, NULL, 0); - ngtcp2_path_storage ps2 = {0}; - ngtcp2_path_storage_init(&ps2, - (struct sockaddr *)&c.local, c.local_len, - (struct sockaddr *)&c.remote, c.remote_len, NULL); - ngtcp2_pkt_info pi2 = {0}; - uint8_t cbuf[1280]; - ngtcp2_ssize cn = ngtcp2_conn_write_connection_close( - c.qc, &ps2.path, &pi2, cbuf, sizeof(cbuf), &ccerr, now_ns()); - if (cn > 0) { - (void)sendto(c.fd, cbuf, (size_t)cn, 0, - (struct sockaddr *)&c.remote, c.remote_len); - } - } + if (!cancelled) close_client(&c); if (c.retired_fd >= 0) { close(c.retired_fd); } diff --git a/tests/phpt/server/h3/050-h3-reactor-pool-mailbox-overflow.phpt b/tests/phpt/server/h3/050-h3-reactor-pool-mailbox-overflow.phpt index b127545c..f4a37e4a 100644 --- a/tests/phpt/server/h3/050-h3-reactor-pool-mailbox-overflow.phpt +++ b/tests/phpt/server/h3/050-h3-reactor-pool-mailbox-overflow.phpt @@ -1,5 +1,5 @@ --TEST-- -HttpServer: reverse-mailbox overflow defers wires + slot release, body stays byte-exact (#106 follow-up) +HttpServer: reverse-mailbox overflow suspends senders without dropping wires, body stays byte-exact (#106 follow-up) --EXTENSIONS-- true_async_server true_async @@ -42,12 +42,8 @@ $config = (new HttpServerConfig()) ->addHttp3Listener('127.0.0.1', $port) ->setCertificate($cert)->setPrivateKey($key) ->setWorkers(2) - /* Floor the reverse mailbox at its minimum (64). A 150-chunk reply cannot - * fit, so most STREAM_CHUNKs and the final stream-slot release take the - * worker's defer/retry FIFO instead of a direct post — exercising - * pending_post_defer / pending_post_flush and the consumed-release-through- - * FIFO path. A byte-exact, in-order body proves the FIFO holds ordering - * under overflow and no slot leaks at teardown. */ + /* A small mailbox exercises coroutine backpressure; each accepted wire + * pins the delivery until apply, independently of request-slot release. */ ->setReactorMailboxCapacity(64); $server = new HttpServer($config); $server->addHttpHandler(function ($req, $res) use ($chunks) { diff --git a/tests/phpt/server/h3/053-h3-pool-sendfile-status-accounting.phpt b/tests/phpt/server/h3/053-h3-pool-sendfile-status-accounting.phpt index 3d942ffd..e026ecf6 100644 --- a/tests/phpt/server/h3/053-h3-pool-sendfile-status-accounting.phpt +++ b/tests/phpt/server/h3/053-h3-pool-sendfile-status-accounting.phpt @@ -1,5 +1,5 @@ --TEST-- -sendFile() accounting (#5): reactor-pool ranged send is counted as 206 by the reactor +sendFile() accounting (#5): reactor-pool ranged send reports its final 206 to the worker --EXTENSIONS-- true_async_server true_async @@ -109,13 +109,12 @@ spawn(function () use ($server, $port, $client_bin, $bigBody, $logPath) { echo 'total=', ($t['total_requests'] === 3 ? 1 : 0), "\n"; echo 'class_sum=', ($classes === $t['total_requests'] ? 1 : 0), "\n"; echo '2xx=', ($t['responses_2xx_total'] === 3 ? 1 : 0), "\n"; - /* The reactor is where they were counted — it must be the one reporting them. */ + /* The reactor publishes its final status; the worker accounts it once. */ $reactor_total = 0; foreach ($stats['reactors'] ?? [] as $r) { $reactor_total += $r['total_requests']; } - echo 'from_reactors=', ($reactor_total === 3 ? 1 : 0), "\n"; + echo 'from_reactors=', ($reactor_total === 0 ? 1 : 0), "\n"; - /* The record can only come from the reactor: the worker marshalled the send - * and never saw the final status. Give the sink's flush timer a tick. */ + /* The completion snapshot lets the worker log the final reactor status. */ usleep(400000); $seen = []; foreach (explode("\n", trim((string)@file_get_contents($logPath))) as $line) { diff --git a/tests/phpt/server/h3/082-h3-delivery-backpressure.phpt b/tests/phpt/server/h3/082-h3-delivery-backpressure.phpt new file mode 100644 index 00000000..3ad9bc8d --- /dev/null +++ b/tests/phpt/server/h3/082-h3-delivery-backpressure.phpt @@ -0,0 +1,123 @@ +--TEST-- +HTTP/3 pool (#351): FULL backpressure yields the coroutine; render/submit failures are not delivered responses +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable')) die('skip fault injection required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { + if ($req->getPath() === '/pressure') { + _http_fault_enable('h3/reverse/queue_full', false); + /* This task must run on the SAME worker while the FULL sender waits. + * Ending the sender early cancels its request scope and this task. */ + spawn(function () use ($tmp) { + delay(40); + file_put_contents("$tmp/progress", 'yes'); + _http_fault_disable('h3/reverse/queue_full'); + }); + } + if ($req->getPath() === '/held') { + static $held = []; + $held[] = $req; /* lastref must not be the terminal-outcome signal */ + } + $res->setBody('body-' . $req->getPath()); +}); +$client = __DIR__ . '/../../../h3client/h3client'; + +spawn(function () use ($server, $port, $client, $tmp) { + delay(600); + foreach (['pressure', 'render', 'submit', 'held'] as $path) { + if ($path === 'render') _http_fault_enable('h3/delivery/render_failed', true); + if ($path === 'submit') _http_fault_enable('h3/delivery/submit_failed', true); + $out = (string) shell_exec('H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /' . $path . ' GET 2>&1'); + if ($path === 'pressure' || $path === 'held') { + echo "$path: ", str_contains($out, 'STATUS=200') + && str_contains($out, 'body-/' . $path) ? 'intact' : 'FAIL', "\n"; + } else { + echo "$path: ", !str_contains($out, 'STATUS=200') + && !str_contains($out, 'timeout') ? 'terminated' : 'FAIL', "\n"; + } + } + echo 'same-worker progress: ', is_file("$tmp/progress") ? 'yes' : 'FAIL', "\n"; + echo 'FULL exercised: ', _http_fault_hits('h3/reverse/queue_full') > 1 ? 'yes' : 'FAIL', "\n"; + for ($i = 0; $i < 100; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 4) break; + delay(10); + } + echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' undelivered=', $t['responses_undelivered_total'], + ' dropped=', $t['worker_wire_dropped_total'], "\n"; + $server->stop(); +}); +$server->start(); + +$rows = []; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $r = json_decode($line, true); + $a = $r['Attributes'] ?? []; + $rows[$a['url.path'] ?? ''][] = $a; +} +foreach (['pressure', 'render', 'submit', 'held'] as $path) { + $records = $rows['/' . $path] ?? []; + $a = $records[0] ?? []; + $ok = count($records) === 1; + if ($path === 'render' || $path === 'submit') { + $ok = $ok && !isset($a['http.response.status_code']) + && ($a['error.type'] ?? '') === 'response_undelivered' + && ($a['http.response.body.size'] ?? -1) === 0; + } else { + $ok = $ok && ($a['http.response.status_code'] ?? 0) === 200 + && !isset($a['error.type']) + && ($a['http.response.body.size'] ?? -1) === strlen('body-/' . $path); + } + echo "log $path: ", $ok ? 'once, correct' : 'FAIL', "\n"; + if (!$ok) echo json_encode($records), "\n"; +} +?> +--EXPECTF-- +%Apressure: intact +render: terminated +submit: terminated +held: intact +same-worker progress: yes +FULL exercised: yes +completed=4 2xx=2 undelivered=2 dropped=1 +%Alog pressure: once, correct +log render: once, correct +log submit: once, correct +log held: once, correct diff --git a/tests/phpt/server/h3/083-h3-delivery-final-ack.phpt b/tests/phpt/server/h3/083-h3-delivery-final-ack.phpt new file mode 100644 index 00000000..5f05de79 --- /dev/null +++ b/tests/phpt/server/h3/083-h3-delivery-final-ack.phpt @@ -0,0 +1,69 @@ +--TEST-- +HTTP/3 pool (#351): enqueue/apply do not count a response before its final ACK +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { $res->setBody('ack-body'); }); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($server, $port, $client, $tmp) { + delay(600); + $proc = proc_open('H3CLIENT_DELAY_FINAL_ACK_MS=700 H3CLIENT_DEADLINE_MS=3000 ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' / GET', + [0 => ['file', '/dev/null', 'r'], 1 => ['file', "$tmp/body", 'w'], + 2 => ['file', "$tmp/stderr", 'w']], $pipes); + for ($i = 0; $i < 500; $i++) { + if (str_contains((string) @file_get_contents("$tmp/stderr"), 'ACK_WAITING')) break; + delay(5); + } + $t = $server->getStats()['totals']; + echo 'ACK delayed: ', str_contains((string) @file_get_contents("$tmp/stderr"), 'ACK_WAITING') + ? 'yes' : 'FAIL', "\n"; + echo 'before ACK: completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], "\n"; + proc_close($proc); + for ($i = 0; $i < 100; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 1) break; + delay(10); + } + echo 'after ACK: completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' undelivered=', $t['responses_undelivered_total'], "\n"; + echo 'body: ', file_get_contents("$tmp/body") === 'ack-body' ? 'intact' : 'FAIL', "\n"; + $server->stop(); +}); +$server->start(); +?> +--EXPECTF-- +%AACK delayed: yes +before ACK: completed=0 2xx=0 +after ACK: completed=1 2xx=1 undelivered=0 +body: intact +%A diff --git a/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt b/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt new file mode 100644 index 00000000..5f382d83 --- /dev/null +++ b/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt @@ -0,0 +1,93 @@ +--TEST-- +HTTP/3 pool (#351): a refused nonblocking first write is retryable; a queue deadline resets through control +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable')) die('skip fault injection required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(1) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + _http_fault_enable('h3/reverse/queue_full', false); + if ($req->getPath() === '/try') { + try { $offered = $res->tryWrite('must-not-leave'); } + finally { _http_fault_disable('h3/reverse/queue_full'); } + /* A refused first offer must not commit the headers or the codec. */ + $res->setHeader('x-first-refused', $offered ? 'FAIL' : 'yes'); + $res->write('accepted')->end(); + } else { + $res->setBody('must-not-leave'); + /* Leave FULL enabled: the sender's deadline must terminate the client + * via the separate control route, not another data-queue message. */ + } +}); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($server, $port, $client) { + delay(600); + $out = (string) shell_exec('H3CLIENT_PRINT_HEADERS=1 H3CLIENT_DEADLINE_MS=2500 ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /try GET 2>&1'); + echo 'nonblocking: ', str_contains($out, 'STATUS=200') + && str_contains($out, 'HDR x-first-refused: yes') + && str_contains($out, 'accepted') && !str_contains($out, 'must-not-leave') + ? 'retryable, intact' : 'FAIL', "\n"; + $out = (string) shell_exec('H3CLIENT_DEADLINE_MS=2500 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /timeout GET 2>&1'); + _http_fault_disable('h3/reverse/queue_full'); + echo 'deadline: ', !str_contains($out, 'STATUS=200') + && !str_contains($out, 'timeout (completed') ? 'terminated' : 'FAIL', "\n"; + for ($i = 0; $i < 100; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 2) break; + delay(10); + } + echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' undelivered=', $t['responses_undelivered_total'], + ' dropped=', $t['worker_wire_dropped_total'], "\n"; + $server->stop(); +}); +$server->start(); +$count = 0; $ok = false; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + if (($a['url.path'] ?? '') !== '/timeout') continue; + $count++; + $ok = !isset($a['http.response.status_code']) + && ($a['error.type'] ?? '') === 'response_undelivered' + && ($a['http.response.body.size'] ?? -1) === 0; +} +echo 'deadline log: ', $count === 1 && $ok ? 'once, no status, zero bytes' : 'FAIL', "\n"; +?> +--EXPECTF-- +%Anonblocking: retryable, intact +deadline: terminated +completed=2 2xx=1 undelivered=1 dropped=1 +%Adeadline log: once, no status, zero bytes diff --git a/tests/phpt/server/h3/085-h3-delivery-writers-early-close.phpt b/tests/phpt/server/h3/085-h3-delivery-writers-early-close.phpt new file mode 100644 index 00000000..b34fa20b --- /dev/null +++ b/tests/phpt/server/h3/085-h3-delivery-writers-early-close.phpt @@ -0,0 +1,107 @@ +--TEST-- +HTTP/3 pool (#351): concurrent writers keep one ordered stream; early close waits for the sender snapshot +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable')) die('skip fault injection required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + if ($req->getPath() === '/early') { + delay(400); + $res->setBody('too late'); + return; + } + $res->setHeader('content-type', 'text/plain'); + _http_fault_enable('h3/reverse/queue_full', false); + $writers = []; + foreach (['A', 'B', 'C'] as $letter) { + $writers[] = spawn(function () use ($res, $letter) { + $chunk = str_repeat($letter, 4000); + if ($letter === 'A') $res->write($chunk); + else while (!$res->tryWrite($chunk)) { + if (!$res->awaitWritable(1000)) throw new RuntimeException('writer did not become ready'); + } + }); + if ($letter === 'A') delay(5); + } + spawn(function () { delay(40); _http_fault_disable('h3/reverse/queue_full'); }); + foreach ($writers as $writer) \Async\await($writer); + $res->end(); +}); +$client = __DIR__ . '/../../../h3client/h3client'; + +spawn(function () use ($server, $port, $client, $tmp) { + delay(600); + foreach (['plain', 'gzip'] as $path) { + $env = $path === 'gzip' ? 'H3CLIENT_HEADER=' . escapeshellarg('accept-encoding: gzip') . ' ' : ''; + $cmd = $env . 'H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /' . $path . ' GET 2>' . escapeshellarg("$tmp/stderr"); + $body = (string) shell_exec($cmd); + if ($path === 'gzip') $body = gzdecode($body); + $want = str_repeat('A', 4000) . str_repeat('B', 4000) . str_repeat('C', 4000); + echo "$path writers: ", $body === $want ? 'ordered, intact' : 'FAIL', "\n"; + } + $out = (string) shell_exec('H3CLIENT_CANCEL_AFTER_MS=40 H3CLIENT_DEADLINE_MS=1500 ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' ' . escapeshellarg('/early?q=1') . ' GET 2>&1'); + echo 'early close: ', str_contains($out, 'EARLY_CLOSE') ? 'yes' : 'FAIL', "\n"; + for ($i = 0; $i < 100; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 3) break; + delay(10); + } + echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' undelivered=', $t['responses_undelivered_total'], "\n"; + $server->stop(); +}); +$server->start(); +$count = 0; $ok = false; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + if (($a['url.path'] ?? '') !== '/early') continue; + $count++; + $ok = !isset($a['http.response.status_code']) + && ($a['error.type'] ?? '') === 'response_undelivered' + && ($a['http.response.body.size'] ?? -1) === 0 + && ($a['url.query'] ?? '') === 'q=1'; +} +echo 'early log: ', $count === 1 && $ok ? 'once, correct identity' : 'FAIL', "\n"; +?> +--EXPECTF-- +%Aplain writers: ordered, intact +gzip writers: ordered, intact +early close: yes +completed=3 2xx=2 undelivered=1 +%Aearly log: once, correct identity diff --git a/tests/phpt/server/h3/086-h3-delivery-wait-bailout.phpt b/tests/phpt/server/h3/086-h3-delivery-wait-bailout.phpt new file mode 100644 index 00000000..9fdd748f --- /dev/null +++ b/tests/phpt/server/h3/086-h3-delivery-wait-bailout.phpt @@ -0,0 +1,89 @@ +--TEST-- +HTTP/3 pool (#351): bailout with a linked capacity waiter retracts subscriptions and owned wire +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable')) die('skip fault injection required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { + if ($req->getPath() === '/bailout') { + _http_fault_enable('h3/reverse/queue_full', false); + _http_fault_enable('h3/delivery/wait_bailout', true); + $res->setBody('must-not-leave'); + } else $res->setBody('recovered'); +}); +$client = __DIR__ . '/../../../h3client/h3client'; + +spawn(function () use ($server, $port, $client, $tmp) { + delay(600); + foreach (['bailout', 'recovery'] as $path) { + $out = (string) shell_exec('H3CLIENT_DEADLINE_MS=2500 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /' . $path . ' GET 2>&1'); + _http_fault_disable('h3/reverse/queue_full'); + if ($path === 'bailout') { + echo 'bailout: ', !str_contains($out, 'STATUS=200') + && !str_contains($out, 'timeout') ? 'terminated' : 'FAIL', "\n"; + } else echo 'next request: ', str_contains($out, 'STATUS=200') + && str_contains($out, 'recovered') ? 'intact' : 'FAIL', "\n"; + } + echo 'linked wait fault: ', _http_fault_hits('h3/delivery/wait_bailout') === 1 ? 'once' : 'FAIL', "\n"; + for ($i = 0; $i < 100; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 2) break; + delay(10); + } + echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' undelivered=', $t['responses_undelivered_total'], "\n"; + $server->stop(); +}); +$server->start(); +$count = 0; $ok = false; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + if (($a['url.path'] ?? '') !== '/bailout') continue; + $count++; + $ok = !isset($a['http.response.status_code']) + && ($a['error.type'] ?? '') === 'response_undelivered' + && ($a['http.response.body.size'] ?? -1) === 0; +} +echo 'bailout log: ', $count === 1 && $ok ? 'once, correct' : 'FAIL', "\n"; +?> +--EXPECTF-- +%Abailout: terminated +next request: intact +linked wait fault: once +completed=2 2xx=1 undelivered=1 +%Abailout log: once, correct diff --git a/tests/phpt/server/telemetry/009-getstats-contract.phpt b/tests/phpt/server/telemetry/009-getstats-contract.phpt index 572f547b..bb008054 100644 --- a/tests/phpt/server/telemetry/009-getstats-contract.phpt +++ b/tests/phpt/server/telemetry/009-getstats-contract.phpt @@ -31,6 +31,7 @@ $expected = [ 'total_requests', 'responses_2xx_total', 'responses_3xx_total', 'responses_4xx_total', 'responses_5xx_total', 'responses_aborted_total', + 'responses_undelivered_total', 'conns_active_h1', 'conns_active_h2', 'conns_active_h3', 'log_records_dropped_total', 'static_zero_coroutine_total', 'static_cache_hits_total', 'static_cache_misses_total', diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 37875892..74f2c686 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -78,6 +78,7 @@ add_test(NAME ThreadQueue COMMAND test_thread_queue) # response; pure malloc-domain C, no PHP/Zend runtime. add_executable(test_response_wire core/test_response_wire.c + common/delivery_stubs.c ${CMAKE_SOURCE_DIR}/../src/core/response_wire.c ) diff --git a/tests/unit/common/delivery_stubs.c b/tests/unit/common/delivery_stubs.c new file mode 100644 index 00000000..0956f79f --- /dev/null +++ b/tests/unit/common/delivery_stubs.c @@ -0,0 +1,14 @@ +/* The flat-wire unit target never attaches a transport delivery. Reaching a + * delivery hook would exercise a subsystem this isolated target does not link. */ +#include "core/response_delivery.h" +#include +void response_delivery_wire_acquire(response_delivery_t *d) +{ + (void)d; + abort(); +} +void response_delivery_wire_release(response_delivery_t *d) +{ + (void)d; + abort(); +} From 39ced90d301d487b57e3d703ff26802142c2fcc2 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Tue, 6 Oct 2026 22:31:41 +0300 Subject: [PATCH 2/3] fix(core): join workers before shared transport teardown (#351) --- CHANGELOG.md | 2 +- docs/REACTOR_RESPONSE_DELIVERY.md | 17 + src/http_server_class.c | 291 +++++++++++++----- .../087-pool-partial-submit-shutdown.phpt | 29 ++ .../core/088-pool-worker-start-bailout.phpt | 32 ++ ...-h3-parent-cancellation-joins-workers.phpt | 69 +++++ ...-h3-parent-cancellation-during-reload.phpt | 77 +++++ .../089-h3-worker-start-bailout-cleanup.phpt | 48 +++ 8 files changed, 481 insertions(+), 84 deletions(-) create mode 100644 tests/phpt/server/core/087-pool-partial-submit-shutdown.phpt create mode 100644 tests/phpt/server/core/088-pool-worker-start-bailout.phpt create mode 100644 tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt create mode 100644 tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt create mode 100644 tests/phpt/server/h3/089-h3-worker-start-bailout-cleanup.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index cc2a8784..9259c1ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ 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. +- **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. - **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. diff --git a/docs/REACTOR_RESPONSE_DELIVERY.md b/docs/REACTOR_RESPONSE_DELIVERY.md index 145db6be..87392ae7 100644 --- a/docs/REACTOR_RESPONSE_DELIVERY.md +++ b/docs/REACTOR_RESPONSE_DELIVERY.md @@ -88,6 +88,18 @@ 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 @@ -105,4 +117,9 @@ TTL and its potentially lost release queue are gone. 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. diff --git a/src/http_server_class.c b/src/http_server_class.c index 339450ac..e3619173 100644 --- a/src/http_server_class.c +++ b/src/http_server_class.c @@ -18,6 +18,7 @@ #include "main/php_network.h" /* php_socket_t, SOCK_ERR, closesocket, php_socket_errno */ #include "Zend/zend_async_API.h" #include "core/bailout_guard.h" +#include "fiu-local.h" #include "Zend/zend_hrtime.h" #include "Zend/zend_enum.h" #include "php_http_server.h" @@ -2375,51 +2376,137 @@ typedef struct { int failed; /* workers whose submission was rejected */ zend_async_event_t *all_done; /* fires when pending == 0 */ zend_async_event_callback_t cb; /* embedded — recovered via offsetof */ + zend_function *protect; + zval drain_closure; + bool bailout; } pool_await_state_t; +static void pool_notify_quiesced(http_server_object *server, pool_await_state_t *st) +{ + if (st->pending == 0 && !server->reload_in_progress && st->all_done != NULL) { + ZEND_ASYNC_CALLBACKS_NOTIFY(st->all_done, NULL, NULL); + } +} + +/* Called directly for an already protected coroutine, otherwise by Async\protect. + * The parent owns all_done throughout the join; cancellation retracts only its + * subscription. Never retire shared transport from a cancelled wait alone. */ +static void http_server_pool_drain(http_server_object *server) +{ + pool_await_state_t *const st = server->pool_await_state; + zend_coroutine_t *const co = ZEND_ASYNC_CURRENT_COROUTINE; + + while (st->pending != 0 || server->reload_in_progress) { + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + ZEND_ASYNC_WAKER_NEW(co); + zend_async_resume_when(co, st->all_done, false, + zend_async_waker_callback_resolve, NULL); + if (st->pending != 0 || server->reload_in_progress) { + ZEND_ASYNC_SUSPEND(); + } + } zend_catch { + http_bailout_state_restore(&state); + st->bailout = true; + } zend_end_try(); + zend_async_waker_clean(co); + /* Protection defers ordinary cancellation. A forced scheduler unwind + * can still wake us: preserve the join condition, not the wake reason. */ + if (EG(exception) != NULL) zend_clear_exception(); + } +} + +static ZEND_NAMED_FUNCTION(http_server_pool_drain_closure) +{ + ZEND_PARSE_PARAMETERS_NONE(); + http_server_pool_drain(Z_HTTP_SERVER_P(ZEND_THIS)); + RETURN_NULL(); +} + +static bool http_server_pool_prepare_drain(pool_await_state_t *st, zval *this_zv) +{ + st->protect = zend_hash_str_find_ptr(EG(function_table), + ZEND_STRL("async\\protect")); + if (st->protect == NULL || ZEND_ASYNC_CURRENT_COROUTINE == NULL + || ZEND_ASYNC_WAKER_NEW(ZEND_ASYNC_CURRENT_COROUTINE) == NULL) { + zend_throw_exception(http_server_runtime_exception_ce, + "Cannot prepare worker pool shutdown", 0); + return false; + } + + zend_internal_function fn = {0}; + fn.type = ZEND_INTERNAL_FUNCTION; + fn.function_name = zend_string_init(ZEND_STRL("http_server_pool_drain"), 0); + fn.scope = http_server_ce; + fn.handler = http_server_pool_drain_closure; + zend_create_closure(&st->drain_closure, (zend_function *)&fn, + http_server_ce, http_server_ce, Z_OBJ_P(this_zv)); + zend_string_release(fn.function_name); + return EG(exception) == NULL; +} + static void pool_worker_handler(zend_async_event_t *event, void *vctx) { (void)event; pool_worker_ctx_t *wctx = (pool_worker_ctx_t *)vctx; - zval server_zv; - ZVAL_UNDEF(&server_zv); - ZEND_ASYNC_THREAD_LOAD_ZVAL_TOPLEVEL(&server_zv, &wctx->server_transit); - - if (EXPECTED(Z_TYPE(server_zv) == IS_OBJECT)) { - zend_call_method_with_0_params(Z_OBJ(server_zv), NULL, NULL, "start", NULL); - /* Exception in worker is captured by the future state via the - * ext/async dispatcher — clear locally so the handler returns - * cleanly. Log loudly first: a silently-cleared worker exception - * means one whole thread's worth of accept capacity is gone for - * the rest of the process lifetime, and we'd otherwise have no - * visibility into the loss. */ - if (UNEXPECTED(EG(exception))) { - zend_object *ex = EG(exception); - const char *cls = ZSTR_VAL(ex->ce->name); - zval *msg_zv = zend_read_property(ex->ce, ex, - "message", sizeof("message") - 1, - /*silent=*/1, NULL); - const char *msg = (msg_zv != NULL && Z_TYPE_P(msg_zv) == IS_STRING) - ? Z_STRVAL_P(msg_zv) : ""; - fprintf(stderr, - "[true-async-server] worker thread died: %s: %s\n", - cls, msg); - fflush(stderr); - zend_clear_exception(); + volatile zval server_slot; + zval *const server_zv = (zval *)&server_slot; + volatile bool bailout = false; + http_bailout_state_t state; + http_bailout_state_save(&state); + ZVAL_UNDEF(server_zv); + zend_try { + ZEND_ASYNC_THREAD_LOAD_ZVAL_TOPLEVEL(server_zv, &wctx->server_transit); + + if (EXPECTED(Z_TYPE_P(server_zv) == IS_OBJECT)) { + fiu_do_on("server/pool/worker_start_bailout", zend_bailout()); + zend_call_method_with_0_params(Z_OBJ_P(server_zv), NULL, NULL, "start", NULL); + /* Exception in worker is captured by the future state via the + * ext/async dispatcher — clear locally so the handler returns + * cleanly. Log loudly first: a silently-cleared worker exception + * means one whole thread's worth of accept capacity is gone for + * the rest of the process lifetime, and we'd otherwise have no + * visibility into the loss. */ + if (UNEXPECTED(EG(exception))) { + zend_object *ex = EG(exception); + const char *cls = ZSTR_VAL(ex->ce->name); + zval *msg_zv = zend_read_property(ex->ce, ex, + "message", sizeof("message") - 1, + /*silent=*/1, NULL); + const char *msg = (msg_zv != NULL && Z_TYPE_P(msg_zv) == IS_STRING) + ? Z_STRVAL_P(msg_zv) : ""; + fprintf(stderr, + "[true-async-server] worker thread died: %s: %s\n", + cls, msg); + fflush(stderr); + zend_clear_exception(); + } else { + /* normal on reload/stop; while running points at a swallowed bailout */ + fprintf(stderr, + "[true-async-server] worker exited\n"); + fflush(stderr); + } } else { - /* normal on reload/stop; while running points at a swallowed bailout */ fprintf(stderr, - "[true-async-server] worker exited\n"); + "[true-async-server] worker thread: server transfer failed\n"); fflush(stderr); } - } else { - fprintf(stderr, - "[true-async-server] worker thread: server transfer failed\n"); - fflush(stderr); - } - zval_ptr_dtor(&server_zv); + } zend_catch { + http_bailout_state_restore(&state); + bailout = true; + } zend_end_try(); + + /* Complete the clone destructor before the dispatcher can resolve this + * task. Do not catch-and-return from a bailout inside the destructor: that + * would falsely acknowledge quiescence with live gates/producers. */ + zval_ptr_dtor(server_zv); + if (bailout) { + zend_throw_exception(http_server_runtime_exception_ce, + "Worker server start bailed out", 0); + } /* ctx is part of the parent's pool_worker_ctx array; freed once * by http_server_free when the parent server destructs. */ } @@ -2464,7 +2551,10 @@ static void pool_worker_done_cb(zend_async_event_t *event, } } - if (--st->pending == 0 && st->all_done != NULL) { + --st->pending; + /* A reload reservation keeps pending positive until its cleanup. It sends + * the final notification after releasing its replacement shells. */ + if (st->pending == 0 && st->all_done != NULL) { ZEND_ASYNC_CALLBACKS_NOTIFY(st->all_done, NULL, NULL); } } @@ -3543,7 +3633,8 @@ static int http_server_start_pool(http_server_object *server, } pool_await_state_t *st = ecalloc(1, sizeof(*st)); - st->pending = workers; + st->pending = 0; + ZVAL_UNDEF(&st->drain_closure); st->all_done = create_server_wait_event(); st->cb.callback = pool_worker_done_cb; st->cb.dispose = pool_worker_cb_dispose; @@ -3555,9 +3646,16 @@ static int http_server_start_pool(http_server_object *server, server->pool_await_state = st; int rc = FAILURE; + zend_object *shutdown_exception = NULL; + bool shutdown_was_graceful = false; + bool submitted_all = true; + if (!http_server_pool_prepare_drain(st, this_zv)) goto cleanup; + st->pending = workers; for (int i = 0; i < workers; i++) { - zend_async_event_t *worker_evt = - pool->submit_internal(pool, pool_worker_handler, &ctxs[i]); + zend_async_event_t *worker_evt = NULL; + if (i == 0 || !fiu_fail("server/pool/partial_submit")) { + worker_evt = pool->submit_internal(pool, pool_worker_handler, &ctxs[i]); + } if (UNEXPECTED(worker_evt == NULL)) { /* submit_internal failed mid-loop. Already-submitted workers @@ -3566,6 +3664,7 @@ static int http_server_start_pool(http_server_object *server, * workers' callbacks fire NOTIFY on a still-valid st, then * fall through to the normal suspend/await path. */ st->pending -= (workers - i); + submitted_all = false; break; } @@ -3585,48 +3684,67 @@ static int http_server_start_pool(http_server_object *server, /* Suspend until all submitted workers report done. Skip the suspend * entirely when nothing got submitted — there is no callback to * notify all_done, so awaiting on it would deadlock. */ - if (st->pending > 0) { - zend_coroutine_t *coroutine = ZEND_ASYNC_CURRENT_COROUTINE; - - if (UNEXPECTED(ZEND_ASYNC_WAKER_NEW(coroutine) == NULL)) { - zend_throw_exception(http_server_runtime_exception_ce, - "Failed to create waker for pool parent", 0); - goto cleanup; - } - - zend_async_resume_when(coroutine, st->all_done, true, - zend_async_waker_callback_resolve, NULL); - ZEND_ASYNC_SUSPEND(); - /* resume_when(..., true) handed all_done to the waker; waker_clean - * disposes it. NULL our pointer so the cleanup dispose below is a no-op - * — disposing again is a use-after-free (same as server->wait_event). */ - st->all_done = NULL; + if (submitted_all && st->pending > 0) { + zend_coroutine_t *const coroutine = ZEND_ASYNC_CURRENT_COROUTINE; + http_bailout_state_t state; + http_bailout_state_save(&state); + zend_try { + zend_async_resume_when(coroutine, st->all_done, false, + zend_async_waker_callback_resolve, NULL); + ZEND_ASYNC_SUSPEND(); + } zend_catch { + http_bailout_state_restore(&state); + st->bailout = true; + } zend_end_try(); zend_async_waker_clean(coroutine); + } - if (EG(exception)) { - zend_clear_exception(); - } + if (EG(exception) != NULL) { + shutdown_exception = EG(exception); + shutdown_was_graceful = ZEND_ASYNC_GRACEFUL_SHUTDOWN; + EG(exception) = NULL; } /* start() answers "did this server serve": a run where every worker was * rejected before it reached accept() is a failure, however cleanly the * parent's await resolved. */ - rc = (st->pending == 0 && st->failed == 0) ? SUCCESS : FAILURE; + rc = (submitted_all && st->pending == 0 && st->failed == 0) ? SUCCESS : FAILURE; if (st->failed > 0) { http_logf_error(&server->log_state, "server.start.failed mode=pool workers=%d", st->failed); } - /* Workers still serving: the await was cancelled, not resolved - * (Async\graceful_shutdown()). Nothing in the engine stops a BUSY worker — - * closing the task channel only reaches one that is back at recv, and ours - * is inside start() — so they would outlive us and wedge the process. */ + /* Admission closes before any cleanup wait. An in-flight reload keeps its + * reservation until its own cleanup and cannot submit a replacement. */ + server->stopping = true; + http_server_hot_reload_down(server); if (st->pending > 0) { http_logf_info(&server->log_state, "server.stop mode=pool reason=shutdown"); pool_ctl_command(pool_ctl, POOL_CMD_STOP); } + if (st->pending != 0 || server->reload_in_progress) { + if (ZEND_COROUTINE_IS_PROTECTED(ZEND_ASYNC_CURRENT_COROUTINE)) { + http_server_pool_drain(server); + } else { + zval result; + ZVAL_UNDEF(&result); + zend_call_known_function(st->protect, NULL, NULL, &result, + 1, &st->drain_closure, NULL); + zval_ptr_dtor(&result); + } + if (EG(exception) != NULL) { + if (shutdown_exception == NULL) { + shutdown_exception = EG(exception); + shutdown_was_graceful = ZEND_ASYNC_GRACEFUL_SHUTDOWN; + EG(exception) = NULL; + } else { + zend_clear_exception(); + } + } + } + cleanup: /* Dispose the completion futures we own, else their cross-thread triggers * linger armed on our reactor. Live cohort, not `ctxs` — reload() swaps it. */ @@ -3653,7 +3771,9 @@ static int http_server_start_pool(http_server_object *server, /* Retire the hot-reload orchestrators before the pool machinery goes. */ http_server_hot_reload_down(server); - /* Workers have quiesced (the suspend returned). Tear down the transport + ZEND_ASSERT(st->pending == 0 && !server->reload_in_progress); + + /* Worker task completions, not cancellation, prove quiescence. Tear down the transport * reactor pool: this releases the H3 listeners the gated reactors own, * frees the worker registry, and stops the reactor loops. */ http_server_reactor_pool_down(server); @@ -3669,11 +3789,12 @@ static int http_server_start_pool(http_server_object *server, st->all_done->dispose(st->all_done); } + const bool bailout = st->bailout; + zval_ptr_dtor(&st->drain_closure); efree(st); ZEND_THREAD_POOL_DELREF(pool); - /* Drop the parent's ref. Workers cancelled out from under us (above) still - * hold theirs and free the channel when the last of them is gone. */ + /* Every worker task has completed; release the parent control reference. */ server->pool_ctl = NULL; pool_ctl_release(pool_ctl); @@ -3695,6 +3816,18 @@ static int http_server_start_pool(http_server_object *server, stopped->dispose(stopped); } + if (shutdown_exception != NULL) { + /* graceful_shutdown historically lets start() return after stopping. + * Keep that contract, but consume its cancellation only after join. */ + if (shutdown_was_graceful + && instanceof_function(shutdown_exception->ce, + ZEND_ASYNC_GET_CE(ZEND_ASYNC_EXCEPTION_CANCELLATION))) { + OBJ_RELEASE(shutdown_exception); + } else { + zend_throw_exception_internal(shutdown_exception); + } + } + if (bailout) zend_bailout(); return rc; } /* }}} */ @@ -3732,14 +3865,10 @@ static bool http_server_await_stop(http_server_object *server, bool *bailout) http_bailout_state_save(&bailout_state); zend_try { + if (server->is_worker_clone) { + fiu_do_on("server/pool/worker_wait_bailout", zend_bailout()); + } ZEND_ASYNC_SUSPEND(); - /* The waker owns the wait_event (resume_when took ownership) and - * waker_clean disposes it below — drop our pointer first so a late - * stop() on a coroutine torn down by cancellation (not by stop()) - * does not notify a freed event. */ - server->wait_event = NULL; - zend_async_waker_clean(coroutine); - if (EG(exception)) { zend_clear_exception(); } @@ -3748,6 +3877,10 @@ static bool http_server_await_stop(http_server_object *server, bool *bailout) bailed = true; } zend_end_try(); + /* Retire the owning subscription on normal and bailout exits alike. The + * clone destructor must never notify an event left in an abandoned waker. */ + server->wait_event = NULL; + zend_async_waker_clean(coroutine); *bailout = bailed; return true; } @@ -4910,6 +5043,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) pefree(fresh, 1); http_logf_info(&server->log_state, "reload.failed stage=transfer"); server->reload_in_progress = false; + pool_notify_quiesced(server, st); RETURN_THROWS(); } } @@ -4930,10 +5064,6 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) * shells; a follow-up reload() builds new ones and heals the cohort. */ st->pending -= workers; - if (st->pending == 0 && st->all_done != NULL) { - ZEND_ASYNC_CALLBACKS_NOTIFY(st->all_done, NULL, NULL); - } - for (int i = 0; i < workers; i++) { http_server_release_worker_shell(&fresh[i].server_transit); } @@ -4942,6 +5072,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) http_logf_info(&server->log_state, "reload.failed duration_ms=%lu", (unsigned long)(ZEND_ASYNC_NOW() - t0)); server->reload_in_progress = false; + pool_notify_quiesced(server, st); RETURN_THROWS(); } @@ -4951,10 +5082,6 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) if (UNEXPECTED(server->stopping)) { st->pending -= workers; - if (st->pending == 0 && st->all_done != NULL) { - ZEND_ASYNC_CALLBACKS_NOTIFY(st->all_done, NULL, NULL); - } - for (int i = 0; i < workers; i++) { http_server_release_worker_shell(&fresh[i].server_transit); } @@ -4962,6 +5089,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) pefree(fresh, 1); http_logf_info(&server->log_state, "reload.aborted reason=stop"); server->reload_in_progress = false; + pool_notify_quiesced(server, st); RETURN_FALSE; } @@ -4995,10 +5123,6 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) submitted++; } - if (st->pending == 0 && st->all_done != NULL) { - ZEND_ASYNC_CALLBACKS_NOTIFY(st->all_done, NULL, NULL); - } - /* The old cohort has exited: its consumed shells can be released now. The * fresh array takes their slot on the server (http_server_free releases * whichever array is current at destruction). */ @@ -5021,6 +5145,7 @@ ZEND_METHOD(TrueAsync_HttpServer, reload) http_logf_info(&server->log_state, "reload.done workers=%d/%d duration_ms=%lu", submitted, workers, (unsigned long)(ZEND_ASYNC_NOW() - t0)); server->reload_in_progress = false; + pool_notify_quiesced(server, st); RETURN_BOOL(submitted == workers); } /* }}} */ diff --git a/tests/phpt/server/core/087-pool-partial-submit-shutdown.phpt b/tests/phpt/server/core/087-pool-partial-submit-shutdown.phpt new file mode 100644 index 00000000..01380f2a --- /dev/null +++ b/tests/phpt/server/core/087-pool-partial-submit-shutdown.phpt @@ -0,0 +1,29 @@ +--TEST-- +HttpServer (#351): partial worker submission stops and joins the submitted tasks +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + +--FILE-- +addListener('127.0.0.1', tas_free_port()) + ->setWorkers(3)->setShutdownTimeout(0); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { $res->setBody('ok'); }); +_http_fault_enable('server/pool/partial_submit', true); +$result = $server->start(); +echo 'start=', $result ? 'true' : 'false', "\n"; +echo 'partial fault=', _http_fault_hits('server/pool/partial_submit'), "\n"; +_http_fault_disable('server/pool/partial_submit'); +echo "shutdown joined\n"; +?> +--EXPECTF-- +%Astart=false +partial fault=1 +shutdown joined diff --git a/tests/phpt/server/core/088-pool-worker-start-bailout.phpt b/tests/phpt/server/core/088-pool-worker-start-bailout.phpt new file mode 100644 index 00000000..97c6577c --- /dev/null +++ b/tests/phpt/server/core/088-pool-worker-start-bailout.phpt @@ -0,0 +1,32 @@ +--TEST-- +HttpServer (#351): an escaped worker start bailout cleans the clone and completes its task +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + +--FILE-- +addListener('127.0.0.1', tas_free_port()) + ->setWorkers(2)->setShutdownTimeout(0); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { $res->setBody('ok'); }); +_http_fault_enable('server/pool/worker_start_bailout', true); +spawn(function () use ($server) { delay(400); $server->stop(); }); +$result = $server->start(); +echo 'start=', $result ? 'true' : 'false', "\n"; +echo 'bailout fault=', _http_fault_hits('server/pool/worker_start_bailout'), "\n"; +_http_fault_disable('server/pool/worker_start_bailout'); +echo "worker completion observed\n"; +?> +--EXPECTF-- +%Astart=false +bailout fault=1 +worker completion observed diff --git a/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt b/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt new file mode 100644 index 00000000..cbd705e4 --- /dev/null +++ b/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt @@ -0,0 +1,69 @@ +--TEST-- +HTTP/3 pool (#351): repeated parent cancellation joins workers before transport teardown +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setShutdownTimeout(2); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { + file_put_contents("$tmp/busy", '1'); + delay(650); + $res->setBody('finished'); + file_put_contents("$tmp/handler-done", '1'); +}); +$ended = false; +$parent = spawn(function () use ($server, $tmp, &$ended) { + try { $server->start(); } + catch (Async\AsyncCancellation $e) {} + echo 'parent waited: ', file_exists("$tmp/handler-done") ? 'yes' : 'FAIL', "\n"; + $ended = true; +}); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($parent, $port, $client, $tmp, &$ended) { + delay(600); + exec('H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' / GET > ' . escapeshellarg("$tmp/client") . ' 2>&1 &'); + for ($i = 0; $i < 200 && !file_exists("$tmp/busy"); $i++) delay(10); + if (!file_exists("$tmp/busy")) { echo "no request\n"; $parent->cancel(); return; } + $parent->cancel(); + delay(80); + echo 'first cancellation: ', !$ended ? 'still joining' : 'FAIL', "\n"; + $parent->cancel(); + delay(80); + echo 'second cancellation: ', !$ended ? 'still joining' : 'FAIL', "\n"; + for ($i = 0; $i < 300 && !$ended; $i++) delay(10); + echo 'parent completed: ', $ended ? 'yes' : 'FAIL', "\n"; +}); +?> +--EXPECTF-- +%Afirst cancellation: still joining +second cancellation: still joining +%Aparent waited: yes +parent completed: yes diff --git a/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt b/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt new file mode 100644 index 00000000..090451b2 --- /dev/null +++ b/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt @@ -0,0 +1,77 @@ +--TEST-- +HTTP/3 pool (#351): parent cancellation waits for an in-flight reload reservation +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setShutdownTimeout(2); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { + file_put_contents("$tmp/busy", '1'); + delay(650); + $res->setBody('finished'); + file_put_contents("$tmp/handler-done", '1'); +}); +$ended = false; +$reloadEnded = false; +$parent = spawn(function () use ($server, $tmp, &$ended, &$reloadEnded) { + try { $server->start(); } + catch (Async\AsyncCancellation $e) {} + echo 'reload unwound: ', $reloadEnded ? 'yes' : 'FAIL', "\n"; + echo 'parent waited: ', file_exists("$tmp/handler-done") ? 'yes' : 'FAIL', "\n"; + $ended = true; +}); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($parent, $server, $port, $client, $tmp, &$ended, &$reloadEnded) { + delay(600); + exec('H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' / GET > ' . escapeshellarg("$tmp/client") . ' 2>&1 &'); + for ($i = 0; $i < 200 && !file_exists("$tmp/busy"); $i++) delay(10); + if (!file_exists("$tmp/busy")) { echo "no request\n"; $parent->cancel(); return; } + spawn(function () use ($server, &$reloadEnded) { + try { $server->reload(); } catch (Async\AsyncCancellation $e) {} + $reloadEnded = true; + }); + delay(30); + $parent->cancel(); + delay(80); + echo 'first cancellation: ', !$ended ? 'still joining' : 'FAIL', "\n"; + $parent->cancel(); + delay(80); + echo 'second cancellation: ', !$ended ? 'still joining' : 'FAIL', "\n"; + for ($i = 0; $i < 300 && !$ended; $i++) delay(10); + echo 'parent completed: ', $ended ? 'yes' : 'FAIL', "\n"; +}); +?> +--EXPECTF-- +%Afirst cancellation: still joining +second cancellation: still joining +%Areload unwound: yes +parent waited: yes +parent completed: yes diff --git a/tests/phpt/server/h3/089-h3-worker-start-bailout-cleanup.phpt b/tests/phpt/server/h3/089-h3-worker-start-bailout-cleanup.phpt new file mode 100644 index 00000000..0f9e7417 --- /dev/null +++ b/tests/phpt/server/h3/089-h3-worker-start-bailout-cleanup.phpt @@ -0,0 +1,48 @@ +--TEST-- +HTTP/3 pool (#351): a bailout from start after inbox publication completes clone teardown +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true]); +if (!function_exists('_http_fault_enable')) die('skip fault injection required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setShutdownTimeout(2); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) { $res->setBody('ok'); }); +_http_fault_enable('server/pool/worker_wait_bailout', true); +spawn(function () use ($server) { delay(400); $server->stop(); }); +$result = $server->start(); +echo 'start=', $result ? 'true' : 'false', "\n"; +echo 'start bailout=', _http_fault_hits('server/pool/worker_wait_bailout'), "\n"; +_http_fault_disable('server/pool/worker_wait_bailout'); +echo "worker cleanup completed\n"; +?> +--EXPECTF-- +%Astart=false +start bailout=1 +worker cleanup completed From 9999b8970ee7dc0799298b49840ea71a340df79a Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:19:10 +0300 Subject: [PATCH 3/3] fix(h3): cover delivery failures and preserve first gzip retries (#351) --- .github/workflows/build-linux.yml | 55 +++++++- CHANGELOG.md | 2 +- docs/REACTOR_RESPONSE_DELIVERY.md | 34 ++++- include/core/worker_dispatch.h | 7 +- scripts/lcov-add-hits.py | 13 ++ src/compression/http_compression_response.c | 34 ++++- src/core/reactor_pool_test_hooks.c | 54 +++++++- src/core/worker_dispatch.c | 13 +- src/http3/http3_static_response.c | 16 ++- tests/h3client/h3client.c | 23 ++-- .../084-h3-delivery-nonblocking-timeout.phpt | 45 +++++-- ...-h3-parent-cancellation-joins-workers.phpt | 7 +- ...-h3-parent-cancellation-during-reload.phpt | 7 +- ...ery-late-wires-and-aborted-accounting.phpt | 124 ++++++++++++++++++ ...091-h3-delivery-sendfile-pump-failure.phpt | 79 +++++++++++ ...092-h3-delivery-started-codec-refusal.phpt | 86 ++++++++++++ .../011-retired-inbox-releases-requests.phpt | 26 ++++ 17 files changed, 588 insertions(+), 37 deletions(-) create mode 100644 tests/phpt/server/h3/090-h3-delivery-late-wires-and-aborted-accounting.phpt create mode 100644 tests/phpt/server/h3/091-h3-delivery-sendfile-pump-failure.phpt create mode 100644 tests/phpt/server/h3/092-h3-delivery-started-codec-refusal.phpt create mode 100644 tests/phpt/server/reactor_pool/011-retired-inbox-releases-requests.phpt diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index faea4075..fbc2cad9 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -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 @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9259c1ed..975d9647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ 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 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. diff --git a/docs/REACTOR_RESPONSE_DELIVERY.md b/docs/REACTOR_RESPONSE_DELIVERY.md index 87392ae7..b8e5ed32 100644 --- a/docs/REACTOR_RESPONSE_DELIVERY.md +++ b/docs/REACTOR_RESPONSE_DELIVERY.md @@ -111,8 +111,9 @@ TTL and its potentially lost release queue are gone. - 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: refused nonblocking first offer is retryable; queue deadline resets - through control without data capacity. +- 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 @@ -123,3 +124,32 @@ TTL and its potentially lost release queue are gone. - 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. diff --git a/include/core/worker_dispatch.h b/include/core/worker_dispatch.h index 1a38681d..88596075 100644 --- a/include/core/worker_dispatch.h +++ b/include/core/worker_dispatch.h @@ -61,7 +61,7 @@ void worker_dispatch_cancel_request(struct http_request_t *req); * `own_scope` mirrors the H3 dispatch flag: true gives each request its own * request_context() subtree (a child of `scope`); false runs directly in * `scope`. When no handler is registered a 404 is synthesised so the sink still - * fires. Buffered responses go out as one FULL wire at dispose; a streaming + * fires. Buffered responses go out as one FULL wire at the sender tail; a streaming * response (send()/writeMessage()/SSE) is marshalled incrementally as * STREAM_HEADERS / STREAM_CHUNK / STREAM_END wires, paced by the per-stream * credit block (stream_credit.h) the reactor acknowledges against. @@ -76,4 +76,9 @@ bool worker_dispatch_request(http_server_object *server, bool own_scope, worker_response_sink_fn sink, void *sink_arg); +#ifdef HTTP_SERVER_TEST_HOOKS +/* ACK barrier for delivery regressions; plain native worker streams only. */ +zend_long worker_dispatch_test_acked_body(zend_object *response); +#endif + #endif /* WORKER_DISPATCH_H */ diff --git a/scripts/lcov-add-hits.py b/scripts/lcov-add-hits.py index 8ac722d1..5a9b017d 100755 --- a/scripts/lcov-add-hits.py +++ b/scripts/lcov-add-hits.py @@ -73,6 +73,8 @@ def main() -> int: ap.add_argument("base", type=Path, help="tracefile whose lines are counted") ap.add_argument("extra", type=Path, help="tracefile whose hits are added") ap.add_argument("-o", "--output", type=Path, required=True) + ap.add_argument("--require-source", action="append", default=[], + help="require hits on canonical lines of this source (suffix path)") args = ap.parse_args() for path in (args.base, args.extra): @@ -80,6 +82,17 @@ def main() -> int: print(f"error: {path} not found", file=sys.stderr) return 2 + if args.require_source: + canonical = read_hits(args.base) + extra = read_hits(args.extra) + for source in args.require_source: + matches = [path for path in canonical + if path == source or path.endswith("/" + source)] + if not any(any(extra.get(path, ({}, {}))[0].get(line, 0) > 0 + for line in canonical[path][0]) for path in matches): + print(f"error: no extra hits on canonical lines of {source}", file=sys.stderr) + return 2 + args.output.write_text(merge(args.base, args.extra)) return 0 diff --git a/src/compression/http_compression_response.c b/src/compression/http_compression_response.c index c3a84bd2..73081506 100644 --- a/src/compression/http_compression_response.c +++ b/src/compression/http_compression_response.c @@ -59,6 +59,8 @@ typedef struct { void *wrapper_ctx; /* ws_ctx_t — owned, freed at teardown */ bool wrapper_installed; bool wrapper_first_chunk; + zval rollback_vary; + zval rollback_length; } http_compression_state_t; /* Cross-TU contract for response-internal accessors. */ @@ -318,6 +320,8 @@ void http_compression_state_free(zend_object *response_obj) efree(st->wrapper_ctx); } + zval_ptr_dtor(&st->rollback_vary); + zval_ptr_dtor(&st->rollback_length); efree(st); http_response_set_compression_slot(response_obj, NULL); } @@ -580,6 +584,10 @@ static int forward_compressed(ws_ctx_t *w, zend_string *zs, const bool nonblocki * against a window the decoder never saw. A truncated body with a 499 is * recoverable; a corrupted stream is not. */ if (UNEXPECTED(rc == HTTP_STREAM_APPEND_BACKPRESSURE)) { + if (w->underlying_ops->is_started != NULL + && !w->underlying_ops->is_started(w->underlying_ctx)) { + return HTTP_STREAM_APPEND_BACKPRESSURE; /* fresh codec can be discarded */ + } return HTTP_STREAM_APPEND_STREAM_DEAD; } @@ -629,6 +637,14 @@ static int ws_append_chunk(void *ctx_opaque, zend_string *chunk, } if (UNEXPECTED(!w->first_chunk_done)) { + if (nonblocking) { + http_compression_state_t *const st = state_of(w->response_obj); + HashTable *const ht = http_response_get_headers(w->response_obj); + zval *prior = zend_hash_str_find(ht, ZEND_STRL("vary")); + if (prior != NULL) ZVAL_DUP(&st->rollback_vary, prior); + prior = zend_hash_str_find(ht, ZEND_STRL("content-length")); + if (prior != NULL) ZVAL_DUP(&st->rollback_length, prior); + } /* Header mutation deferred to first chunk: by now the handler * has finalised setHeader/setStatusCode (committed=true was set * by HttpResponse::send before we got here), and we know we @@ -868,7 +884,23 @@ void http_compression_reset_unstarted(zend_object *obj) http_compression_state_t *st = state_of(obj); if (st == NULL || !st->wrapper_installed) return; ws_ctx_t *w = st->wrapper_ctx; - ZEND_ASSERT(!w->first_chunk_done); + ZEND_ASSERT(w->underlying_ops->is_started == NULL + || !w->underlying_ops->is_started(w->underlying_ctx)); + if (w->first_chunk_done) { + HashTable *const ht = http_response_get_headers(obj); + /* decide() installs a wrapper only in the absence of user encoding. */ + zend_hash_str_del(ht, ZEND_STRL("content-encoding")); + if (Z_ISUNDEF(st->rollback_vary)) zend_hash_str_del(ht, ZEND_STRL("vary")); + else { + zend_hash_str_update(ht, ZEND_STRL("vary"), &st->rollback_vary); + ZVAL_UNDEF(&st->rollback_vary); /* ownership transferred before next allocation */ + } + if (Z_ISUNDEF(st->rollback_length)) zend_hash_str_del(ht, ZEND_STRL("content-length")); + else { + zend_hash_str_update(ht, ZEND_STRL("content-length"), &st->rollback_length); + ZVAL_UNDEF(&st->rollback_length); + } + } http_response_replace_stream_ops(obj, w->underlying_ops, w->underlying_ctx); if (st->encoder != NULL) http_compression_pool_release(st->encoder); st->encoder = NULL; diff --git a/src/core/reactor_pool_test_hooks.c b/src/core/reactor_pool_test_hooks.c index 6cc5f2bc..8ac5b28c 100644 --- a/src/core/reactor_pool_test_hooks.c +++ b/src/core/reactor_pool_test_hooks.c @@ -603,10 +603,18 @@ static bool inbox_probe_sink(response_wire_t *rw, void *arg) return true; } +static void inbox_cancel_release(http_request_t *req) +{ + int *const released = req->reactor_conn; + ++*released; + pefree(req, 1); +} + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_worker_inbox_selftest, 0, 2, MAY_BE_ARRAY | MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, server, IS_OBJECT, 0) ZEND_ARG_TYPE_INFO(0, count, IS_LONG, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, cancelPending, _IS_BOOL, 0, "false") ZEND_END_ARG_INFO() /* Drive the worker-inbox path: stand up a worker_inbox on this thread, post @@ -619,10 +627,14 @@ PHP_FUNCTION(_http_server_worker_inbox_selftest) { zval *server_zv; zend_long count; + bool cancel_pending = false; + int released = 0; - ZEND_PARSE_PARAMETERS_START(2, 2) + ZEND_PARSE_PARAMETERS_START(2, 3) Z_PARAM_OBJECT_OF_CLASS(server_zv, http_server_ce) Z_PARAM_LONG(count) + Z_PARAM_OPTIONAL + Z_PARAM_BOOL(cancel_pending) ZEND_PARSE_PARAMETERS_END(); if (count <= 0) { @@ -646,6 +658,17 @@ PHP_FUNCTION(_http_server_worker_inbox_selftest) RETURN_FALSE; } + /* Invalid admission must fail without allocating an inbox or accepting a + * request; NULL cleanup remains safe when startup never published one. */ + if (worker_inbox_create(NULL, ZEND_ASYNC_CURRENT_SCOPE, true, + inbox_probe_sink, &probe) != NULL + || worker_inbox_create(server, NULL, true, inbox_probe_sink, &probe) != NULL + || worker_inbox_post(NULL, NULL)) { + probe.done->dispose(probe.done); + RETURN_FALSE; + } + worker_inbox_free(NULL); + worker_inbox_t *const inbox = worker_inbox_create(server, ZEND_ASYNC_CURRENT_SCOPE, /*own_scope=*/true, inbox_probe_sink, &probe); @@ -666,12 +689,27 @@ PHP_FUNCTION(_http_server_worker_inbox_selftest) continue; } + if (cancel_pending) { + req->reactor_conn = &released; + req->release = inbox_cancel_release; + } if (!worker_inbox_post(inbox, req)) { http_request_destroy(req); /* full: backpressure, we keep ownership */ probe.expected--; } } + if (cancel_pending) { + worker_inbox_cancel_pending(inbox); + worker_inbox_free(inbox); + probe.done->dispose(probe.done); + array_init(return_value); + add_assoc_long(return_value, "expected", probe.expected); + add_assoc_long(return_value, "received", probe.received); + add_assoc_long(return_value, "released", released); + return; + } + /* Suspend until every dispatched handler has rendered its response; a * timeout timer keeps the loop alive and bounds a hang. */ zend_coroutine_t *const co = ZEND_ASYNC_CURRENT_COROUTINE; @@ -1374,7 +1412,21 @@ PHP_FUNCTION(_http_log_flood) RETURN_LONG((zend_long)http_server_counters(server)->log_records_dropped_total); } +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_response_acked_body, 0, 1, IS_LONG, 0) + ZEND_ARG_OBJ_INFO(0, response, TrueAsync\\HttpResponse, 0) +ZEND_END_ARG_INFO() + +PHP_FUNCTION(_http_server_response_acked_body) +{ + zval *response; + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_OBJECT_OF_CLASS(response, http_response_ce) + ZEND_PARSE_PARAMETERS_END(); + RETURN_LONG(worker_dispatch_test_acked_body(Z_OBJ_P(response))); +} + static const zend_function_entry reactor_pool_test_functions[] = { + ZEND_FE(_http_server_response_acked_body, arginfo_response_acked_body) ZEND_FE(_http_server_reactor_pool_selftest, arginfo_reactor_pool_selftest) ZEND_FE(_http_server_persistent_request_selftest, arginfo_persistent_request_selftest) ZEND_FE(_http_server_reactor_pool_exec_selftest, arginfo_reactor_pool_exec_selftest) diff --git a/src/core/worker_dispatch.c b/src/core/worker_dispatch.c index fc2eb499..39791086 100644 --- a/src/core/worker_dispatch.c +++ b/src/core/worker_dispatch.c @@ -608,7 +608,8 @@ static int worker_stream_append_impl(worker_dispatch_ctx_t *ctx, zend_string *ch /* headers undeliverable → the stream never opened; don't copy and * post a chunk wire the reactor would only throw away */ if (UNEXPECTED(!worker_wire_post(ctx, hw, nonblocking))) { - if (nonblocking) worker_encoded_offer_refused(ctx); + /* No header was accepted: a first nonblocking offer can discard + * its fresh codec and restore the uncommitted response. */ if (!ctx->stream_failed) { stream_credit_release(ctx->credit); ctx->credit = NULL; @@ -1367,3 +1368,13 @@ bool worker_dispatch_request(http_server_object *server, return true; } + +#ifdef HTTP_SERVER_TEST_HOOKS +zend_long worker_dispatch_test_acked_body(zend_object *response) +{ + if (http_response_get_stream_ops(response) != &worker_stream_ops) return -1; + const worker_dispatch_ctx_t *const ctx = http_response_get_stream_ctx(response); + return ctx != NULL && ctx->credit != NULL + ? (zend_long)stream_credit_acked(ctx->credit) : 0; +} +#endif diff --git a/src/http3/http3_static_response.c b/src/http3/http3_static_response.c index a41e0f33..221c2e4f 100644 --- a/src/http3/http3_static_response.c +++ b/src/http3/http3_static_response.c @@ -53,6 +53,8 @@ #include "http3_listener.h" #include "http3/http3_stream.h" +#include "fiu-local.h" + #include #include @@ -275,6 +277,13 @@ static void h3_static_finalize(h3_static_state_t *state) state->window_cb = NULL; } + if (state->pending_req != NULL) { + zend_async_io_req_t *const req = state->pending_req; + state->pending_req = NULL; + state->read_in_flight = false; + if (req->dispose != NULL) req->dispose(req); + } + if (state->pending_chunk != NULL) { /* Never pushed, so never charged to the budget — just drop it. */ zend_string_release(state->pending_chunk); @@ -422,7 +431,7 @@ static bool h3_static_submit_read(h3_static_state_t *state) /* The single decision point: finish, park, or read on. */ static void h3_static_try_read(h3_static_state_t *state) { - if (state->busy || state->done_fired) { + if (state->busy || state->done_fired || state->eof_reached) { return; } @@ -484,7 +493,10 @@ static void h3_static_read_dispatch(zend_async_event_t *event, state->read_in_flight = false; const ssize_t transferred = (ssize_t)req->transferred; - const bool err = (exception != NULL || req->exception != NULL); + bool err = (exception != NULL || req->exception != NULL); + if (state->bytes_sent > 0) { + fiu_do_on("h3/sendfile/read_failed_after_chunk", { err = true; }); + } if (req->exception != NULL) { OBJ_RELEASE(req->exception); diff --git a/tests/h3client/h3client.c b/tests/h3client/h3client.c index 6d5c3cc0..7f131515 100644 --- a/tests/h3client/h3client.c +++ b/tests/h3client/h3client.c @@ -182,18 +182,19 @@ static int h3_stop_sending(nghttp3_conn *conn, int64_t stream_id, return 0; } -/* The peer reset the stream: the transfer FAILED, however many bytes arrived. - * Say so — a caller that only looked at the status line and the body length - * would otherwise read a truncated response as a complete one. */ +/* A QUIC reset is a failed transfer even if headers/body arrived already. */ +static void mark_reset(h3c_t *c, int64_t stream_id, uint64_t err) +{ + if (stream_id != c->stream_id) return; + if (!c->stream_reset) fprintf(stderr, "RESET=%llu\n", (unsigned long long)err); + c->stream_reset = true; + c->response_done = true; +} + static int h3_reset_stream(nghttp3_conn *conn, int64_t stream_id, uint64_t err, void *cu, void *su) { (void)conn; (void)su; - h3c_t *c = cu; - if (stream_id == c->stream_id) { - fprintf(stderr, "RESET=%llu\n", (unsigned long long)err); - c->stream_reset = true; - c->response_done = true; - } + mark_reset(cu, stream_id, err); return 0; } @@ -260,8 +261,9 @@ static int stream_close_cb(ngtcp2_conn *qc, uint32_t flags, int64_t stream_id, static int stream_reset_cb(ngtcp2_conn *qc, int64_t stream_id, uint64_t fs, uint64_t err, void *user_data, void *stream_user_data) { - (void)qc; (void)fs; (void)err; (void)stream_user_data; + (void)qc; (void)fs; (void)stream_user_data; h3c_t *c = user_data; + mark_reset(c, stream_id, err); if (c->h3) nghttp3_conn_shutdown_stream_read(c->h3, stream_id); return 0; } @@ -341,6 +343,7 @@ static const nghttp3_callbacks H3_CB = { .end_stream = h3_end_stream, .reset_stream = h3_reset_stream, .recv_header = h3_recv_header, + .recv_trailer = h3_recv_header, }; /* Submit the GET/POST request. Called from main once nghttp3 is up. diff --git a/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt b/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt index 5f382d83..12f2c6ae 100644 --- a/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt +++ b/tests/phpt/server/h3/084-h3-delivery-nonblocking-timeout.phpt @@ -39,10 +39,19 @@ $server = new HttpServer($config); $server->addHttpHandler(function ($req, $res) { _http_fault_enable('h3/reverse/queue_full', false); if ($req->getPath() === '/try') { - try { $offered = $res->tryWrite('must-not-leave'); } + $res->setHeader('content-type', 'text/plain'); + $res->setHeader('vary', ['Origin', 'X-Probe']); + if ($req->getHeader('accept-encoding') === null) $res->setHeader('content-length', '8'); + $yielded = false; + spawn(function () use (&$yielded) { $yielded = true; }); + $start = hrtime(true); + try { $offered = $res->tryWrite('refused!'); } finally { _http_fault_disable('h3/reverse/queue_full'); } /* A refused first offer must not commit the headers or the codec. */ + $res->setHeader('x-rollback', $res->getHeaders()['vary'] === ['Origin', 'X-Probe'] + && $res->getHeader('content-length') === ($req->getHeader('accept-encoding') === null ? '8' : null) ? 'yes' : 'FAIL'); $res->setHeader('x-first-refused', $offered ? 'FAIL' : 'yes'); + $res->setHeader('x-nonblocking', !$yielded && (hrtime(true) - $start) < 250000000 ? 'yes' : 'FAIL'); $res->write('accepted')->end(); } else { $res->setBody('must-not-leave'); @@ -51,22 +60,31 @@ $server->addHttpHandler(function ($req, $res) { } }); $client = __DIR__ . '/../../../h3client/h3client'; -spawn(function () use ($server, $port, $client) { +spawn(function () use ($server, $port, $client, $tmp) { delay(600); - $out = (string) shell_exec('H3CLIENT_PRINT_HEADERS=1 H3CLIENT_DEADLINE_MS=2500 ' - . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /try GET 2>&1'); - echo 'nonblocking: ', str_contains($out, 'STATUS=200') - && str_contains($out, 'HDR x-first-refused: yes') - && str_contains($out, 'accepted') && !str_contains($out, 'must-not-leave') - ? 'retryable, intact' : 'FAIL', "\n"; + foreach (['plain', 'gzip'] as $codec) { + $env = $codec === 'gzip' ? 'H3CLIENT_HEADER=' . escapeshellarg('accept-encoding: gzip') . ' ' : ''; + $body = (string) shell_exec($env . 'H3CLIENT_PRINT_HEADERS=1 H3CLIENT_DEADLINE_MS=2500 ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /try GET 2>' . escapeshellarg("$tmp/try.stderr")); + $err = (string) file_get_contents("$tmp/try.stderr"); + if ($codec === 'gzip') $body = gzdecode($body); + echo "nonblocking $codec: ", str_contains($err, 'STATUS=200') + && str_contains($err, 'HDR x-first-refused: yes') + && str_contains($err, 'HDR x-nonblocking: yes') + && str_contains($err, 'HDR x-rollback: yes') + && ($codec !== 'gzip' || str_contains($err, 'HDR content-encoding: gzip')) + && $body === 'accepted' ? 'retryable, no yield, intact' : 'FAIL', "\n"; + } + $deadlineStart = hrtime(true); $out = (string) shell_exec('H3CLIENT_DEADLINE_MS=2500 ' . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /timeout GET 2>&1'); + $deadlineMs = (hrtime(true) - $deadlineStart) / 1000000; _http_fault_disable('h3/reverse/queue_full'); echo 'deadline: ', !str_contains($out, 'STATUS=200') - && !str_contains($out, 'timeout (completed') ? 'terminated' : 'FAIL', "\n"; + && !str_contains($out, 'timeout (completed') && $deadlineMs >= 800 && $deadlineMs < 2300 ? 'terminated at deadline' : 'FAIL', "\n"; for ($i = 0; $i < 100; $i++) { $t = $server->getStats()['totals']; - if ($t['total_requests'] === 2) break; + if ($t['total_requests'] === 3) break; delay(10); } echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], @@ -87,7 +105,8 @@ foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $ echo 'deadline log: ', $count === 1 && $ok ? 'once, no status, zero bytes' : 'FAIL', "\n"; ?> --EXPECTF-- -%Anonblocking: retryable, intact -deadline: terminated -completed=2 2xx=1 undelivered=1 dropped=1 +%Anonblocking plain: retryable, no yield, intact +nonblocking gzip: retryable, no yield, intact +deadline: terminated at deadline +completed=3 2xx=2 undelivered=1 dropped=1 %Adeadline log: once, no status, zero bytes diff --git a/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt b/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt index cbd705e4..cce31494 100644 --- a/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt +++ b/tests/phpt/server/h3/087-h3-parent-cancellation-joins-workers.phpt @@ -40,8 +40,10 @@ $server->addHttpHandler(function ($req, $res) use ($tmp) { }); $ended = false; $parent = spawn(function () use ($server, $tmp, &$ended) { + $cancelled = false; try { $server->start(); } - catch (Async\AsyncCancellation $e) {} + catch (Async\AsyncCancellation $e) { $cancelled = true; } + echo 'cancellation propagated: ', $cancelled ? 'yes' : 'FAIL', "\n"; echo 'parent waited: ', file_exists("$tmp/handler-done") ? 'yes' : 'FAIL', "\n"; $ended = true; }); @@ -65,5 +67,6 @@ spawn(function () use ($parent, $port, $client, $tmp, &$ended) { --EXPECTF-- %Afirst cancellation: still joining second cancellation: still joining -%Aparent waited: yes +%Acancellation propagated: yes +parent waited: yes parent completed: yes diff --git a/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt b/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt index 090451b2..e5a712fc 100644 --- a/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt +++ b/tests/phpt/server/h3/088-h3-parent-cancellation-during-reload.phpt @@ -41,8 +41,10 @@ $server->addHttpHandler(function ($req, $res) use ($tmp) { $ended = false; $reloadEnded = false; $parent = spawn(function () use ($server, $tmp, &$ended, &$reloadEnded) { + $cancelled = false; try { $server->start(); } - catch (Async\AsyncCancellation $e) {} + catch (Async\AsyncCancellation $e) { $cancelled = true; } + echo 'cancellation propagated: ', $cancelled ? 'yes' : 'FAIL', "\n"; echo 'reload unwound: ', $reloadEnded ? 'yes' : 'FAIL', "\n"; echo 'parent waited: ', file_exists("$tmp/handler-done") ? 'yes' : 'FAIL', "\n"; $ended = true; @@ -72,6 +74,7 @@ spawn(function () use ($parent, $server, $port, $client, $tmp, &$ended, &$reload --EXPECTF-- %Afirst cancellation: still joining second cancellation: still joining -%Areload unwound: yes +%Acancellation propagated: yes +reload unwound: yes parent waited: yes parent completed: yes diff --git a/tests/phpt/server/h3/090-h3-delivery-late-wires-and-aborted-accounting.phpt b/tests/phpt/server/h3/090-h3-delivery-late-wires-and-aborted-accounting.phpt new file mode 100644 index 00000000..b92e7049 --- /dev/null +++ b/tests/phpt/server/h3/090-h3-delivery-late-wires-and-aborted-accounting.phpt @@ -0,0 +1,124 @@ +--TEST-- +HTTP/3 pool (#351): late CHUNK, END, ABORT and SEND_FILE survive FULL; acknowledged abort is counted once +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable') || !function_exists('_http_server_response_acked_body')) die('skip fault injection and test hooks required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$fileBody = str_repeat('sendfile-0123456789', 8192); +file_put_contents("$tmp/body.bin", $fileBody); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { + $path = substr($req->getPath(), 1); + if ($path !== 'headers' && $path !== 'file') { + $res->write('prefix-'); + if ($path === 'abort') { + for ($i = 0; $i < 200 && _http_server_response_acked_body($res) < 7; $i++) delay(5); + if (_http_server_response_acked_body($res) !== 7) throw new RuntimeException('ACK barrier failed'); + } + } + _http_fault_enable('h3/reverse/queue_full', false); + spawn(function () use ($tmp, $path) { + delay(60); + file_put_contents("$tmp/released-$path", 'yes'); + _http_fault_disable('h3/reverse/queue_full'); + }); + if ($path === 'file') $res->sendFile("$tmp/body.bin", new TrueAsync\SendFileOptions( + contentType: 'application/x-delivery-test', downloadName: 'result.bin', + cacheControl: 'private, max-age=60')); + + elseif ($path === 'abort') $res->abort(258); + else { + if ($path === 'headers') $res->write('prefix-'); + if ($path === 'chunk') $res->write('chunk'); + $res->setTrailer('x-terminal', $path); + $res->end(); + } +}); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($server, $port, $client, $tmp, $fileBody) { + delay(600); + foreach (['headers', 'chunk', 'end', 'abort', 'file'] as $path) { + $out = (string) shell_exec('H3CLIENT_PRINT_HEADERS=1 H3CLIENT_DEADLINE_MS=3000 ' + . escapeshellarg($client) . ' 127.0.0.1 ' . $port . ' /' . $path + . ' GET 2>' . escapeshellarg("$tmp/stderr")); + $err = (string) file_get_contents("$tmp/stderr"); + $full = _http_fault_hits('h3/reverse/queue_full') > 0 + && file_exists("$tmp/released-$path"); + if ($path === 'abort') $ok = str_contains($err, 'RESET=258') && $out === 'prefix-'; + elseif ($path === 'file') $ok = $out === $fileBody + && str_contains($err, 'HDR content-type: application/x-delivery-test') + && str_contains($err, 'filename="result.bin"') + && str_contains($err, 'HDR cache-control: private, max-age=60'); + else $ok = $out === 'prefix-' . ($path === 'chunk' ? 'chunk' : '') + && str_contains($err, 'HDR x-terminal: ' . $path); + echo "$path: ", $full && $ok ? 'FULL exercised, correct' : 'FAIL', "\n"; + } + for ($i = 0; $i < 200; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 5) break; + delay(5); + } + echo 'completed=', $t['total_requests'], ' 2xx=', $t['responses_2xx_total'], + ' aborted=', $t['responses_aborted_total'], ' undelivered=', $t['responses_undelivered_total'], "\n"; + $server->stop(); +}); +$server->start(); +$rows = []; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + $rows[$a['url.path'] ?? ''][] = $a; +} +foreach (['headers', 'chunk', 'end', 'abort', 'file'] as $path) { + $a = $rows['/' . $path][0] ?? []; + $want = $path === 'file' ? strlen($fileBody) : ($path === 'chunk' ? 12 : 7); + $ok = count($rows['/' . $path] ?? []) === 1 && ($a['http.response.status_code'] ?? 0) === 200 + && ($a['http.response.body.size'] ?? -1) === $want + && ($path === 'abort' ? ($a['error.type'] ?? '') === 'response_aborted' : !isset($a['error.type'])); + echo "log $path: ", $ok ? 'once, correct' : 'FAIL', "\n"; +} +?> +--EXPECTF-- +%Aheaders: FULL exercised, correct +chunk: FULL exercised, correct +end: FULL exercised, correct +abort: FULL exercised, correct +file: FULL exercised, correct +completed=5 2xx=5 aborted=1 undelivered=0 +%Alog headers: once, correct +log chunk: once, correct +log end: once, correct +log abort: once, correct +log file: once, correct diff --git a/tests/phpt/server/h3/091-h3-delivery-sendfile-pump-failure.phpt b/tests/phpt/server/h3/091-h3-delivery-sendfile-pump-failure.phpt new file mode 100644 index 00000000..93ac2f97 --- /dev/null +++ b/tests/phpt/server/h3/091-h3-delivery-sendfile-pump-failure.phpt @@ -0,0 +1,79 @@ +--TEST-- +HTTP/3 pool (#351): sendFile pump failure after its first chunk resets and reports one failure +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable') || !function_exists('_http_server_response_acked_body')) die('skip fault injection and test hooks required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$fileBody = str_repeat('sendfile-0123456789', 8192); +file_put_contents("$tmp/body.bin", $fileBody); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { $res->sendFile("$tmp/body.bin"); }); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($server, $port, $client, $tmp, $fileBody) { + delay(600); + _http_fault_enable('h3/sendfile/read_failed_after_chunk', true); + $out = (string) shell_exec('H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /file GET 2>' . escapeshellarg("$tmp/stderr")); + $err = (string) file_get_contents("$tmp/stderr"); + echo 'pump fault=', _http_fault_hits('h3/sendfile/read_failed_after_chunk'), "\n"; + echo 'peer: ', str_contains($err, 'RESET=') && !str_contains($err, 'timeout') + && strlen($out) < strlen($fileBody) ? 'reset, incomplete' : 'FAIL', "\n"; + if (!str_contains($err, 'RESET=')) echo 'client diagnostic: ', $err, "\n"; + for ($i = 0; $i < 200; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 1) break; + delay(5); + } + echo 'outcome: ', $t['total_requests'] === 1 + && $t['responses_aborted_total'] + $t['responses_undelivered_total'] === 1 ? 'one failure' : 'FAIL', "\n"; + $server->stop(); +}); +$server->start(); +$rows = []; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + if (($a['url.path'] ?? '') === '/file') $rows[] = $a; +} +$a = $rows[0] ?? []; +$type = isset($a['http.response.status_code']) ? 'response_aborted' : 'response_undelivered'; +echo 'access: ', count($rows) === 1 && ($a['error.type'] ?? '') === $type + && ($a['http.response.body.size'] ?? -1) < strlen($fileBody) ? 'once, failed' : 'FAIL', "\n"; +?> +--EXPECTF-- +%Apump fault=1 +peer: reset, incomplete +outcome: one failure +%Aaccess: once, failed diff --git a/tests/phpt/server/h3/092-h3-delivery-started-codec-refusal.phpt b/tests/phpt/server/h3/092-h3-delivery-started-codec-refusal.phpt new file mode 100644 index 00000000..1dd037f1 --- /dev/null +++ b/tests/phpt/server/h3/092-h3-delivery-started-codec-refusal.phpt @@ -0,0 +1,86 @@ +--TEST-- +HTTP/3 pool (#351): refusing a chunk of an already started gzip stream terminates it instead of corrupting a retry +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + true, 'h3client' => true]); +if (!function_exists('_http_fault_enable') || !function_exists('_http_server_response_acked_body')) die('skip fault injection and test hooks required'); +?> +--ENV-- +TRUE_ASYNC_SERVER_REACTOR_POOL=1 +PHP_HTTP3_DISABLE_RETRY=1 +--FILE-- +addListener('127.0.0.1', $port + 1) + ->addHttp3Listener('127.0.0.1', $port) + ->setCertificate("$tmp/cert.pem")->setPrivateKey("$tmp/key.pem") + ->setWorkers(2)->setStatsEnabled(true)->setWriteTimeout(2) + ->setReactorMailboxCapacity(64) + ->setLogSinks([['type' => 'file', 'path' => "$tmp/access.log", 'format' => 'json', + 'category' => 'access', 'level' => LogSeverity::INFO]]); +$fileBody = str_repeat('sendfile-0123456789', 8192); +file_put_contents("$tmp/body.bin", $fileBody); +$server = new HttpServer($config); +$server->addHttpHandler(function ($req, $res) use ($tmp) { + $res->setHeader('content-type', 'text/plain')->write('prefix'); + _http_fault_enable('h3/reverse/queue_full', false); + try { + $res->tryWrite('rejected'); + file_put_contents("$tmp/refusal", 'FAIL'); + } catch (TrueAsync\HttpException $e) { + file_put_contents("$tmp/refusal", $e->getCode() === 499 ? '499' : 'FAIL'); + } finally { _http_fault_disable('h3/reverse/queue_full'); } +}); +$client = __DIR__ . '/../../../h3client/h3client'; +spawn(function () use ($server, $port, $client, $tmp) { + delay(600); + $out = (string) shell_exec('H3CLIENT_HEADER=' . escapeshellarg('accept-encoding: gzip') + . ' H3CLIENT_DEADLINE_MS=3000 ' . escapeshellarg($client) + . ' 127.0.0.1 ' . $port . ' /late GET 2>' . escapeshellarg("$tmp/stderr")); + $err = (string) file_get_contents("$tmp/stderr"); + echo 'late refusal: ', (string) @file_get_contents("$tmp/refusal") === '499' + && _http_fault_hits('h3/reverse/queue_full') > 0 ? '499, FULL exercised' : 'FAIL', "\n"; + echo 'peer: ', str_contains($err, 'RESET=') && !str_contains($err, 'timeout') ? 'reset' : 'FAIL', "\n"; + for ($i = 0; $i < 200; $i++) { + $t = $server->getStats()['totals']; + if ($t['total_requests'] === 1) break; + delay(5); + } + echo 'outcome: ', $t['total_requests'] === 1 + && $t['responses_aborted_total'] + $t['responses_undelivered_total'] === 1 ? 'one failure' : 'FAIL', "\n"; + $server->stop(); +}); +$server->start(); +$rows = []; +foreach (explode("\n", trim((string) file_get_contents("$tmp/access.log"))) as $line) { + $a = json_decode($line, true)['Attributes'] ?? []; + if (($a['url.path'] ?? '') === '/late') $rows[] = $a; +} +$a = $rows[0] ?? []; +$want = isset($a['http.response.status_code']) ? 'response_aborted' : 'response_undelivered'; +echo 'access: ', count($rows) === 1 && ($a['error.type'] ?? '') === $want ? 'once, failed' : 'FAIL', "\n"; +?> +--EXPECTF-- +%Alate refusal: 499, FULL exercised +peer: reset +outcome: one failure +%Aaccess: once, failed diff --git a/tests/phpt/server/reactor_pool/011-retired-inbox-releases-requests.phpt b/tests/phpt/server/reactor_pool/011-retired-inbox-releases-requests.phpt new file mode 100644 index 00000000..656b0867 --- /dev/null +++ b/tests/phpt/server/reactor_pool/011-retired-inbox-releases-requests.phpt @@ -0,0 +1,26 @@ +--TEST-- +Worker inbox (#351): retirement releases queued requests without invoking their handlers +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + +--FILE-- +addHttpHandler(function ($req, $res) use (&$called) { $called++; $res->setBody('bad'); }); +$result = await(spawn(function () use ($server) { + return _http_server_worker_inbox_selftest($server, 128, true); +})); +echo 'queued=', $result['expected'], ' dispatched=', $result['received'], + ' released=', $result['released'], ' handlers=', $called, "\n"; +?> +--EXPECT-- +queued=128 dispatched=0 released=128 handlers=0