Skip to content

Commit 60e9f16

Browse files
committed
servlet: fix write when not ready, narrow to writeBytes only
Apply the readiness fix only to writeBytes actions, not flush. Flush can still go direct when isReady is true since it is less latency-sensitive. This prevents potential stalling of responses when a follow-up flush would be queued behind onWritePossible. Also update WriteState docs to reflect the new behavior. Fixes #12723
1 parent 3f87103 commit 60e9f16

1 file changed

Lines changed: 26 additions & 8 deletions

File tree

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

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,28 @@ private void runOrBuffer(ActionItem actionItem) throws IOException {
222222
if (actionItem == completeAction) {
223223
return;
224224
}
225-
boolean successful =
226-
writeState.compareAndSet(curState, curState.withReadyAndDrained(false));
227-
LockSupport.unpark(parkingThread);
228-
checkState(successful, "Bug: curState is unexpectedly changed by another thread");
229-
log.finest("the servlet output stream becomes not ready");
225+
if (actionItem == writeAction) {
226+
// For writeBytes, always set readyAndDrained to false even when isReady()
227+
// returns true. Tomcat requires onWritePossible() to fire between writes,
228+
// even if isReady() is still true. For flush, keep the original behavior
229+
// since flush is less latency-sensitive and can safely wait for
230+
// onWritePossible.
231+
boolean successful =
232+
writeState.compareAndSet(curState, curState.withReadyAndDrained(false));
233+
LockSupport.unpark(parkingThread);
234+
checkState(successful, "Bug: curState is unexpectedly changed by another thread");
235+
log.finest("the servlet output stream becomes not ready");
236+
} else {
237+
// For flush, only set to false if isReady() returns false.
238+
// If isReady() is still true, keep readyAndDrained true so flush goes direct.
239+
if (!isReady.getAsBoolean()) {
240+
boolean successful =
241+
writeState.compareAndSet(curState, curState.withReadyAndDrained(false));
242+
LockSupport.unpark(parkingThread);
243+
checkState(successful, "Bug: curState is unexpectedly changed by another thread");
244+
log.finest("the servlet output stream becomes not ready");
245+
}
246+
}
230247
} else { // buffer to the writeChain
231248
writeChain.offer(actionItem);
232249
if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) {
@@ -272,9 +289,10 @@ private static final class WriteState {
272289
* check of {@link javax.servlet.ServletOutputStream#isReady()} is true.
273290
*
274291
* <p>readyAndDrained turns from true to false when:
275-
* {@code runOrBuffer()} exits while either the action item is written directly to the
276-
* servlet output stream and the check of {@link javax.servlet.ServletOutputStream#isReady()}
277-
* right after that returns false, or the action item is buffered into the writeChain.
292+
* {@code runOrBuffer()} exits after writing directly to the servlet output stream.
293+
* For writeBytes actions, it always transitions to false. For flush actions,
294+
* it transitions to false only when {@link javax.servlet.ServletOutputStream#isReady()}
295+
* returns false, or when the action is buffered into the writeChain.
278296
*/
279297
final boolean readyAndDrained;
280298

0 commit comments

Comments
 (0)