Skip to content

Commit 66016fd

Browse files
committed
xds: Use real ScheduledExecutorService in ext_proc draining test to avoid FakeClock TSAN race
In ExternalProcessorClientInterceptorTest.givenDrainingStream_whenExtProcStreamCompletes_thenOnReady(), the mock ext-proc server spawns a background thread that invokes responseObserver.onNext(), scheduling completion tasks on scheduler. Concurrently, the test runner thread was advancing fakeClock.forwardTime(...). Because FakeClock is not thread-safe, this resulted in a TSAN data race on FakeClock.currentTimeNanos. This commit replaces the FakeClock-backed scheduler with a dedicated single-thread ScheduledExecutorService (realScheduler), configures the in-process server/channel with directExecutor(), and relies on explicit latch synchronization instead of FakeClock ticks, eliminating the data race.
1 parent 78e1e42 commit 66016fd

1 file changed

Lines changed: 7 additions & 31 deletions

File tree

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

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7756,18 +7756,19 @@ public void onCompleted() {
77567756
};
77577757
}
77587758
};
7759+
ScheduledExecutorService realScheduler = Executors.newSingleThreadScheduledExecutor();
77597760
grpcCleanup.register(InProcessServerBuilder.forName(uniqueExtProcServerName)
77607761
.addService(extProcImpl)
7761-
.executor(scheduler)
7762+
.directExecutor()
77627763
.build().start());
77637764

77647765
CachedChannelManager channelManager = new CachedChannelManager(config -> {
77657766
return grpcCleanup.register(
7766-
InProcessChannelBuilder.forName(uniqueExtProcServerName).executor(scheduler).build());
7767+
InProcessChannelBuilder.forName(uniqueExtProcServerName).directExecutor().build());
77677768
});
77687769

77697770
ExternalProcessorClientInterceptor interceptor = new ExternalProcessorClientInterceptor(
7770-
filterConfig, channelManager, scheduler, FAKE_CONTEXT);
7771+
filterConfig, channelManager, realScheduler, FAKE_CONTEXT);
77717772

77727773
MutableHandlerRegistry uniqueRegistry = new MutableHandlerRegistry();
77737774
grpcCleanup.register(InProcessServerBuilder.forName(uniqueDataPlaneServerName)
@@ -7806,49 +7807,24 @@ public void onReady() {
78067807
ClientCall<String, String> proxyCall =
78077808
interceptCall(interceptor, METHOD_SAY_HELLO, callOptions, dataPlaneChannel);
78087809
proxyCall.start(appListener, new Metadata());
7809-
for (int i = 0; i < 10; i++) {
7810-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7811-
}
7812-
proxyCall.request(1);
7813-
for (int i = 0; i < 10; i++) {
7814-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7815-
}
7816-
78177810
// Wait for sidecar to send drain and test to observe it
78187811
assertThat(sidecarOnNextLatch.await(5, TimeUnit.SECONDS)).isTrue();
7819-
for (int i = 0; i < 10; i++) {
7820-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7821-
}
7812+
proxyCall.request(1);
78227813
assertThat(proxyCall.isReady()).isFalse();
78237814

78247815
// Now let sidecar complete
78257816
sidecarFinishLatch.countDown();
7826-
for (int i = 0; i < 10; i++) {
7827-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7828-
}
7817+
assertThat(sidecarOnCompletedLatch.await(5, TimeUnit.SECONDS)).isTrue();
78297818

78307819
dataPlaneFinishLatch.countDown();
7831-
for (int i = 0; i < 10; i++) {
7832-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7833-
}
7834-
7835-
assertThat(sidecarOnCompletedLatch.await(5, TimeUnit.SECONDS)).isTrue();
7836-
for (int i = 0; i < 10; i++) {
7837-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7838-
}
78397820

78407821
// After sidecar stream completes, it should trigger onReady and become ready
78417822
assertThat(onReadyLatch.await(5, TimeUnit.SECONDS)).isTrue();
78427823
assertThat(proxyCall.isReady()).isTrue();
78437824

78447825
proxyCall.cancel("Cleanup", null);
7845-
for (int i = 0; i < 10; i++) {
7846-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7847-
}
78487826
channelManager.close();
7849-
for (int i = 0; i < 10; i++) {
7850-
fakeClock.forwardTime(1, TimeUnit.SECONDS);
7851-
}
7827+
realScheduler.shutdown();
78527828
}
78537829

78547830
@Test

0 commit comments

Comments
 (0)