From 1bf5128e6d8736fb3bd62525dfae1bd6a7c7eba8 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 31 May 2018 19:41:34 +1000 Subject: [PATCH 1/3] Fixes #2592 - changes to fix ServerTimeoutsTest.testAsyncWriteIdleTimeoutFires[transport: HTTP] on windows Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/server/HttpOutput.java | 2 +- .../jetty/http/client/ServerTimeoutsTest.java | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 1cc0e1fc732..49ef35ea636 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -1044,7 +1044,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (LOG.isDebugEnabled()) LOG.debug("onError", th); _writeListener.onError(th); - close(); + abort(th); return; } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java index 8a8f6bae1c9..49d66a6757c 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java @@ -143,6 +143,7 @@ public class ServerTimeoutsTest extends AbstractTest { if (t instanceof TimeoutException) response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + asyncContext.complete(); } }); @@ -494,9 +495,10 @@ public class ServerTimeoutsTest extends AbstractTest if (failure instanceof TimeoutException) { response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); - asyncContext.complete(); handlerLatch.countDown(); } + + asyncContext.complete(); } }); } @@ -542,17 +544,17 @@ public class ServerTimeoutsTest extends AbstractTest @Override public void onWritePossible() throws IOException { - output.write(new byte[64 * 1024 * 1024]); + if (output.isReady()) + output.write(new byte[64 * 1024 * 1024]); } @Override public void onError(Throwable failure) { if (failure instanceof TimeoutException) - { - asyncContext.complete(); handlerLatch.countDown(); - } + + asyncContext.complete(); } }); } @@ -749,9 +751,10 @@ public class ServerTimeoutsTest extends AbstractTest if (failure instanceof TimeoutException) { response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); - asyncContext.complete(); handlerLatch.countDown(); } + + asyncContext.complete(); } }); } From 72dcfc15e52c04c6ed7747aefcb62f22bc80935d Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 5 Jun 2018 19:21:30 +1000 Subject: [PATCH 2/3] Issue #2592 - Failing test on Windows: ServerTimeoutsTest.testAsyncWriteIdleTimeoutFires[transport: HTTP] changed write flusher to go from pending state to failed state reverted previous HttpOutput changes Signed-off-by: Lachlan Roberts --- .../src/main/java/org/eclipse/jetty/io/WriteFlusher.java | 4 ++-- .../src/main/java/org/eclipse/jetty/server/HttpOutput.java | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java index c9b56d4ecdf..6c580bfe7bc 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java @@ -59,7 +59,7 @@ abstract public class WriteFlusher // fill the state machine __stateTransitions.put(StateType.IDLE, EnumSet.of(StateType.WRITING)); __stateTransitions.put(StateType.WRITING, EnumSet.of(StateType.IDLE, StateType.PENDING, StateType.FAILED)); - __stateTransitions.put(StateType.PENDING, EnumSet.of(StateType.COMPLETING, StateType.IDLE)); + __stateTransitions.put(StateType.PENDING, EnumSet.of(StateType.COMPLETING, StateType.IDLE, StateType.FAILED)); __stateTransitions.put(StateType.COMPLETING, EnumSet.of(StateType.IDLE, StateType.PENDING, StateType.FAILED)); __stateTransitions.put(StateType.FAILED, EnumSet.of(StateType.IDLE)); } @@ -512,7 +512,7 @@ abstract public class WriteFlusher LOG.debug("failed: " + this, cause); PendingState pending = (PendingState)current; - if (updateState(pending, __IDLE)) + if (updateState(pending, new FailedState(cause))) return pending.fail(cause); break; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 49ef35ea636..8af7217ae18 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -26,7 +26,6 @@ import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritePendingException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; - import javax.servlet.RequestDispatcher; import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; @@ -283,7 +282,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable IOException ex = new IOException("Closed while Pending/Unready"); LOG.warn(ex.toString()); LOG.debug(ex); - _channel.abort(ex); + abort(ex); return; } default: @@ -1044,7 +1043,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (LOG.isDebugEnabled()) LOG.debug("onError", th); _writeListener.onError(th); - abort(th); + close(); return; } } From 29c9afe135a35bbb5409dc1da38e5bfd1c5c8fef Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 6 Jun 2018 15:35:54 +1000 Subject: [PATCH 3/3] Issue #2592 - Failing test on Windows: ServerTimeoutsTest.testAsyncWriteIdleTimeoutFires[transport: HTTP] removed HttpOutput.close(Closeable) method as IO.close(Closeable) should be used instead added isFailed() method to WriteFlusher and used it to fix WriteFlusherTest.testFailWhileBlocking() surrounded usage of onError() in HttpOutput.run() with try-finally so that IO.close(this) is executed if onError throws Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/io/WriteFlusher.java | 5 ++++ .../eclipse/jetty/io/WriteFlusherTest.java | 2 +- .../org/eclipse/jetty/server/HttpOutput.java | 30 ++++++++----------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java index 6c580bfe7bc..b3e8c358386 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java @@ -532,6 +532,11 @@ abstract public class WriteFlusher onFail(new ClosedChannelException()); } + boolean isFailed() + { + return _state.get().getType() == StateType.FAILED; + } + boolean isIdle() { return _state.get().getType() == StateType.IDLE; diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java index f472544c13c..61280f224e6 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java @@ -252,7 +252,7 @@ public class WriteFlusherTest Assert.assertEquals(reason, cause.getMessage()); } Assert.assertEquals("", endPoint.takeOutputString()); - Assert.assertTrue(flusher.isIdle()); + Assert.assertTrue(flusher.isFailed()); } @Test diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 8af7217ae18..6fcd6370435 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.server; -import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; @@ -36,6 +35,7 @@ import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.IteratingNestedCallback; import org.eclipse.jetty.util.SharedBlockingCallback; @@ -1042,8 +1042,16 @@ public class HttpOutput extends ServletOutputStream implements Runnable _onError = null; if (LOG.isDebugEnabled()) LOG.debug("onError", th); - _writeListener.onError(th); - close(); + + try + { + _writeListener.onError(th); + } + finally + { + IO.close(this); + } + return; } } @@ -1078,18 +1086,6 @@ public class HttpOutput extends ServletOutputStream implements Runnable } } - private void close(Closeable resource) - { - try - { - resource.close(); - } - catch (Throwable x) - { - LOG.ignore(x); - } - } - @Override public String toString() { @@ -1331,7 +1327,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { abort(x); _channel.getByteBufferPool().release(_buffer); - HttpOutput.this.close(_in); + IO.close(_in); super.onCompleteFailure(x); } } @@ -1391,7 +1387,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { abort(x); _channel.getByteBufferPool().release(_buffer); - HttpOutput.this.close(_in); + IO.close(_in); super.onCompleteFailure(x); } }