Cover concurrent reads from one shared file handle (#280) - #281
Merged
Conversation
Two coroutines reading the same handle lose and duplicate data: both derive the same buffer address from a state neither has finished updating, so 1024 of 2000 distinct records survive. The test states the contract rather than the counts: every byte reaches exactly one reader. Needs the buffer lock from true-async/php-src#32.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Closing a handle from a second coroutine frees the stream inside the first one's ops->read or ops->write, and the buffered layer then reached the freed stream through its lock and through the write path's eof check. The test reads and writes under such a close; on the previous php-src code it segfaults under a sanitizer build. Needs true-async/php-src#32.
Six coroutine tests over one shared handle: a read filter appended and removed while another coroutine is inside fread(), a cast to a file descriptor during a read, and a close landing while a copy, a stream_get_contents() or a write filter flush is parked. Each one loses data, hangs or crashes without the lock those paths now take.
EdmondDantes
deleted the
280-guzzle-streamfread-reads-with-a-264-2-length-and-fails-with-async-io-error-during-s3-upload-of-large-local-file
branch
September 8, 2026 16:55
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.
Regression tests for #280.
tests/io/085-shared_handle_concurrent_reads.phpt— two coroutines read onefopen()handle of a file holding 2000 distinct 16-byte records. Every byte mustreach exactly one reader; today 1024 records of 2000 survive, while the byte
count still adds up — which is why the S3 upload in the issue produced a wrong
body instead of an error.
tests/io/086-close_during_io.phpt— a second coroutine closes the handlewhile the first is parked inside the read or the write. The stream is freed
there, and the buffered layer then reached it again: through the lock on the read
path, and through the
eofcheck ofphp_stream_write_buffer()on the writepath. On the previous php-src code this segfaults under a sanitizer build (and
under a poisoned free); with the fix both loops run to the end.
Both are green with true-async/php-src#32, which serialises the
derive-read-credit sequence in the buffered stream API and keeps the lock alive
across a close.