Honour ZEND_ASYNC_IO_WRITEV_AWAIT: a vectored write the caller can wait for - #262
Merged
Conversation
The vectored write reported nothing: the completion freed the request and sent no notification, so the only write a caller could wait for was the single-buffer one — which leaves the buffer with the caller and therefore makes the caller free it when its own wait ends. Cancel the waiting coroutine and those moments come apart: the wait is over while the write is still in libuv's queue, pointing at memory the caller has released. With the flag set the completion releases the slots as before, then either finishes a dispose that arrived while the write was queued, or notifies io->event with the request as the result and no exception. The exception is left on the request for the awaiter to read: passing it on the broadcast would wake every reader and writer on the handle, because each listener forwards an exception unconditionally and filters by result only otherwise. The mode now comes from the low bit rather than from equality, so the two existing values keep their meaning.
An assert aborts a debug build and, on release, silently gives an unknown bit whatever meaning this version happens to have — for a caller compiled against a newer header, the quiet outcome is the dangerous one. AWAIT is ZSTR-only: the awaited completion hands the request to its awaiter, so it cannot also keep the IOV promise that free_cb runs at completion.
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.
Reactor half of true-async/php-src#24. Consumer: true-async/server (HTTP/1 chunk frames).
Why
The vectored write reports nothing —
io_pipe_writev_cbfreed the request and sent no NOTIFY — so a caller that needs the outcome has to use the single-buffer write. That one does not take the buffer over, so the caller frees it, and the only moment it can is when its own wait ends. Cancel the waiting coroutine and those two moments come apart: the wait is over, the write is still queued, and libuv holds a pointer into memory the caller has released.Reproduced in the server: a handler parked writing to a peer that never reads,
setShutdownTimeout(0), thenstop(). Probes fired in order — the await returns withreq->completed == false,libuv_io_req_disposedefers becauseASYNC_IO_REQ_F_UV_IN_FLIGHTis set, and the caller frees a 20008-byte frame. Stamping the freed block showed the allocator hands the same address straight back.What changes
ASYNC_IO_REQ_F_AWAITEDis set at submit when the caller passes the flag. The completion releases the slots exactly as before, then:io->eventis notified with the request as the result and no exception.The exception stays on the request. Passing it on the broadcast would wake every reader and writer on the handle, because each listener forwards an exception unconditionally and filters by result only otherwise — that is load-bearing elsewhere (it is how a writer parked on a closed handle is woken), so the awaited path must not use it.
The mode is now read from the low bit instead of by equality, so
ZSTRandIOVkeep their values and an unknown bit trips an assert instead of silently selecting a mode.Evidence
Built against php-src#24.
ext/async/tests: 1104 passed, 0 failed.true-async/servertests/phptagainst the same build: 390 passed, 0 failed, with the server's HTTP/1 frame path converted to the new op — after which the caller frees nothing and the probe that caught the early free stops firing.