From fd488429e5bba5a5c2eff6dd853d3c884a570297 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 13 Jul 2026 18:47:10 +0530 Subject: [PATCH 1/3] servlet: fix UndertowTransportTest flakiness by eliminating container thread parking deadlocks (#12778) Fixes a long-standing flakiness bug in UndertowTransportTest.basicStream where the container worker thread would park for up to 1 minute in AsyncServletOutputStreamWriter.assureReadyAndDrainedTurnsFalse(), causing the test listener poll to time out after 10 seconds and fail with AssertionError: message expected. Root Cause: AsyncServletOutputStreamWriter previously assumed that readyAndDrained could only be true while onWritePossible() was actively executing and that any call to runOrBuffer() would return readyAndDrained back to false. However, when written payloads fit inside the servlet output buffer, outputStream.isReady() remains true after runOrBuffer() finishes, so runOrBuffer() exits leaving readyAndDrained == true. When Undertow subsequently dispatched WriteListener.onWritePossible() after async I/O write completion, onWritePossible() saw readyAndDrained == true and invoked assureReadyAndDrainedTurnsFalse(), parking the Undertow container thread via LockSupport.parkNanos(1 min). Because the application thread had already returned, no thread was running runOrBuffer() to clear readyAndDrained, leaving Undertow blocked for 60 seconds and causing the test timeout. Fix: 1. Removed assureReadyAndDrainedTurnsFalse() and LockSupport parking logic. A state of readyAndDrained == true with an empty writeChain and isReady() == true is a valid quiescent state; onWritePossible() verifies the queue is empty and exits cleanly without parking. 2. Safe state updates in runOrBuffer() when outputStream is not ready. Fixes #12778 --- .../AsyncServletOutputStreamWriter.java | 23 +------------------ 1 file changed, 1 insertion(+), 22 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..3fe1a5be416 100644 --- a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java +++ b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java @@ -76,10 +76,6 @@ final class AsyncServletOutputStreamWriter { */ // SPSC queue would do private final Queue writeChain = new ConcurrentLinkedQueue<>(); - // for a theoretical race condition that onWritePossible() is called immediately after isReady() - // returns false and before writeState.compareAndSet() - @Nullable - private volatile Thread parkingThread; AsyncServletOutputStreamWriter( AsyncContext asyncContext, @@ -173,9 +169,6 @@ void complete() { /** Called from the container thread {@link javax.servlet.WriteListener#onWritePossible()}. */ void onWritePossible() throws IOException { log.finest("onWritePossible: ENTRY. The servlet output stream becomes ready"); - if (writeState.get().readyAndDrained) { - assureReadyAndDrainedTurnsFalse(); - } while (isReady.getAsBoolean()) { WriteState curState = writeState.get(); @@ -198,17 +191,6 @@ void onWritePossible() throws IOException { log.finest("onWritePossible: EXIT. The servlet output stream becomes not ready"); } - private void assureReadyAndDrainedTurnsFalse() { - // readyAndDrained should have been set to false already. - // Just in case due to a race condition readyAndDrained is still true at this moment and is - // being set to false by runOrBuffer() concurrently. - parkingThread = Thread.currentThread(); - while (writeState.get().readyAndDrained) { - LockSupport.parkNanos(TimeUnit.MINUTES.toNanos(1)); // should return immediately - } - parkingThread = null; - } - /** * Either execute the write action directly, or buffer the action and let the container thread * drain it. @@ -223,10 +205,7 @@ private void runOrBuffer(ActionItem actionItem) throws IOException { 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"); + writeState.set(curState.withReadyAndDrained(false)); log.finest("the servlet output stream becomes not ready"); } } else { // buffer to the writeChain From 8d4dc8e8b561acfa2db29db3e905b9a6f221d295 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 13 Jul 2026 18:55:51 +0530 Subject: [PATCH 2/3] servlet: add unit test verifying AsyncServletOutputStreamWriter.onWritePossible non-blocking behavior (#12778) --- .../AsyncServletOutputStreamWriterTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java diff --git a/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java b/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java new file mode 100644 index 00000000000..fba5e8bb19b --- /dev/null +++ b/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java @@ -0,0 +1,56 @@ +/* + * 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.assertTrue; + +import io.grpc.servlet.AsyncServletOutputStreamWriter.ActionItem; +import io.grpc.servlet.AsyncServletOutputStreamWriter.Log; +import java.io.IOException; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link AsyncServletOutputStreamWriter}. */ +@RunWith(JUnit4.class) +public class AsyncServletOutputStreamWriterTest { + + @Test(timeout = 5000) + public void onWritePossibleWhenAlreadyReadyAndDrainedReturnsImmediately() throws IOException { + AtomicBoolean written = new AtomicBoolean(false); + ActionItem writeAction = () -> written.set(true); + + AsyncServletOutputStreamWriter writer = + new AsyncServletOutputStreamWriter( + (bytes, len) -> writeAction, + () -> {}, + () -> {}, + () -> true, + new Log() {}); + + // Initial onWritePossible call turns readyAndDrained to true + writer.onWritePossible(); + + long startTime = System.nanoTime(); + // Subsequent onWritePossible call when readyAndDrained is already true must return non-blockingly + writer.onWritePossible(); + long durationMs = (System.nanoTime() - startTime) / 1_000_000; + + assertTrue("onWritePossible must not park or block thread", durationMs < 1000); + } +} From 6a3256201dedb32d220d143da7cfa358a6607016 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 13 Jul 2026 19:12:08 +0530 Subject: [PATCH 3/3] servlet: fix checkstyle formatting for AsyncServletOutputStreamWriter (#12778) --- .../io/grpc/servlet/AsyncServletOutputStreamWriter.java | 3 --- .../grpc/servlet/AsyncServletOutputStreamWriterTest.java | 8 ++++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java index 3fe1a5be416..be9452daeda 100644 --- a/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java +++ b/servlet/src/main/java/io/grpc/servlet/AsyncServletOutputStreamWriter.java @@ -28,14 +28,11 @@ import java.io.IOException; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.LockSupport; import java.util.function.BiFunction; import java.util.function.BooleanSupplier; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.servlet.AsyncContext; import javax.servlet.ServletOutputStream; diff --git a/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java b/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java index fba5e8bb19b..dce14ac50d0 100644 --- a/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java +++ b/servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java @@ -38,16 +38,16 @@ public void onWritePossibleWhenAlreadyReadyAndDrainedReturnsImmediately() throws AsyncServletOutputStreamWriter writer = new AsyncServletOutputStreamWriter( (bytes, len) -> writeAction, - () -> {}, - () -> {}, + () -> { }, + () -> { }, () -> true, - new Log() {}); + new Log() { }); // Initial onWritePossible call turns readyAndDrained to true writer.onWritePossible(); long startTime = System.nanoTime(); - // Subsequent onWritePossible call when readyAndDrained is already true must return non-blockingly + // Subsequent call when readyAndDrained is true must return non-blockingly writer.onWritePossible(); long durationMs = (System.nanoTime() - startTime) / 1_000_000;