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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 171 additions & 0 deletions docs/ISSUE_pool_shutdown_teardown_hang.md
Original file line number Diff line number Diff line change
@@ -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 <hung pid> -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/<pid>/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 <pid> -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.
21 changes: 18 additions & 3 deletions docs/ISSUE_reload_persistent_coroutine_segv.md
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
47 changes: 26 additions & 21 deletions include/http1/http_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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).
Expand Down
Loading