Skip to content

Commit cf92f2d

Browse files
xds: Prevent concurrent cancellations of data plane call in ExternalProcessorClientInterceptor (grpc#12996)
If the ext-proc stream terminates with a non-OK status, the interceptor cancels the downstream data plane call (on the executor thread). Concurrently, the application thread may call cancel() on the returned proxy call (e.g. for user cancellation or cleanup). Since ClientCallImpl does not synchronize its cancelCalled field, concurrent cancellations from these two threads resulted in a TSAN data race (that was discussed in grpc#12975). This commit introduces an AtomicBoolean downstreamCancelled in DataPlaneClientCall to guard all downstream cancellations, ensuring that only the first cancellation is forwarded to delayedCall/super.cancel(). This deduplicates and serializes cancellation handling from both the application and the interceptor threads, preventing the data race.
1 parent 0e8e0dd commit cf92f2d

1 file changed

Lines changed: 11 additions & 4 deletions

File tree

xds/src/main/java/io/grpc/xds/ExternalProcessorClientInterceptor.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ private static class DataPlaneClientCall
315315
final AtomicBoolean isProcessingTrailers = new AtomicBoolean(false);
316316
final AtomicBoolean pendingHalfClose = new AtomicBoolean(false);
317317
final AtomicBoolean bodyMessageSentToExtProc = new AtomicBoolean(false);
318+
private final AtomicBoolean downstreamCancelled = new AtomicBoolean(false);
318319

319320
protected DataPlaneClientCall(
320321
DataPlaneDelayedCall<InputStream, InputStream> delayedCall,
@@ -403,7 +404,7 @@ private boolean validateCompressionSupport(BodyResponse bodyResponse) {
403404
}
404405
activateCall();
405406
markExtProcStreamFailed(extProcStreamState);
406-
delayedCall.cancel("gRPC message compression not supported in ext_proc", ex);
407+
cancelDownstream("gRPC message compression not supported in ext_proc", ex);
407408
closeExtProcStream();
408409
return false;
409410
}
@@ -590,7 +591,7 @@ public void onError(Throwable t) {
590591
handleFailOpen(wrappedListener);
591592
} else {
592593
String message = "External processor stream failed";
593-
delayedCall.cancel(message, t);
594+
cancelDownstream(message, t);
594595
wrappedListener.proceedWithClose();
595596
}
596597
}
@@ -712,7 +713,7 @@ private void internalOnError(Throwable t) {
712713
handleFailOpen(wrappedListener);
713714
} else {
714715
String message = "External processor stream failed";
715-
delayedCall.cancel(message, t);
716+
cancelDownstream(message, t);
716717
wrappedListener.proceedWithClose();
717718
}
718719
}
@@ -899,6 +900,12 @@ public void halfClose() {
899900
.build());
900901
}
901902

903+
private void cancelDownstream(@Nullable String message, @Nullable Throwable cause) {
904+
if (downstreamCancelled.compareAndSet(false, true)) {
905+
delayedCall.cancel(message, cause);
906+
}
907+
}
908+
902909
@Override
903910
public void cancel(@Nullable String message, @Nullable Throwable cause) {
904911
synchronized (streamLock) {
@@ -910,7 +917,7 @@ public void cancel(@Nullable String message, @Nullable Throwable cause) {
910917
.asRuntimeException());
911918
}
912919
}
913-
super.cancel(message, cause);
920+
cancelDownstream(message, cause);
914921
}
915922

916923
private void handleRequestBodyResponse(BodyResponse bodyResponse) {

0 commit comments

Comments
 (0)