From 9bf9ad1966d8ead929d386758b5bfa20cf87fcfd Mon Sep 17 00:00:00 2001 From: themechbro Date: Thu, 26 Mar 2026 16:46:37 +0530 Subject: [PATCH 1/2] servlet: fix TomcatTransportTest detects write when not ready --- .../AsyncServletOutputStreamWriter.java | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java index 3c8d3d07571..3f86b974467 100644 --- a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java +++ b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java @@ -217,7 +217,11 @@ private void assureReadyAndDrainedTurnsFalse() { */ private void runOrBuffer(ActionItem actionItem) throws IOException { WriteState curState = writeState.get(); - if (curState.readyAndDrained) { // write to the outputStream directly + + // Evaluate Tomcat's actual state alongside our cached state + boolean actualReady = curState.readyAndDrained && isReady.getAsBoolean(); + + if (actualReady) { // write to the outputStream directly actionItem.run(); if (actionItem == completeAction) { return; @@ -232,13 +236,21 @@ private void runOrBuffer(ActionItem actionItem) throws IOException { } else { // buffer to the writeChain writeChain.offer(actionItem); if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) { - checkState( - writeState.get().readyAndDrained, - "Bug: onWritePossible() should have changed readyAndDrained to true, but not"); - ActionItem lastItem = writeChain.poll(); - if (lastItem != null) { - checkState(lastItem == actionItem, "Bug: lastItem != actionItem"); - runOrBuffer(lastItem); + // STATE CHANGED! Determine why the CAS failed based on our initial state. + if (curState.readyAndDrained) { + // We dropped here solely because isReady() was false. + // CAS failed because another concurrent thread already CAS'd it to false. + // This is completely safe. Tomcat will call onWritePossible(). Do nothing. + } else { + // Original logic: We started as false, CAS failed because onWritePossible set it to true. + checkState( + writeState.get().readyAndDrained, + "Bug: onWritePossible() should have changed readyAndDrained to true, but not"); + ActionItem lastItem = writeChain.poll(); + if (lastItem != null) { + checkState(lastItem == actionItem, "Bug: lastItem != actionItem"); + runOrBuffer(lastItem); + } } } // state has not changed since } From b2cde71196bf07a88326304c1fa1f15f5548bf2f Mon Sep 17 00:00:00 2001 From: themechbro Date: Sat, 28 Mar 2026 22:49:14 +0530 Subject: [PATCH 2/2] chore: trigger CI pipeline --- .../AsyncServletOutputStreamWriter.java | 72 +++++++++++-------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java index 3f86b974467..85972bf9461 100644 --- a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java +++ b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java @@ -217,42 +217,54 @@ private void assureReadyAndDrainedTurnsFalse() { */ private void runOrBuffer(ActionItem actionItem) throws IOException { WriteState curState = writeState.get(); - - // Evaluate Tomcat's actual state alongside our cached state - boolean actualReady = curState.readyAndDrained && isReady.getAsBoolean(); - if (actualReady) { // write to the outputStream directly - actionItem.run(); - if (actionItem == completeAction) { + if (curState.readyAndDrained) { + if (isReady.getAsBoolean()) { + // Path 1: Container is truly ready. Write directly. + actionItem.run(); + if (actionItem == completeAction) { + return; + } + if (!isReady.getAsBoolean()) { + boolean successful = + writeState.compareAndSet(curState, curState.withReadyAndDrained(false)); + LockSupport.unpark(parkingThread); + checkState(successful, "Bug: curState is unexpectedly changed by another thread"); + log.finest("the servlet output stream becomes not ready"); + } return; } - if (!isReady.getAsBoolean()) { - boolean successful = - writeState.compareAndSet(curState, curState.withReadyAndDrained(false)); + } + + // Path 2: Container is secretly not ready (Tomcat bug) OR already known to be false. + // We must safely buffer the item and ensure the state reflects reality. + writeChain.offer(actionItem); + if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) { + // CAS failed. State changed mid-flight. + if (curState.readyAndDrained) { + // Started as true, but CAS failed because another thread + // concurrently buffered and flipped it to false. + // Safe to do nothing. The winning thread handles the unpark. + } else { + // Started as false, CAS failed because onWritePossible flipped it to true. + // Original logic: retry the write since it's ready again. + checkState( + writeState.get().readyAndDrained, + "Bug: onWritePossible() should have changed readyAndDrained to true, but not"); + ActionItem lastItem = writeChain.poll(); + if (lastItem != null) { + checkState(lastItem == actionItem, "Bug: lastItem != actionItem"); + runOrBuffer(lastItem); + } + } + } else { + // CAS succeeded! + // CRITICAL FIX: If we just flipped the state from true to false, + // we MUST wake up the container! + if (curState.readyAndDrained) { LockSupport.unpark(parkingThread); - checkState(successful, "Bug: curState is unexpectedly changed by another thread"); log.finest("the servlet output stream becomes not ready"); } - } else { // buffer to the writeChain - writeChain.offer(actionItem); - if (!writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) { - // STATE CHANGED! Determine why the CAS failed based on our initial state. - if (curState.readyAndDrained) { - // We dropped here solely because isReady() was false. - // CAS failed because another concurrent thread already CAS'd it to false. - // This is completely safe. Tomcat will call onWritePossible(). Do nothing. - } else { - // Original logic: We started as false, CAS failed because onWritePossible set it to true. - checkState( - writeState.get().readyAndDrained, - "Bug: onWritePossible() should have changed readyAndDrained to true, but not"); - ActionItem lastItem = writeChain.poll(); - if (lastItem != null) { - checkState(lastItem == actionItem, "Bug: lastItem != actionItem"); - runOrBuffer(lastItem); - } - } - } // state has not changed since } }