Repository navigation
xds: Use real ScheduledExecutorService in ext_proc draining test to avoid FakeClock TSAN race - #13091
Open
kannanjgithub wants to merge 5 commits into
Open
kannanjgithub wants to merge 5 commits into
kannanjgithub wants to merge 5 commits into
Conversation
Only add RawMessageClientInterceptor to the interceptor chain in XdsNameResolver when either GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_CLIENT or GRPC_EXPERIMENTAL_XDS_EXT_PROC_ON_SERVER is true. This prevents RawMessageClientInterceptor from corrupting method descriptors and causing empty payloads on retry attempts for regular xDS channels. Fixes grpc#13010 TAG=agy CONV=56c87717-243d-4f12-af9e-c532e509892a
…void 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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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(...). BecauseFakeClockis not thread-safe, thisresulted in a TSAN data race on
FakeClock.currentTimeNanos.This change replaces the FakeClock-backed scheduler with a dedicated single-thread
ScheduledExecutorService(realScheduler), configures the in-process server/channel withdirectExecutor(), and relies on explicit latch synchronization instead ofFakeClockticks, eliminating the data race.
Fixes http://shortn/_IBJaU5B59O.