okhttp: Fix race condition overwriting MAX_CONCURRENT_STREAMS (#12548)

becomeStar · web-flow · commit 8d49dc1c9129 · 2025-12-05T16:41:46.000+05:30
### 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
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
@@ -800,13 +800,13 @@ public void run() {
         if (connectingCallback != null) {
           connectingCallback.run();
         }
-        // ClientFrameHandler need to be started after connectionPreface / settings, otherwise it
-        // may send goAway immediately.
-        executor.execute(clientFrameHandler);
         synchronized (lock) {
           maxConcurrentStreams = Integer.MAX_VALUE;
           startPendingStreams();
         }
+        // ClientFrameHandler need to be started after connectionPreface / settings, otherwise it
+        // may send goAway immediately.
+        executor.execute(clientFrameHandler);
         if (connectedFuture != null) {
           connectedFuture.set(null);
         }

Original file line number	Diff line number	Diff line change
`@@ -800,13 +800,13 @@ public void run() {`
`800`	`800`	`if (connectingCallback != null) {`
`801`	`801`	`connectingCallback.run();`
`802`	`802`	`}`
`803`		`- // ClientFrameHandler need to be started after connectionPreface / settings, otherwise it`
`804`		`- // may send goAway immediately.`
`805`		`- executor.execute(clientFrameHandler);`
`806`	`803`	`synchronized (lock) {`
`807`	`804`	`maxConcurrentStreams = Integer.MAX_VALUE;`
`808`	`805`	`startPendingStreams();`
`809`	`806`	`}`
	`807`	`+ // ClientFrameHandler need to be started after connectionPreface / settings, otherwise it`
	`808`	`+ // may send goAway immediately.`
	`809`	`+ executor.execute(clientFrameHandler);`
`810`	`810`	`if (connectedFuture != null) {`
`811`	`811`	`connectedFuture.set(null);`
`812`	`812`	`}`