Skip to content

servlet: fix TomcatTransportTest detects write when not ready#12732

Open
themechbro wants to merge 1 commit intogrpc:masterfrom
themechbro:fix/tomcat-async-write-ready
Open

servlet: fix TomcatTransportTest detects write when not ready#12732
themechbro wants to merge 1 commit intogrpc:masterfrom
themechbro:fix/tomcat-async-write-ready

Conversation

@themechbro
Copy link
Contributor

Fixes #12723

What was happening

In AsyncServletOutputStreamWriter#runOrBuffer, the application thread was blindly trusting the cached readyAndDrained boolean state. Under certain asynchronous conditions (e.g., TCP window shrinking), Tomcat's internal buffer would fill up, but readyAndDrained was still true. Bypassing the live isReady() check caused the container to throw an IllegalStateException when actionItem.run() was executed.

The Fix

This PR updates the primary execution path in runOrBuffer to explicitly evaluate isReady.getAsBoolean() alongside the cached state.

If the cached state is true but the container is actually not ready, the thread gracefully drops into the else block, buffers the action into the writeChain, and attempts to update readyAndDrained to false.

Handling Concurrent CAS Failures

By forcing threads into the else block when isReady() evaluates to false, we introduce a safe race condition: multiple threads might try to execute writeState.compareAndSet(true, false) simultaneously.

To prevent the losing threads from crashing on the subsequent checkState, an explicit check for the initial state (if (curState.readyAndDrained)) was added inside the CAS failure block. If a thread started as true but failed the CAS, it simply means another concurrent thread already safely toggled the state to false. The thread performs a safe no-op and exits, allowing Tomcat's onWritePossible() to eventually drain the queue.

Passed local testing via:
./gradlew :grpc-servlet:tomcat9Test --tests "*TomcatTransportTest*" -PskipAndroid=true -PskipCodegen=true

@themechbro themechbro force-pushed the fix/tomcat-async-write-ready branch from 97ef4d7 to 9bf9ad1 Compare March 26, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

servlet: TomcatTransportTest detects write when not ready

1 participant