diff --git a/docs/ISSUE_pool_shutdown_teardown_hang.md b/docs/ISSUE_pool_shutdown_teardown_hang.md new file mode 100644 index 00000000..ca1d83a1 --- /dev/null +++ b/docs/ISSUE_pool_shutdown_teardown_hang.md @@ -0,0 +1,171 @@ +# ISSUE: pool graceful-shutdown teardown hang (pre-existing, non-deterministic) + +**Status:** RESOLVED (root cause found 2026-07-09). Not a server-repo bug — a +**stale PHP binary**. The fix already exists in php-async (ext/async) commit +`937bcfe`, 2026-07-04, shipped in TrueAsync ABI v0.24.0. The environment's +installed `/usr/local/bin/php` is ABI v0.23.0 built 2026-07-03 — one day +*before* the fix. Rebuild/reinstall PHP (≥ v0.24.0); no server code change. +**Discovered:** 2026-07-09, during review-follow-up work on the merged gRPC branch. +**Severity:** flaky CI hang — the process never exits, so a naive `run-tests` +invocation wedges until an external timeout kills it (and leaves orphan PHP +children that `timeout` on the runner does NOT reap). + +--- + +## ROOT CAUSE (confirmed via gdb + A/B binary swap) + +The earlier "leftover libuv handle in the server's completion-future path" lead +was **wrong**. `gdb -p -batch -ex "thread apply all bt"` on a live +hang shows: + +- **Main thread** — `main → php_module_shutdown → libuv_reactor_quiesce → + uv_cond_wait(child_thread_registry_cond)`. It is NOT in `uv_run`; it is blocked + at module shutdown waiting for every child (worker) thread to deregister from + `child_thread_registry` (`ext/async/libuv_reactor.c`). +- **The 3 worker threads** — each still alive in its own scheduler loop + (`fiber_entry → libuv_reactor_execute → uv_run(UV_RUN_ONCE, timeout=200)`); one + caught mid `zif_Async_delay` → the bootloader's `while(true){ \Async\delay(200); }` + coroutine is **still running on every worker**. + +Mechanism: `graceful_shutdown()` stops the *server* on each worker (listeners +closed, "worker exited" logged) but the persistent bootloader coroutine was +spawned into the worker's **main scope**, outside the server scope, so the +server-side scope drain never reaches it. A sync-mode (`coroutine_mode=false`) +pool worker only retires after `RUN_SCHEDULER_AFTER_MAIN` drains, and that drain +loops while any reactor handle is alive — the delay(200) timer never lets it +finish. The worker thread never ends → never deregisters → `libuv_reactor_quiesce` +(the process-exit join) blocks forever. + +The fix (commit `937bcfe`) adds, on the worker's `done:` exit for `!coroutine_mode` +pools only, a `start_graceful_shutdown()` call *before* the drain — it cancels the +main-scope stragglers and arms the drain escape valve, so the worker retires. + +### Proof (same server.so, same test, only the PHP binary differs) + +| PHP binary | build | 056 repro | +|---|---|---| +| `/usr/local/bin/php` v0.23.0 | 2026-07-03 (pre-fix) | **hangs** (EXIT=124) every run | +| `php-src/sapi/cli/php` v0.24.0 | 2026-07-08 (has `937bcfe`) | **clean exit**, 5/5 runs | + +### Resolution + +Rebuild + reinstall PHP from the current tree (php-async ≥ v0.24.0) so the +runtime carries `937bcfe`. The server code (`http_server_class.c`) needs no +change for this hang. After reinstall, un-skip / re-enable the pool-shutdown +tests (055/056/057) in CI. + +--- + +## Symptom + +`tests/phpt/server/core/056-pool-shutdown-timeout-zero.phpt` (and the sibling +pool-shutdown tests `055-pool-graceful-shutdown`, `057-pool-reload-then-shutdown`) +**produce their full, correct output and then hang forever at process exit.** + +Captured from a direct foreground run of the `--FILE--` body: + +``` +=== STDOUT === +up=pong +stopped=clean <-- start() returned, script main body finished +=== STDERR === +[true-async-server] worker shutting down (reason=reload, grace=0s) +[true-async-server] worker exited +[true-async-server] worker shutting down (reason=reload, grace=0s) +[true-async-server] worker exited +[true-async-server] worker shutting down (reason=reload, grace=0s) +[true-async-server] worker exited +=== exit === +EXIT=124 (timeout -k 3 60 fired; process did not exit on its own) +``` + +So the test **logic** completes: `graceful_shutdown()` runs, `start()` returns, +`stopped=clean` prints, and all 3 workers quiesce and log `worker exited`. +The hang is **after** that, during final teardown of the **main thread's** +scheduler. Something keeps the main libuv loop alive so `uv_run` never returns +and the process never reaches a clean exit. + +## Key facts established + +1. **Not caused by PR #109 (the review fixes).** Proven with a clean-tree repro: + `git stash` all working changes → rebuild pristine merged `main` → run the + same repro → **identical hang** (correct stdout, `EXIT=124`). +2. **Non-deterministic / flaky.** It does not hang every time: + - `013-grpc-streaming-read` was observed as "Tests passed on retry attempt". + - A full `core/` suite run once completed 107/108 (only a warn on retry). + - So there is a race in final teardown, not a hard deadlock every run. +3. **The hang is a live libuv handle, not a TrueAsync deadlock.** If it were a + scheduler deadlock, `resolve_deadlocks()` would fire; instead the loop has a + legitimately-active handle and blocks in `epoll_wait` forever. Thread states + observed via `/proc//task/*/wchan`: main thread `futex_wait_queue` + (joining), worker/reactor threads `do_epoll_wait`. +4. **The test shape that triggers it:** `setWorkers(3)` + `setShutdownTimeout(0)` + + a **persistent bootloader coroutine** (`while(true){ \Async\delay(200); }`) + that `graceful_shutdown()` is supposed to cancel. The `delay(200)` creates a + recurring timer event on each worker; the test exists precisely to prove + graceful_shutdown cancels it. The leftover live handle at MAIN-thread teardown + is the thing to find. + +## The right way to debug this (per the actual mechanism) + +**"To understand the hang it's enough to know which events were not completed."** +Do NOT bisect blindly. The process is blocked in `uv_run` because ≥1 handle is +still active. Enumerate them: + +- Add a `uv_walk(loop, print_cb, NULL)` / `uv_print_all_handles(loop, stderr)` + dump at the point where the main scheduler is about to block during teardown + (ext/async scheduler final loop), or gate it behind an env var. +- Cross-check with the TrueAsync scheduler's own leftover accounting: + `ZEND_ASYNC_ACTIVE_EVENT_COUNT`, the `ASYNC_G(coroutines)` hash, and the + `"%u microtasks were not executed"` warning path in + `php-src/ext/async/scheduler.c`. In a DEBUG build these may already print — + capture **all** stderr (earlier runs discarded it by grepping only for + "Tests passed"). +- The DEBUG PHP build here is `ZTS DEBUG TrueAsync ABI v0.23.0`. + +Likely suspects to look at once the live handle is identified: +- the per-worker completion-future trigger events quiesced by + `graceful_shutdown()` when `shutdown_timeout == 0` (the "stop trigger, unref, + leave handle open for a safe late signal" path in `http_server_class.c`) — + an unref'd-but-not-closed handle that still counts as active on the main loop. +- the bootloader persistent coroutine's timer if cancellation races worker exit. + +## Reliable repro (safe — foreground, hard timeout, orphan cleanup) + +`run-tests.php` masks this: it compares output, sees a match, reports +"Tests passed", then hangs reaping the child — and its child PHP survives a +`timeout` on the runner. **Always** run this class of test foreground with a +kill-timeout and pkill orphans afterward: + +```bash +cd /home/edmond/true-async-server2 +# extract the runnable body (relative require needs the test dir) +awk '/^--FILE--$/{f=1;next}/^--EXPECT/{f=0}f' \ + tests/phpt/server/core/056-pool-shutdown-timeout-zero.phpt \ + > tests/phpt/server/core/_t056_repro.php + +timeout -k 3 60 php -n -d extension_dir=$PWD/modules \ + -d extension=true_async_server.so -d memory_limit=128M \ + tests/phpt/server/core/_t056_repro.php +echo "EXIT=$?" # 124 == hung; 0 == exited cleanly this run (flaky) +pkill -9 -f "_t056_repro" # reap orphans regardless +rm -f tests/phpt/server/core/_t056_repro.php +``` + +Run it several times — it does not hang every invocation. + +## Getting a stack (needs sudo — ptrace is restricted here) + +`/proc/sys/kernel/yama/ptrace_scope == 1`, so `gdb -p` needs sudo: + +```bash +sudo gdb -p -batch -ex "thread apply all bt 20" +``` + +Look for the main thread inside `uv_run` / the scheduler final loop, and check +which handle types are still active on that loop. + +## Scope note + +PR #109 (`fix/grpc-review-followups`) is the review follow-ups and is +independent of this hang. This teardown hang should be fixed on its own branch. diff --git a/docs/ISSUE_reload_persistent_coroutine_segv.md b/docs/ISSUE_reload_persistent_coroutine_segv.md index 5c7f9d0c..c5f5eb75 100644 --- a/docs/ISSUE_reload_persistent_coroutine_segv.md +++ b/docs/ISSUE_reload_persistent_coroutine_segv.md @@ -1,8 +1,23 @@ # Known issue — SIGSEGV during pool reload when the bootloader spawns a long‑lived coroutine (#93) -Status: reload **deadlock** is fixed (see "Fix that shipped"). The **intermittent -crash** it exposes is now **root-caused and filed as an ext/async bug**: -[true-async/php-async#176](https://github.com/true-async/php-async/issues/176). +Status: **RESOLVED (2026-07-09).** Both halves are fixed in php-async / TrueAsync +ABI **v0.24.0** and verified here after rebuild+reinstall: +- reload **deadlock** — fixed by `937bcfe` (sync-worker graceful-shutdown backstop). +- intermittent **SIGSEGV** (cross-thread run_time_cache UAF, #176) — fixed by + `0cbdfe8` "deep-copy nested closures per worker" (PR #180). `thread.c` now + `thread_persist_copy_xlat`'s `dynamic_func_defs` per worker, so nested closures + are no longer shared across worker arenas. + +**Verification:** the synthetic stress harness below (2 workers, persistent nested +closure `while(true){delay()}`, hot-reload rotation) ran **30/30 clean** on the +v0.24.0 build (`up=v1; after=v2; stopped=clean`, zero rc 139/134) — versus ~50% +SIGSEGV pre-fix. No server-repo change required; the environment just needed +php ≥ v0.24.0. Original investigation trail retained below for reference. + +--- + +_Historical (pre-fix):_ the intermittent crash was **root-caused and filed as an +ext/async bug** [true-async/php-async#176](https://github.com/true-async/php-async/issues/176). **Root cause (confirmed — code + ASAN + gdb):** `async_thread_create_closure` (`thread.c`) does `memcpy(&func, copy->func, sizeof(zend_op_array))` — a *shallow* diff --git a/include/http1/http_parser.h b/include/http1/http_parser.h index 02e2b08f..330e0497 100644 --- a/include/http1/http_parser.h +++ b/include/http1/http_parser.h @@ -187,7 +187,30 @@ struct http_request_t { uint8_t trace_id[16]; uint8_t span_id[8]; uint8_t trace_flags; - bool has_trace; + + /* Per-request bit flags, parked here to fill the padding hole after + * trace_flags (7 bytes before the next pointer). Single-owner request + * (handed off by ownership, never mutated concurrently) → non-atomic + * bit RMW is safe. */ + bool chunked : 1; + bool keep_alive : 1; + bool complete : 1; + bool use_multipart : 1; + bool body_streaming : 1; + bool body_eof : 1; + bool body_error : 1; + /* TODO(issue #26 backpressure): set true when on_body trips llhttp_pause; + * readBody clears it via llhttp_resume below the low-water mark. Unused + * by the MVP — see TODO in on_body. */ + bool body_paused : 1; + /* W3C Trace Context present (see trace_id/span_id/trace_flags above). */ + bool has_trace : 1; + /* Allocation domain of reactor-produced fields (method/uri/headers + the + * headers HashTable + the struct block). false = ZMM (worker-built, + * default), true = persistent malloc (reactor-built) → those frees go + * through pefree, flag-aware. Body/worker-derived fields stay ZMM. */ + bool persistent : 1; + zend_string *traceparent_raw; zend_string *tracestate_raw; @@ -232,31 +255,13 @@ struct http_request_t { int64_t reactor_stream_id; uint32_t reactor_id; - /* 1-byte fields clustered */ + /* 1-byte scalars. The bool flag cluster lives up next to trace_flags, + * packed into what was tail padding (see there). */ uint8_t http_major; uint8_t http_minor; - bool chunked; - bool keep_alive; - bool complete; - bool use_multipart; - bool body_streaming; - bool body_eof; - bool body_error; - /* grpc_mode_t stamped once at headers-complete; body policy derives * from it (http_request_body_must_buffer / _size_uncapped). */ uint8_t grpc_mode; - /* TODO(issue #26 backpressure): set true when on_body trips - * llhttp_pause; readBody clears it via llhttp_resume below the - * low-water mark. Unused by the MVP — see TODO in on_body. */ - bool body_paused; - - /* Allocation domain of reactor-produced fields - * (method/uri/headers + the headers HashTable + the struct block). - * false = ZMM (worker-built, default), true = persistent malloc - * (reactor-built) → those frees go through pefree, flag-aware. Body - * and worker-derived fields (path/query/post/files) stay ZMM. */ - bool persistent; }; /* Single chunk node in the streaming body queue (linked list).