Skip to content

test(spanner): unflake InlineBeginTransactionTest - #14686

Open
sakthivelmanii wants to merge 2 commits into
mainfrom
unflake-b575390510-InlineBeginTransactionTest
Open

sakthivelmanii wants to merge 2 commits into
mainfrom
unflake-b575390510-InlineBeginTransactionTest

Conversation

@sakthivelmanii

Copy link
Copy Markdown
Contributor

In ResumableStreamIterator, when close() is called while startStream() is in flight, stream is null so cancellation is dropped and isWithBeginTransaction() evaluates to false, causing listener.onDone(false) and unhandled transactionIdFuture resolution on mock unfreeze. This fix cleanly checks closed state immediately after stream start under lock, invokes onStreamClosedWhileStarting to fail transactionIdFuture with CANCELLED without modifying StreamStarter signatures, and adds deterministic latch-based unit tests.

In ResumableStreamIterator, when close() is called while startStream() is in flight, stream is null so cancellation is dropped and isWithBeginTransaction() evaluates to false, causing listener.onDone(false) and unhandled transactionIdFuture resolution on mock unfreeze. This fix cleanly checks closed state immediately after stream start under lock, invokes onStreamClosedWhileStarting to fail transactionIdFuture with CANCELLED without modifying StreamStarter signatures, and adds deterministic latch-based unit tests.
@sakthivelmanii
sakthivelmanii requested review from a team as code owners October 19, 2026 13:27
@sakthivelmanii
sakthivelmanii enabled auto-merge (squash) October 19, 2026 13:27

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a concurrency issue where a ResultSet is closed while its underlying stream is being started. It introduces synchronization and state tracking in ResumableStreamIterator to ensure that a concurrently started stream is properly closed and its BeginTransaction option is reported. A critical review comment identifies a potential bug where listener.onDone could be called twice concurrently (once with false and once with true), which may cause the transaction to hang or fail silently due to onDone being guarded by a compareAndSet check.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant