Skip to content

Commit 8d49dc1

Browse files
authored
okhttp: Fix race condition overwriting MAX_CONCURRENT_STREAMS (#12548)
### What this PR does This PR fixes a race condition in `OkHttpClientTransport` where `MAX_CONCURRENT_STREAMS` sent by the server could be incorrectly overwritten by the client's default initialization. The fix simply reorders the initialization to happen **before** starting the reader thread, ensuring that any updates from the server are preserved. ### Note on Testing I attempted to add a deterministic reproduction test, but reliably simulating this specific race condition proved difficult without intrusive changes. I request reviewers to primarily verify the logical correctness of the reordering. I am open to collaborating with the team to develop a suitable test case if required. ### Future Work This PR covers **Step 1** (Fixing the race condition) of the plan discussed in #11985. I plan to follow up with **Step 2** (Adding assertions to verify no pending streams exist) in a separate PR. Part of #11985
1 parent b1a94a4 commit 8d49dc1

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -800,13 +800,13 @@ public void run() {
800800
if (connectingCallback != null) {
801801
connectingCallback.run();
802802
}
803-
// ClientFrameHandler need to be started after connectionPreface / settings, otherwise it
804-
// may send goAway immediately.
805-
executor.execute(clientFrameHandler);
806803
synchronized (lock) {
807804
maxConcurrentStreams = Integer.MAX_VALUE;
808805
startPendingStreams();
809806
}
807+
// ClientFrameHandler need to be started after connectionPreface / settings, otherwise it
808+
// may send goAway immediately.
809+
executor.execute(clientFrameHandler);
810810
if (connectedFuture != null) {
811811
connectedFuture.set(null);
812812
}

0 commit comments

Comments
 (0)