fix(http3): dispose multishot UDP recv req on listener teardown#86
Merged
Conversation
http3_listener_destroy closed the UDP io and disposed the event but never freed the multishot recv request it submitted via ZEND_ASYNC_UDP_RECVFROM. ZEND_ASYNC_IO_CLOSE only detaches io->active_req (its await-handoff assumes a parked coroutine frees the req) and the recv callback merely counts datagrams, so the req struct plus its 2 KiB recv buffer leaked on every listener teardown. Capture recv_req before close and dispose it after: close clears the reactor's reference first, so there is no use-after-free, and the typed zend_async_udp_req_t pointer frees through the correct layout. This matches the reactor's documented ownership contract (a multishot recv req is owned by the consumer that submitted it) and how http_connection already disposes its own multishot read req. Verified on Windows (Debug_TS): before = 2 Zend MM leaks (320 B req + 2048 B buf); after = 0 leaks, h3 suite green. Pairs with true-async/php-async#windows-reactor-fixes (ed2730d), which fixes a latent type-confusion in the reactor's dispose backstop so the UDP path can never crash.
Contributor
CoverageTotal lines: 81.42% → 81.20% (-0.22 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
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 memory leak in the HTTP/3 listener: the multishot UDP recv request submitted by
http3_listenerwas never freed on teardown, leaking the request struct + its 2 KiB recv buffer on every listener destroy.Root cause
http3_listener_destroydidZEND_ASYNC_IO_CLOSE(io)+event.dispose(io)but never disposed the recv req it submitted viaZEND_ASYNC_UDP_RECVFROM.ZEND_ASYNC_IO_CLOSEonly detachesio->active_req(its await-handoff path assumes a parked coroutine frees the req), and the listener's recv callback only counts datagrams — so nothing freed the req.Fix
Capture
recv_reqbefore close, thenrecv_req->dispose(recv_req)afterZEND_ASYNC_IO_CLOSE:zend_async_udp_req_tpointer frees through the correct layout.This follows the reactor's documented ownership contract (a multishot recv req is owned by the consumer that submitted it) and matches how
http_connectionalready disposes its own multishot read req (src/core/http_connection.c).Verification (Windows, Debug_TS)
=== Total 2 memory leaks detected ===— 320 B req + 2048 B buffer (fromlibuv_udp_recvfrom).datagrams_received=3.tests/phpt/server/h3: 14 passed / 0 failed / 22 skipped (skips are platform-only: POSIX-only h3client / objdump n/a on Windows).Related
Pairs with the
true-async/php-asyncPR (branchwindows-reactor-fixes, commited2730d), which fixes a latent type-confusion in the reactor's dispose backstop so the UDP path can never crash. That fix is independent — this PR closes the leak on its own — but both come from the same investigation.