Skip to content

fix: fix race identified by tsan #2891

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
mutianf merged 2 commits into from
Apr 16, 2026
Merged

fix: fix race identified by tsan #2891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
788be9d
fix: fix race identified by tsan
mutianf Apr 16, 2026
e2de403
fix
mutianf Apr 16, 2026
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
4 changes: 2 additions & 2 deletions ...ulator-core/src/main/java/com/google/cloud/bigtable/emulator/core/EmulatorController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ public synchronized void start(int port)
throw e;
}
}
Thread stdoutThread = pipeStreamToLog(process.getInputStream(), Level.INFO);
Thread stderrThread = pipeStreamToLog(process.getErrorStream(), Level.WARNING);
stdoutThread = pipeStreamToLog(process.getInputStream(), Level.INFO);
stderrThread = pipeStreamToLog(process.getErrorStream(), Level.WARNING);
isStopped = false;

shutdownHook =
Expand Down
4 changes: 2 additions & 2 deletions ...table/src/main/java/com/google/cloud/bigtable/data/v2/internal/PreparedStatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ ApiFuture<PrepareResponse> getFreshPlan() {
* Check the expiry of the current plan, if it's future is resolved. If we are within 1s of
* expiry, call startBackgroundRefresh with the version of the latest PrepareQuery.
*/
void backgroundRefreshIfNeeded() {
synchronized void backgroundRefreshIfNeeded() {
PrepareQueryState localState = this.currentState.get();
if (localState.maybeBackgroundRefresh().isPresent()) {
// We already have an ongoing refresh
Expand Down Expand Up @@ -183,7 +183,7 @@ void backgroundRefreshIfNeeded() {
* Returns the most recently refreshed PreparedQueryData. It may still be refreshing if the
* previous plan has expired.
*/
public PreparedQueryData getLatestPrepareResponse() {
public synchronized PreparedQueryData getLatestPrepareResponse() {
PrepareQueryState localState = currentState.get();
if (localState.maybeBackgroundRefresh().isPresent()
&& localState.maybeBackgroundRefresh().get().prepareFuture().isDone()) {
Expand Down
13 changes: 7 additions & 6 deletions .../google/cloud/bigtable/data/v2/internal/csm/exporter/BigtableCloudMonitoringExporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class BigtableCloudMonitoringExporter implements MetricExporter {
private final MetricServiceClient client;

private final AtomicReference<State> state;
private CompletableResultCode lastExportCode;
private AtomicReference<CompletableResultCode> lastExportCode = new AtomicReference<>();
private final AtomicBoolean exportFailureLogged = new AtomicBoolean(false);

private enum State {
Expand Down Expand Up @@ -149,8 +149,9 @@ public void close() {
public CompletableResultCode export(Collection<MetricData> metricData) {
Preconditions.checkState(state.get() != State.Closed, "Exporter is closed");

lastExportCode = doExport(metricData);
return lastExportCode;
CompletableResultCode result = doExport(metricData);
lastExportCode.set(result);
return result;
}

private CompletableResultCode doExport(Collection<MetricData> metricData) {
Expand Down Expand Up @@ -194,7 +195,7 @@ public void onFailure(Throwable throwable) {
RuntimeException asyncWrapper = new RuntimeException("export failed", throwable);
asyncWrapper.setStackTrace(stackTrace);

if (state.get() != State.Closing && state.get() != State.Closed) {
if (state.get() != State.Closing || state.get() != State.Closed) {
// ignore the export warning when client is shutting down
LOGGER.log(Level.WARNING, msg, asyncWrapper);
}
Expand Down Expand Up @@ -231,8 +232,8 @@ private List<ApiFuture<Empty>> exportTimeSeries(

@Override
public CompletableResultCode flush() {
if (lastExportCode != null) {
return lastExportCode;
if (lastExportCode.get() != null) {
return lastExportCode.get();
}
return CompletableResultCode.ofSuccess();
}
Expand Down
2 changes: 1 addition & 1 deletion ...igtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/DynamicFlowControlStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ synchronized void update(long value) {
weightedCount += weight;
}

double getMean() {
synchronized double getMean() {
return mean;
}

Expand Down
Loading