fix(pool): dispose per-worker completion futures on shutdown/reload — no leaked cross-thread triggers (#93)#107
Merged
Conversation
…o their cross-thread triggers don't leak on the parent reactor (#93) http_server_start_pool() owns the completion futures returned by pool->submit_internal() but never disposed them. Their shared-state cross-thread wakeup triggers (uv_async on the parent reactor) therefore lingered armed after a graceful shutdown — tripping the scheduler loop-alive assert on debug builds and leaking a libuv handle per worker on release. reload() had the same leak per rotation. The parent now stores each worker_evt and disposes it: in start_pool's cleanup (covering every exit path) and in reload() for the retired cohort. Disposal is race-safe against a still-running worker via the paired remote_future_dispose fix in php-async (true-async/php-async#182) — the shared callback carries a no-op dispose so freeing a future's callback vector never touches the embedded, shared st->cb. Tests: 055 (graceful_shutdown clean), 056 (setShutdownTimeout(0) immediate quiesce), 057 (reload then graceful_shutdown — both cohorts reaped).
Contributor
CoverageTotal lines: 74.29% → 75.44% (+1.16 pp)
|
) resume_when(..., true) hands all_done to the waker and waker_clean disposes it, but the cleanup path disposed it again — a use-after-free in server_wait_event_dispose (harmless on Linux/debug where the freed slot is still readable, a hard EXC_BAD_ACCESS crash on macOS ARM64 release). NULL the pointer after waker_clean so the cleanup dispose is a no-op, mirroring the existing server->wait_event teardown. Surfaced by the 055-057 clean-shutdown tests — the only ones reaching this cleanup without SIGKILL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
http_server_start_pool()owns the completion futures returned bypool->submit_internal()(one per worker) but never disposed them. Each future's shared-state cross-thread wakeup trigger (auv_asyncon the parent reactor) stayed armed after shutdown:scheduler.c— "The event loop must be stopped") aborts on the parent thread's finalize;uv_asynchandle leaks per worker (async: leftover libuv handle).reload()leaked the same way, once per rotation.The reason it was never seen: every existing pool test exits via
SIGKILL, so the process dies before the parent reactor finalizes.Async\graceful_shutdown()is the real clean path, and it tripped this.Fix
Store each
worker_evtinpool_worker_ctx_t.done_eventand dispose it:start_pool'scleanup:(covers every exit path, including thegotos);reload()for the retired cohort, so they don't accumulate across reloads.Disposal is race-safe against a still-running worker via the paired fix true-async/php-async#182 (
remote_future_disposenow severs the producer under the shared-state mutex before closing the trigger). The sharedst->cbcarries a no-op dispose so freeing a disposed future's callback vector never touches the embedded, shared callback.Tests
055-pool-graceful-shutdown— pool serves,graceful_shutdown(),start()returns clean.056-pool-shutdown-timeout-zero—setShutdownTimeout(0), immediate quiesce.057-pool-reload-then-shutdown—reload()thengraceful_shutdown(), both cohorts reaped.All green ×5, no flake (was a race).
Dependency