fix: resolve pool-shutdown hang + reload SIGSEGV (#176) via ABI v0.24.0; pack http_request_t flags#110
Merged
Merged
Conversation
…BI v0.24.0 Both were runtime bugs fixed upstream in php-async, not server-repo bugs: - pool graceful-shutdown teardown hang (core/055-057): stale php v0.23.0 binary; fixed by php-async 937bcfe (sync-worker graceful-shutdown backstop). - intermittent reload SIGSEGV (#176): cross-thread run_time_cache UAF from shared nested-closure dynamic_func_defs; fixed by php-async 0cbdfe8 (PR #180). Verified after rebuild+reinstall to v0.24.0: 055/056/057 PASS; #176 stress harness 30/30 clean (was ~50% SIGSEGV pre-fix). No server code change.
The 10 per-request bool flags (chunked/keep_alive/complete/use_multipart/ body_streaming/body_eof/body_error/body_paused/has_trace/persistent) were 1 byte each and, with the surrounding 1-byte scalars, rounded the struct tail up to a full 8-byte line. Convert them to bool:1 bitfields and park the cluster in the 7-byte padding hole that already sat after trace_flags. Field syntax is unchanged (bool:1 keeps read/write ergonomics and 0/1 conversion), so no use-site edits. A request is single-owner (handed off by ownership, never mutated concurrently), so non-atomic bit RMW is safe. sizeof(http_request_t): 368 -> 360. Verified: server phpt core 57, grpc 15, h1 26, h2 29, h3 51, static 23, compression 12, sendfile 5, tls 15, reactor_pool 9 — all pass.
Contributor
CoverageTotal lines: 81.23% → 80.84% (-0.39 pp)
|
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.
Summary
Two long-standing flaky failures in the pool-shutdown / hot-reload tests were
runtime bugs fixed upstream in
php-async, not server-repo bugs — the CIbox was simply running a PHP built one day before the fixes landed. This PR
records that finding and lands one unrelated struct-packing win from the review
backlog.
1. Root-caused two "known issues" → both resolved by ABI v0.24.0
core/055–057): confirmed via gdbon a live hang — the main thread blocks in
php_module_shutdown → libuv_reactor_quiescejoining worker threads that never exit, because thebootloader's persistent coroutine is never cancelled on retire. Fixed by
php-async
937bcfe(sync-worker graceful-shutdown backstop).run_time_cacheUAF froma shared nested-closure
dynamic_func_defs. Fixed by php-async0cbdfe8(deep-copy nested closures per worker, PR #180).
Both were verified here after rebuilding + reinstalling PHP to ABI v0.24.0:
055/056/057PASS, and a synthetic #176 stress harness ran 30/30 clean(≈50 % SIGSEGV before). The two
docs/ISSUE_*files are updated to RESOLVEDwith the full investigation trail. No server code change was needed for
either — the environment just needs php ≥ v0.24.0.
2.
http_request_t: pack bool flags into a bit cluster (368 → 360 B)The 10 per-request bool flags were 1 byte each and, with the neighbouring
1-byte scalars, rounded the struct tail up to a full 8-byte line. Convert them
to
bool:1bitfields and park the cluster in the 7-byte padding hole thatalready sat after
trace_flags. Named bitfields keep read/write ergonomics(and 0/1 conversion) identical, so there are no use-site edits. A request
is single-owner (handed off by ownership, never mutated concurrently), so
non-atomic bit RMW is safe.
Testing
Full server phpt suite against the rebuilt v0.24.0 stack, all green:
Plus:
055/056/057PASS, #176 stress harness 30/30 clean.