Uh oh!
There was an error while loading. Please reload this page.
fix: Return immediately from a zero-length ReadFullBufferAsync/ReadFullBuffer instead of calling into the stream - #426
Merged
CurtHagenlocher merged 2 commits intoAug 25, 2026
Conversation
…llBuffer instead of calling into the stream A RecordBatch message body is legitimately zero bytes whenever the batch has no buffers (e.g. a batch built from a zero-column schema). StreamExtensions.ReadFullBufferAsync/ReadFullBuffer called stream.ReadAsync/stream.Read with that zero-length buffer unconditionally. Over a MemoryStream this is a harmless no-op. Over a real socket-backed NetworkStream, a zero-byte ReadAsync/Read does not complete immediately as it logically should -- it blocks as though waiting for the peer to send more data (or close the connection), instead of trivially returning 0. In a lockstep/RPC-style protocol where the peer is itself waiting for a response before sending anything further, this blocks indefinitely. Both read helpers now short-circuit buffer.Length == 0 and return 0 immediately, before ever touching the stream -- matching the Go standard library's documented behavior for io.ReadFull, which special-cases a zero-length buffer and never issues the read at all. Closesapache#425.
There was a problem hiding this comment.
Pull request overview
This PR fixes a hang in Arrow IPC stream reading when a message body is legitimately empty by ensuring StreamExtensions.ReadFullBufferAsync / ReadFullBuffer short-circuit on buffer.Length == 0 and return immediately rather than calling into Stream.ReadAsync / Stream.Read (which can block on socket-backed streams for zero-length reads).
Changes:
- Add a zero-length fast path to
ReadFullBufferAsyncto avoid issuing a zero-byteReadAsyncagainst the underlying stream. - Add the same zero-length fast path to the synchronous
ReadFullBuffer. - Add unit tests validating the zero-length fast path and confirming normal reads remain unchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Apache.Arrow/Extensions/StreamExtensions.cs | Adds zero-length buffer guards to sync/async full-buffer read helpers to prevent socket read hangs. |
| test/Apache.Arrow.Tests/StreamExtensionsTests.cs | Adds coverage for the zero-length fast path and verifies behavior for non-empty reads. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CurtHagenlocher
left a comment
Contributor
There was a problem hiding this comment.
Thanks! Other than the netstandard2.0 build break, looks good.
Uh oh!
There was an error while loading. Please reload this page.
…with #if NETCOREAPP Stream.ReadAsync(Memory<byte>, CancellationToken) is only a virtual member of Stream on netcoreapp targets; net462/net472 don't declare it, so overriding it unconditionally in ThrowsIfReadStream broke the .NET Framework builds (CS0115). Addresses review comment from @CurtHagenlocher on apache#426.
CurtHagenlocher
approved these changes
Aug 25, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
What's Changed
StreamExtensions.ReadFullBufferAsync/ReadFullBuffercalledstream.ReadAsync/stream.Readwith a zero-length buffer unconditionally, whenever a message body is legitimately empty (e.g. aRecordBatchbuilt from a zero-column schema, which has no buffers). Over aMemoryStreamthis is a harmless no-op, but over a real socket-backedNetworkStreama zero-byte read does not complete immediately — it blocks as though waiting for the peer to send more data, instead of trivially returning0. In a lockstep/RPC-style protocol this blocks indefinitely, since the peer is itself waiting for a response before sending anything further.Both helpers now short-circuit
buffer.Length == 0and return0immediately, before ever touching the stream — matching Go'sio.ReadFull, which documents the same special-case for a zero-length buffer.Full repro (real
pyarrowclient, real socket,straceevidence pinning the exact hang to this call) is in the linked issue.Added
StreamExtensionsTests.cs: aStreamsubclass whoseRead/ReadAsyncthrow if ever invoked confirms the zero-length fast path never touches the underlying stream, plus two tests confirming normal non-empty reads are unaffected.dotnet test test/Apache.Arrow.Testspasses in full (1874 passed, 28 skipped — unrelated Python interop tests, pre-existing).Closes#425.
🤖 Generated with Claude Code