From 1da19dc2fdf794f0f23acc776be94ebba41b2640 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 15 Mar 2018 23:12:58 +1100 Subject: [PATCH] [WIP!] HTTP Close #1832 (#2338) * Only close if parser closed and output is shutdown Signed-off-by: Greg Wilkins * a better possible fix Signed-off-by: Greg Wilkins * after review Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/io/AbstractEndPoint.java | 8 ++++++++ .../main/java/org/eclipse/jetty/io/WriteFlusher.java | 2 ++ .../java/org/eclipse/jetty/server/HttpConnection.java | 9 ++------- .../eclipse/jetty/server/ConnectionOpenCloseTest.java | 11 ++++++----- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java index 5695e988e45..105bbd74584 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java @@ -62,6 +62,8 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint protected final void shutdownInput() { + if (LOG.isDebugEnabled()) + LOG.debug("shutdownInput {}",this); while(true) { State s = _state.get(); @@ -114,6 +116,8 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint @Override public final void shutdownOutput() { + if (LOG.isDebugEnabled()) + LOG.debug("shutdownOutput {}",this); while(true) { State s = _state.get(); @@ -166,11 +170,15 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint @Override public final void close() { + if (LOG.isDebugEnabled()) + LOG.debug("close {}",this); close(null); } protected final void close(Throwable failure) { + if (LOG.isDebugEnabled()) + LOG.debug("close({}) {}",failure,this); while(true) { State s = _state.get(); 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..86465065a79 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 @@ -496,6 +496,8 @@ abstract public class WriteFlusher public boolean onFail(Throwable cause) { // Keep trying to handle the failure until we get to IDLE or FAILED state + if (cause!=null) + cause.addSuppressed(new Throwable()); while (true) { State current = _state.get(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index 9577392af53..fc10d546594 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -243,6 +243,8 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http int filled = fillRequestBuffer(); if (filled>0) bytesIn.add(filled); + else if (filled==-1 && getEndPoint().isOutputShutdown()) + close(); // Parse the request buffer. boolean handle = parseRequestBuffer(); @@ -252,13 +254,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http if (getEndPoint().getConnection()!=this) break; - // Handle closed parser. - if (_parser.isClose() || _parser.isClosed()) - { - close(); - break; - } - // Handle channel event if (handle) { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectionOpenCloseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectionOpenCloseTest.java index 97bf92123de..f71c3a24496 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectionOpenCloseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ConnectionOpenCloseTest.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -42,9 +43,11 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.annotation.Slow; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -224,11 +227,9 @@ public class ConnectionOpenCloseTest extends AbstractHttpTest "\r\n").getBytes(StandardCharsets.UTF_8)); output.flush(); - InputStream inputStream = socket.getInputStream(); - HttpTester.Response response = HttpTester.parseResponse(inputStream); - Assert.assertEquals(200, response.getStatus()); - - Assert.assertEquals(-1, inputStream.read()); + // Read to EOF + String response = BufferUtil.toString(ByteBuffer.wrap(IO.readBytes(socket.getInputStream()))); + assertThat(response,Matchers.containsString("200 OK")); socket.close(); Assert.assertTrue(openLatch.await(5, TimeUnit.SECONDS));