Repository navigation
Conversation
…close Fixes several race conditions, potential stream leaks, and state loss when closing streaming result sets: 1. Early Close Race Condition: If ResultSet.close() was called before asynchronous gRPC stream initialization finished (before setCall was invoked), the cancellation was silently dropped because the call reference was still null. When setCall ran moments later, the stream remained open and was never cancelled. Now, closed state is tracked so that setCall immediately cancels the stream if closed early. 2. Concurrent Close Idempotency: close() is now synchronized and extracts the call reference atomically, guaranteeing call.cancel() is executed at most once, and executing the cancellation outside the lock to prevent deadlocks. 3. Superfluous Cancellation on Completed Streams: The call handle is now cleared as soon as a PartialResultSet with last=true is consumed, avoiding unnecessary RPC cancellations when closing after reading the final chunk without an extra next() call. 4. END_OF_STREAM Queue Deduplication: Atomically deduplicate END_OF_STREAM markers to avoid queue overflow or stalls when prefetchChunks is configured to small values (e.g. prefetchChunks = 1). 5. Inlined Begin Transaction State on Close: In GrpcResultSet.close(), capture withBeginTransaction before closing the underlying iterator so listener.onDone() properly unblocks any threads waiting for inlined transaction IDs.
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves the thread safety and state management of GrpcStreamIterator and GrpcResultSet by introducing thread-safe call handling, improved cancellation logic, and a dedicated Timeout class. Additionally, it adds comprehensive unit tests to verify these changes, including concurrency and inline transaction scenarios. I have no feedback to provide.
This branch has not been deployed
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
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.
Fixes several race conditions, potential stream leaks, and state loss when closing streaming result sets: