Skip to content

fix(spanner): restore setCall overload for stream test compatibility #14885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sakthivelmanii merged 1 commit into from
Nov 2, 2026
Merged

fix(spanner): restore setCall overload for stream test compatibility #14885

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion ...anner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/GrpcStreamIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public String toString() {
private boolean callCleared;
private boolean closed;
@Nullable private String closeMessage;
private final boolean withBeginTransaction;
private volatile boolean withBeginTransaction;
private final boolean lastStatement;
@Nullable private volatile Timeout streamWaitTimeout;
@Nullable private volatile SpannerException error;
Expand Down Expand Up @@ -156,6 +156,14 @@ void registerListener(AsyncResultSet.StreamMessageListener streamMessageListener
}
}

@VisibleForTesting
void setCall(SpannerRpc.StreamingCall call, boolean withBeginTransaction) {
synchronized (this) {
this.withBeginTransaction = withBeginTransaction;
}
Comment on lines +161 to +163

@gemini-code-assist gemini-code-assist Bot Nov 2, 2026

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.

medium

The withBeginTransaction variable is declared as volatile, which already guarantees visibility and atomicity for writes. If this assignment does not need to be atomic with other operations protected by synchronized (this), wrapping it in a synchronized block is redundant. Please verify whether the lock is required for coordinating other states before removing it.

this.withBeginTransaction = withBeginTransaction;
References
  1. Before suggesting lock-free alternatives or atomic variables to avoid lock contention, verify whether the target method actually acquires any locks or if the state is already managed without synchronization.

@olavloite olavloite Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it can be simplified, then we will do so in a follow-up PR (currently under investigation).

setCall(call);
}

void setCall(SpannerRpc.StreamingCall call) {
Timeout timeout = extractTimeout(call);
boolean shouldCancel;
Expand Down
Loading