Skip to content

Commit 46f3080

Browse files
authored
servlet: fix TomcatTransportTest detects write when not ready (#12732)
Fixes #12723 ### What was happening In `AsyncServletOutputStreamWriter#runOrBuffer`, the application thread relies on the cached `readyAndDrained `state to determine if it can write directly to the `ServletOutputStream`. In highly concurrent scenarios (observed in `TomcatTransportTest`), this cached state can become stale. The servlet container may have already transitioned to a 'not ready' state, but the corresponding callback has not yet updated gRPC's internal state. When the application thread attempts to write based on the stale `true `value, the container throws an `IllegalStateException`. ### 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`.
1 parent 073fd5e commit 46f3080

1 file changed

Lines changed: 34 additions & 18 deletions

File tree

servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ final class AsyncServletOutputStreamWriter {
7878
private final Queue<ActionItem> writeChain = new ConcurrentLinkedQueue<>();
7979
// for a theoretical race condition that onWritePossible() is called immediately after isReady()
8080
// returns false and before writeState.compareAndSet()
81+
8182
@Nullable
8283
private volatile Thread parkingThread;
8384

@@ -209,6 +210,13 @@ private void assureReadyAndDrainedTurnsFalse() {
209210
parkingThread = null;
210211
}
211212

213+
private void markNotReadyAndUnpark(WriteState curState) {
214+
boolean successful = writeState.compareAndSet(curState, curState.withReadyAndDrained(false));
215+
checkState(successful, "Bug: curState is unexpectedly changed by another thread");
216+
LockSupport.unpark(parkingThread);
217+
log.finest("the servlet output stream becomes not ready");
218+
}
219+
212220
/**
213221
* Either execute the write action directly, or buffer the action and let the container thread
214222
* drain it.
@@ -217,30 +225,38 @@ private void assureReadyAndDrainedTurnsFalse() {
217225
*/
218226
private void runOrBuffer(ActionItem actionItem) throws IOException {
219227
WriteState curState = writeState.get();
220-
if (curState.readyAndDrained) { // write to the outputStream directly
228+
229+
// Tomcat Spontaneous State Change Mitigation ---
230+
// If our cache says true, but the container is secretly not ready,
231+
// intercept the stale state and sync it before proceeding.
232+
if (curState.readyAndDrained && !isReady.getAsBoolean()) {
233+
markNotReadyAndUnpark(curState);
234+
// Update local state so it gracefully bypasses the
235+
// direct write and falls into the buffer block
236+
curState = writeState.get();
237+
}
238+
// -------------------------------------------------------
239+
if (curState.readyAndDrained) {
221240
actionItem.run();
222241
if (actionItem == completeAction) {
223242
return;
224243
}
225244
if (!isReady.getAsBoolean()) {
226-
boolean successful =
227-
writeState.compareAndSet(curState, curState.withReadyAndDrained(false));
228-
LockSupport.unpark(parkingThread);
229-
checkState(successful, "Bug: curState is unexpectedly changed by another thread");
230-
log.finest("the servlet output stream becomes not ready");
245+
markNotReadyAndUnpark(curState);
246+
}
247+
return;
248+
}
249+
250+
writeChain.offer(actionItem);
251+
if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) {
252+
checkState(
253+
writeState.get().readyAndDrained,
254+
"Bug: onWritePossible() should have changed readyAndDrained to true, but not");
255+
ActionItem lastItem = writeChain.poll();
256+
if (lastItem != null) {
257+
checkState(lastItem == actionItem, "Bug: lastItem != actionItem");
258+
runOrBuffer(lastItem);
231259
}
232-
} else { // buffer to the writeChain
233-
writeChain.offer(actionItem);
234-
if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) {
235-
checkState(
236-
writeState.get().readyAndDrained,
237-
"Bug: onWritePossible() should have changed readyAndDrained to true, but not");
238-
ActionItem lastItem = writeChain.poll();
239-
if (lastItem != null) {
240-
checkState(lastItem == actionItem, "Bug: lastItem != actionItem");
241-
runOrBuffer(lastItem);
242-
}
243-
} // state has not changed since
244260
}
245261
}
246262

0 commit comments

Comments
 (0)