ext/async: fix UDP req type-confusion in IO dispose backstop#157
Merged
Conversation
libuv_io_event_dispose's leftover-request sweep cast io->active_req to async_io_req_t and called req->base.dispose. But dispose() sits at a different struct offset for zend_async_udp_req_t (after a 4-byte flags) than for zend_async_io_req_t (after an 8-byte free_cb), so for a UDP recv req this read the request's sockaddr bytes as a function pointer -> access violation. Branch on io->base.type and dispose UDP reqs through the correct layout. Also document the libuv_io_close ownership contract: a multishot recv req (persistent callback, no awaiter) is owned and freed by the consumer that submitted it (e.g. the HTTP/3 listener on teardown); freeing it in io_close would double-free. Surfaced on Windows while fixing the true-async/server HTTP/3 listener UDP recv-request leak.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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
Fixes a latent type-confusion crash in the libuv reactor's IO-dispose backstop when an in-flight request is a UDP recv req.
Root cause
libuv_io_event_dispose's leftover-request sweep castio->active_reqtoasync_io_req_tand readreq->base.dispose. Butdispose()sits at a different struct offset forzend_async_udp_req_t(after a 4-byte flags) than forzend_async_io_req_t(after an 8-byte free_cb). For a UDP recv req this read the request's sockaddr bytes as a function pointer → access violation (0xC0000005). The old comment ("dispose at the same offset for both") was false; the path never worked for UDP.Fix
io->base.type == ZEND_ASYNC_IO_TYPE_UDPand dispose UDP reqs through the correct layout.libuv_io_closeownership contract: a multishot recv req (persistent callback, no awaiter) is owned and freed by the consumer that submitted it; freeing it inio_closewould double-free.libuv_io_close/udp_recv_cbbehaviour is otherwise unchanged.Context
Surfaced on Windows while fixing the
true-async/serverHTTP/3 listener UDP recv-request leak (paired server PR: true-async/server#86). The server-side consumer-frees fix closes the leak on its own; this reactor fix ensures the dispose backstop can never crash on a UDP req.Verification
Built into php.exe (Debug_TS) and exercised via the server h3 suite on Windows: no crash, h3 14 passed / 0 failed / 22 skipped, and the paired leak fix verified clean (Zend MM: before 2 leaks → after 0).