Skip to content

Commit 33f3b12

Browse files
committed
Fix sendMessage race condition and test expectations.
1. Remove requestDrainComplete check from sendMessage to prevent out-of-order message delivery when app calls sendMessage concurrently with drain completion. Now we only bypass queue when passThroughMode is true. 2. Update givenExtProcStreamCompleted_whenIsReadyCalled_thenDelegatesToSuper test to expect the correct number of downstream isReady calls, which increased by 1 because we now correctly query isReady inside onReadyNotify on stream completion. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
1 parent c9e02fa commit 33f3b12

2 files changed

Lines changed: 5 additions & 4 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -849,13 +849,13 @@ public void sendMessage(InputStream message) {
849849
return;
850850
}
851851

852-
if (passThroughMode.get() || requestDrainComplete.get()) {
852+
if (passThroughMode.get()) {
853853
super.sendMessage(message);
854854
return;
855855
}
856856

857857
synchronized (streamLock) {
858-
if (passThroughMode.get() || requestDrainComplete.get()) {
858+
if (passThroughMode.get()) {
859859
super.sendMessage(message);
860860
return;
861861
}
@@ -1394,6 +1394,7 @@ void unblockAfterStreamComplete() {
13941394
proceedWithHeaders();
13951395
proceedWithSavedMessages();
13961396
dataPlaneClientCall.drainPendingDrainingMessages();
1397+
onReadyNotify();
13971398
proceedWithClose();
13981399
}
13991400

xds/src/test/java/io/grpc/xds/ExternalProcessorClientInterceptorTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6070,11 +6070,11 @@ public boolean isReady() {
60706070
// 4. Assert that proxyCall.isReady() delegates directly to the downstream call
60716071
downstreamReady.set(true);
60726072
assertThat(proxyCall.isReady()).isTrue();
6073-
assertThat(downstreamIsReadyCallCount.get()).isEqualTo(1);
6073+
assertThat(downstreamIsReadyCallCount.get()).isEqualTo(2);
60746074

60756075
downstreamReady.set(false);
60766076
assertThat(proxyCall.isReady()).isFalse();
6077-
assertThat(downstreamIsReadyCallCount.get()).isEqualTo(2);
6077+
assertThat(downstreamIsReadyCallCount.get()).isEqualTo(3);
60786078

60796079
proxyCall.cancel("cleanup", null);
60806080
channelManager.close();

0 commit comments

Comments
 (0)