[sakthivelmanii]

Summary

In GrpcResultSet.close(), a previous commit inadvertently moved iterator.close() ahead of listener.onDone(). Cancelling the stream unblocked concurrent reader threads in next() with CANCELLED, triggering yieldError() which completed transactionIdFuture with ABORTED before listener.onDone() could complete it with FAILED_PRECONDITION. This race caused flaky test failures in InlineBeginTransactionTest.

This PR fixes the issue by reordering listener.onDone(withBeginTransaction) before iterator.close(), guaranteeing the future is marked FAILED_PRECONDITION before the stream is cancelled. It also includes a deterministic regression test to ensure the fix is robust.

[sakthivelmanii]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates GrpcResultSet.close() to notify the listener before closing the iterator, preventing a race condition where a concurrent reader thread fails with a CANCELLED error before the listener is notified. A new unit test has been added to verify this behavior. The review feedback suggests wrapping the listener notification and iterator closing in a try-finally block to ensure the iterator is always closed, even if the listener throws an unexpected exception.

[gemini-code-assist]

Code Review

This pull request updates GrpcResultSet to notify the listener before closing the iterator, preventing a race condition where concurrent threads blocked in next() might fail with a cancellation error instead of the correct transaction outcome. A corresponding unit test is also added to verify this behavior. The review feedback suggests wrapping the listener notification in a try-finally block to guarantee that the iterator is always closed, and replacing Thread.yield() with Thread.sleep(10) in the test's waiting loop to avoid high CPU utilization and potential test flakiness.

[sakthivelmanii]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies GrpcResultSet.close() to notify the listener before cancelling the stream, preventing race conditions with concurrent readers, and adds a corresponding unit test. However, the change accidentally omitted the call to iterator.close(), leaving an empty finally block. This critical issue will cause resource leaks, and the iterator must be closed within the finally block as suggested.

[sakthivelmanii]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies GrpcResultSet.close() to notify the listener before closing the iterator, preventing a race condition where a concurrent thread blocked in next() fails with a CANCELLED error before the listener is notified. A unit test was added to verify this behavior. The review feedback suggests improving the unit test by checking if the reader thread's future is done inside the waiting loop to fail fast if the thread terminates unexpectedly.

[sakthivelmanii]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies GrpcResultSet.close() to notify the listener before closing the iterator (cancelling the stream) using a try-finally block. This prevents a race condition where a concurrent thread blocked in next() fails with a CANCELLED error before the listener is notified. Additionally, a unit test testCloseNotifiesListenerBeforeCancelErrorFromConcurrentReader has been added to GrpcResultSetTest.java to verify this behavior. I have no feedback to provide.

[sakthivelmanii]

Closing in favor of #14770.