Skip to content

Commit c7a946b

Browse files
Revert "servlet: fix write when not ready in AsyncServletOutputStreamWriter (#12867)
This reverts commit 243b79e. The approach taken of ensuring subsequent writes go through the container thread via onWritePossible() hasn't eliminated the need for catching the IllegalStateException (which you are catching and buffering the write if it happens). We have observed that the UndertowInteropTest now hangs instead of the earlier flaky failures due to uncaught exception. There has been another PR #12732 for the same issue that explicitly checks isReady.getAsBoolean() in addition to the cached state, that's probably most reliable, because you don't have to deal with callback scheduling and the state changing in the meantime. We would like to proceed with that PR and revert this one. @mgustimz
1 parent 4456721 commit c7a946b

2 files changed

Lines changed: 12 additions & 338 deletions

File tree

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

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,7 @@ private void assureReadyAndDrainedTurnsFalse() {
218218
private void runOrBuffer(ActionItem actionItem) throws IOException {
219219
WriteState curState = writeState.get();
220220
if (curState.readyAndDrained) { // write to the outputStream directly
221-
try {
222-
actionItem.run();
223-
} catch (IllegalStateException e) {
224-
if (actionItem == flushAction || actionItem == completeAction) {
225-
throw e;
226-
}
227-
buffer(actionItem, curState);
228-
return;
229-
}
221+
actionItem.run();
230222
if (actionItem == completeAction) {
231223
return;
232224
}
@@ -238,26 +230,20 @@ private void runOrBuffer(ActionItem actionItem) throws IOException {
238230
log.finest("the servlet output stream becomes not ready");
239231
}
240232
} else { // buffer to the writeChain
241-
buffer(actionItem, curState);
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
242244
}
243245
}
244246

245-
private void buffer(ActionItem actionItem, WriteState curState) throws IOException {
246-
writeChain.offer(actionItem);
247-
if (writeState.compareAndSet(curState, curState.withReadyAndDrained(false))) {
248-
LockSupport.unpark(parkingThread);
249-
} else {
250-
checkState(
251-
writeState.get().readyAndDrained,
252-
"Bug: onWritePossible() should have changed readyAndDrained to true, but not");
253-
ActionItem lastItem = writeChain.poll();
254-
if (lastItem != null) {
255-
checkState(lastItem == actionItem, "Bug: lastItem != actionItem");
256-
runOrBuffer(lastItem);
257-
}
258-
} // state has not changed since
259-
}
260-
261247
/** Write actions, e.g. writeBytes, flush, complete. */
262248
@FunctionalInterface
263249
@VisibleForTesting

servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java

Lines changed: 0 additions & 312 deletions
This file was deleted.

0 commit comments

Comments
 (0)