fix(spanner): add closeAsync to ReadContext and make transaction closing non-blocking

[olavloite]

Adds ReadContext.closeAsync() to allow closing read contexts asynchronously without blocking caller threads. Refactors MultiUseReadOnlyTransaction so that background query initializations and BeginTransaction RPCs do not block thread execution during close, while preserving legacy synchronous close() semantics and guarding against new operations during closing.

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing support for read contexts (closeAsync()) and optimizes transaction initialization by executing the BeginTransaction RPC outside of the transaction lock (txnLock) to prevent blocking concurrent operations. It also adds comprehensive unit tests to verify these asynchronous behaviors. The review feedback highlights two critical concurrency issues: a race condition in decrementPendingStartsAndSignal() where pendingStarts is decremented and checked outside of the lock, and a potential duplicate BeginTransaction RPC issue in initFallbackTransaction() due to the lack of a leader/follower synchronization pattern.

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing capability (closeAsync()) to ReadContext and its implementations, tracking pending asynchronous queries to ensure the context is closed only after they complete. It also optimizes transaction initialization by executing the BeginTransaction RPC outside of the transaction lock (txnLock). The reviewer identified potential deadlock risks in both decrementPendingStartsAndSignal() and closeAsync() due to calling super.close() while holding txnLock, which can conflict with session pool locks. The reviewer recommended refactoring these methods to execute super.close() outside of the locked sections.

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing capabilities (closeAsync()) to Spanner read contexts, allowing transactions to close safely after all pending asynchronous query initializations complete. It also refactors transaction initialization to execute the BeginTransaction RPC outside of the transaction lock to prevent blocking concurrent operations. The reviewer identified two critical issues: first, closeAsync() in AbstractReadContext could release a session back to the pool while a BeginTransaction RPC is still in-flight, potentially causing session corruption; second, closeAsync() in DelayedReadContext should cancel the pending readContextFuture if it is not yet done to prevent hangs and resource leaks.

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing capabilities (closeAsync()) to ReadContext, AbstractReadContext, and DelayedReadContext in the Google Cloud Spanner Java client, allowing transactions to be closed without blocking threads during in-flight operations. It also adds comprehensive unit tests to verify the new asynchronous behavior, idempotency, and error handling. The review feedback suggests several improvements: removing a redundant override of closeAsync() in AbstractReadContext that duplicates the default interface implementation, using defensive checks (<= 0 instead of == 0) for tracking pending starts to prevent potential hangs, and refactoring closeAsync() to consistently initialize and return the same closeFuture to ensure robust exception propagation on subsequent calls.

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing capabilities to the Spanner ReadContext by adding a closeAsync() method, implemented across AbstractReadContext and DelayedReadContext, alongside comprehensive unit tests. In AbstractReadContext, the closing process is coordinated with pending asynchronous query initializations, and transaction initialization is refactored to execute the BeginTransaction RPC outside of the transaction lock to prevent blocking concurrent operations. The review feedback highlights critical race conditions in DelayedReadContext's close() and closeAsync() methods, where the underlying readContextFuture could complete between the isDone() check and the cancel(true) call, potentially resulting in unclosed resources. Addressing these by directly checking the result of cancel(true) or isCancelled() will ensure thread safety.

[olavloite]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces asynchronous closing capabilities to the Spanner ReadContext by adding a new closeAsync() method. In AbstractReadContext, the synchronous blocking mechanism using a Condition is replaced with a non-blocking, future-based approach (closeFuture) that tracks pending asynchronous queries. Additionally, initTransaction() is updated to run the BeginTransaction RPC outside of the transaction lock using a leader/follower pattern with transactionIdFuture to prevent blocking concurrent operations. DelayedReadContext is also updated to support asynchronous closing and cancellation of pending initialization futures. Comprehensive unit tests have been added to verify these new behaviors. There are no review comments, so no feedback is provided.