Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,24 @@ private void runOrBuffer(ActionItem actionItem) throws IOException {
if (actionItem == completeAction) {
return;
}
if (!isReady.getAsBoolean()) {
if (actionItem == flushAction) {
// flush path: only set readyAndDrained=false if isReady() returns false
// If isReady() is still true, keep readyAndDrained=true so flush goes direct
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");
}
} else {
// writeBytes path: always set readyAndDrained=false even when isReady()
// returns true. Tomcat requires onWritePossible() to fire between writes.
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");
log.finest("direct write: cleared readyAndDrained, next writes buffered");
}
} else { // buffer to the writeChain
writeChain.offer(actionItem);
Expand Down Expand Up @@ -274,9 +286,10 @@ private static final class WriteState {
* check of {@link javax.servlet.ServletOutputStream#isReady()} is true.
*
* <p>readyAndDrained turns from true to false when:
* {@code runOrBuffer()} exits while either the action item is written directly to the
* servlet output stream and the check of {@link javax.servlet.ServletOutputStream#isReady()}
* right after that returns false, or the action item is buffered into the writeChain.
* {@code runOrBuffer()} exits after writing directly to the servlet output stream.
* For writeBytes actions, it always transitions to false. For flush actions,
* it transitions to false only when {@link javax.servlet.ServletOutputStream#isReady()}
* returns false, or when the action is buffered into the writeChain.
*/
final boolean readyAndDrained;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/*
* Copyright 2026 The gRPC Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.grpc.servlet;

import static org.junit.Assert.assertEquals;

import io.grpc.servlet.AsyncServletOutputStreamWriter.ActionItem;
import io.grpc.servlet.AsyncServletOutputStreamWriter.Log;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.function.BooleanSupplier;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/**
* Unit test for {@link AsyncServletOutputStreamWriter} with a mock isReady supplier.
* Tests two scenarios: (1) writes with isReady always true, where onWritePossible()
* is called between writes; (2) writes with readyAndDrained=false (triggered by first
* write), where subsequent writes are buffered until onWritePossible() drains them.
*/
@RunWith(JUnit4.class)
public class AsyncServletOutputStreamWriterTest {

/**
* Test that a write without an intervening onWritePossible() is buffered and
* does not execute immediately. This validates the Tomcat fix: writeBytes sets
* readyAndDrained=false so subsequent writes go through the container callback.
*/
Comment thread
mgustimz marked this conversation as resolved.
@Test
public void writeBytes_withoutOnWritePossible_isBuffered() throws IOException {
AtomicBoolean isReady = new AtomicBoolean(true);
List<String> actions = new ArrayList<>();

BiFunction<byte[], Integer, ActionItem> writeAction =
(bytes, numBytes) -> () -> {
actions.add("write");
};

ActionItem flushAction = () -> {};

ActionItem completeAction = () -> {};

BooleanSupplier isReadySupplier = () -> isReady.get();

AsyncServletOutputStreamWriter writer =
new AsyncServletOutputStreamWriter(writeAction, flushAction, completeAction, isReadySupplier, new Log() {});

// Initial onWritePossible to set readyAndDrained=true
writer.onWritePossible();

// First write - goes direct, readyAndDrained becomes false
byte[] data1 = new byte[]{1};
writer.writeBytes(data1, 1);
assertEquals("First write should execute", 1, actions.size());

// Second write without onWritePossible - should be buffered since readyAndDrained=false
byte[] data2 = new byte[]{2};
writer.writeBytes(data2, 1);

// Without onWritePossible, second write should be buffered, not executed
assertEquals("Second write should be buffered until onWritePossible", 1, actions.size());

// onWritePossible drains the buffered write
writer.onWritePossible();
assertEquals("Buffered write should drain after onWritePossible", 2, actions.size());
}

/**
* Test that multiple consecutive writes succeed when isReady() always returns true
* and the container calls onWritePossible() between writes.
*/
@Test
public void writeBytes_onWritePossibleBetweenWrites_succeeds() throws IOException {
AtomicBoolean isReady = new AtomicBoolean(true);
List<byte[]> writtenData = new ArrayList<>();

BiFunction<byte[], Integer, ActionItem> writeAction =
(bytes, numBytes) -> () -> {
writtenData.add(bytes);
};

ActionItem flushAction = () -> {};

ActionItem completeAction = () -> {};

BooleanSupplier isReadySupplier = () -> isReady.get();

AsyncServletOutputStreamWriter writer =
new AsyncServletOutputStreamWriter(writeAction, flushAction, completeAction, isReadySupplier, new Log() {});

// Initial onWritePossible to set readyAndDrained=true
writer.onWritePossible();

// Write 5 times, calling onWritePossible after each write.
// With isReady staying true, each onWritePossible should set readyAndDrained back to true,
// allowing the next write to go direct again.
for (int i = 0; i < 5; i++) {
byte[] data = new byte[]{(byte) i};
writer.writeBytes(data, 1);
writer.onWritePossible();
}

// Verify all 5 writes completed
assertEquals("All writes should complete", 5, writtenData.size());
}
Comment thread
mgustimz marked this conversation as resolved.

/**
* Test that when isReady() returns false after a direct write, subsequent writes
* are buffered and only drain when onWritePossible() is called.
*/
@Test
public void writeBytes_isReadyFalse_buffersUntilOnWritePossible() throws IOException {
AtomicBoolean isReady = new AtomicBoolean(true);
List<String> actions = new ArrayList<>();

BiFunction<byte[], Integer, ActionItem> writeAction =
(bytes, numBytes) -> () -> {
actions.add("write");
};

ActionItem flushAction = () -> {
actions.add("flush");
};

ActionItem completeAction = () -> {};

BooleanSupplier isReadySupplier = () -> isReady.get();

AsyncServletOutputStreamWriter writer =
new AsyncServletOutputStreamWriter(writeAction, flushAction, completeAction, isReadySupplier, new Log() {});

// Initial onWritePossible to set readyAndDrained=true
writer.onWritePossible();

// First write - goes direct, readyAndDrained becomes false
byte[] data1 = new byte[]{1};
writer.writeBytes(data1, 1);

// After first write, readyAndDrained=false, so any subsequent write is buffered
isReady.set(false);

// Second write - should be buffered since readyAndDrained=false
byte[] data2 = new byte[]{2};
writer.writeBytes(data2, 1);
Comment thread
mgustimz marked this conversation as resolved.

// Only the first write should have executed
assertEquals("First write should complete, second buffered", 1, actions.size());

// Container calls onWritePossible to drain buffered writes
isReady.set(true);
writer.onWritePossible();

// Now both writes should have completed
assertEquals("Both writes should complete after onWritePossible", 2, actions.size());
}
}
Loading