Skip to content

Commit 4ce53b1

Browse files
Fix ext_proc mock response handling in ext_proc client interceptor test (grpc#12975)
In `ExternalProcessorClientInterceptorTest.clientInterceptor_contextPropagatedToStartCall()`, the mock ext_proc service blindly returned a `ProcessingResponse` with `setRequestHeaders` for every incoming `ProcessingRequest`. When the downstream RPC finished and sent back response headers, the interceptor sent a `ProcessingRequest` with `responseHeaders`. The mock's blind `requestHeaders` reply caused the interceptor to detect an out-of-order protocol error, triggering `internalOnError()` which called `delayedCall.cancel()` on the executor thread. This async cancellation on the executor thread raced with the test code's cleanup `proxyCall.cancel()` on the test runner thread, causing TSAN to report a data race on the non-volatile field `ClientCallImpl.cancelCalled`. This commit fixes the test mock to check the request type (`hasRequestHeaders()` vs. `hasResponseHeaders()`) before responding, matching the behavior in `clientInterceptor_contextPropagatedToListenerCallbacks()`. Note: The unsynchronized cancellation between the application thread and an interceptor's cancellation on an executor thread, while technically a data race from TSAN's perspective due to `ClientCallImpl.cancelCalled` not being volatile, is not consequential in practice because the underlying `stream.cancel()` and `CancellationHandler.tearDown()` are thread-safe and idempotent. Therefore, modifying `cancelCalled` in `ClientCallImpl` is not necessary. Internal TSAN test failure [link](http://shortn/_sh1PGgNkbQ)
1 parent 1053f4b commit 4ce53b1

1 file changed

Lines changed: 9 additions & 3 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14309,9 +14309,15 @@ public StreamObserver<ProcessingRequest> process(
1430914309
return new StreamObserver<ProcessingRequest>() {
1431014310
@Override
1431114311
public void onNext(ProcessingRequest request) {
14312-
responseObserver.onNext(ProcessingResponse.newBuilder()
14313-
.setRequestHeaders(HeadersResponse.newBuilder().build())
14314-
.build());
14312+
if (request.hasRequestHeaders()) {
14313+
responseObserver.onNext(ProcessingResponse.newBuilder()
14314+
.setRequestHeaders(HeadersResponse.newBuilder().build())
14315+
.build());
14316+
} else if (request.hasResponseHeaders()) {
14317+
responseObserver.onNext(ProcessingResponse.newBuilder()
14318+
.setResponseHeaders(HeadersResponse.newBuilder().build())
14319+
.build());
14320+
}
1431514321
}
1431614322

1431714323
@Override

0 commit comments

Comments
 (0)