diff --git a/Jenkinsfile b/Jenkinsfile index 78999ded56d..822859a218c 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -40,17 +40,6 @@ pipeline { } } - stage("Build / Test - JDK12") { - agent { node { label 'linux' } } - steps { - timeout(time: 120, unit: 'MINUTES') { - mavenBuild("jdk12", "-Pmongodb install", "maven3", true) - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] - junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' - } - } - } - stage("Build / Test - JDK13") { agent { node { label 'linux' } } steps { diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index f55569858c1..e1d4e13b497 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -78,7 +78,7 @@ public class HttpGenerator FLUSH, // The buffers previously generated should be flushed CONTINUE, // Continue generating the message SHUTDOWN_OUT, // Need EOF to be signaled - DONE // Message generation complete + DONE // The current phase of generation is complete } // other statics 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 3ad7cf25ec8..8ff9e822032 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 @@ -756,9 +756,9 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http { HttpGenerator.Result result = _generator.generateResponse(_info, _head, _header, chunk, _content, _lastContent); if (LOG.isDebugEnabled()) - LOG.debug("{} generate: {} ({},{},{})@{}", - this, + LOG.debug("generate: {} for {} ({},{},{})@{}", result, + this, BufferUtil.toSummaryString(_header), BufferUtil.toSummaryString(_content), _lastContent, @@ -849,8 +849,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http } case DONE: { - // If shutdown after commit, we can still close here. - if (getConnector().isShutdown()) + // If this is the end of the response and the connector was shutdown after response was committed, + // we can't add the Connection:close header, but we are still allowed to close the connection + // by shutting down the output. + if (getConnector().isShutdown() && _generator.isEnd() && _generator.isPersistent()) _shutdownOut = true; return Action.SUCCEEDED; 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 0d5c5b8ac21..88c7d839faf 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 @@ -583,16 +583,18 @@ public class HttpOutput extends ServletOutputStream implements Runnable // handle blocking write // Should we aggregate? + // Yes - if the write is smaller than the commitSize (==aggregate buffer size) + // and the write is not the last one, or is last but will fit in an already allocated aggregate buffer. boolean last = isLastContentToWrite(len); - if (!last && len <= _commitSize) + if (len <= _commitSize && (!last || len <= BufferUtil.space(_aggregate))) { acquireBuffer(); // YES - fill the aggregate with content from the buffer int filled = BufferUtil.fill(_aggregate, b, off, len); - // return if we are not complete, not full and filled all the content - if (filled == len && !BufferUtil.isFull(_aggregate)) + // return if we are not the last write and have aggregated all of the content + if (!last && filled == len && !BufferUtil.isFull(_aggregate)) return; // adjust offset/length diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java index 6a0e4369f5a..f4a55a05179 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/GracefulStopTest.java @@ -56,11 +56,13 @@ import org.junit.jupiter.api.condition.DisabledOnOs; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -166,6 +168,66 @@ public class GracefulStopTest client.close(); } + + /** + * Test completed writes during shutdown do not close output + * @throws Exception on test failure + */ + @Test + public void testWriteDuringShutdown() throws Exception + { + Server server = new Server(); + server.setStopTimeout(1000); + + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + ABHandler handler = new ABHandler(); + StatisticsHandler stats = new StatisticsHandler(); + server.setHandler(stats); + stats.setHandler(handler); + + server.start(); + + Thread stopper = new Thread(() -> + { + try + { + handler.latchA.await(); + server.stop(); + } + catch (Exception e) + { + e.printStackTrace(); + } + }); + stopper.start(); + + final int port = connector.getLocalPort(); + try(Socket client = new Socket("127.0.0.1", port)) + { + client.getOutputStream().write(( + "GET / HTTP/1.1\r\n" + + "Host: localhost:" + port + "\r\n" + + "\r\n" + ).getBytes()); + client.getOutputStream().flush(); + + while (!connector.isShutdown()) + Thread.sleep(10); + + handler.latchB.countDown(); + + String response = IO.toString(client.getInputStream()); + assertThat(response, startsWith("HTTP/1.1 200 ")); + assertThat(response, containsString("Content-Length: 2")); + assertThat(response, containsString("Connection: close")); + assertThat(response, endsWith("ab")); + } + stopper.join(); + } + /** * Test of standard graceful timeout mechanism when a block request does * complete. Note that even though the request completes after 100ms, the @@ -736,6 +798,30 @@ public class GracefulStopTest } } + static class ABHandler extends AbstractHandler + { + final CountDownLatch latchA = new CountDownLatch(1); + final CountDownLatch latchB = new CountDownLatch(1); + + @Override + public void handle(String s, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentLength(2); + response.getOutputStream().write("a".getBytes()); + try + { + latchA.countDown(); + latchB.await(); + } + catch (InterruptedException e) + { + throw new RuntimeException(e); + } + response.flushBuffer(); + response.getOutputStream().write("b".getBytes()); + } + } + static class TestHandler extends AbstractHandler { final CountDownLatch latch = new CountDownLatch(1); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java index 2660ee326fb..7408e042e17 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java @@ -21,17 +21,39 @@ package org.eclipse.jetty.util.component; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; -/* A Lifecycle that can be gracefully shutdown. +/** + *

Jetty components that wish to be part of a Graceful shutdown implement this interface so that + * the {@link Graceful#shutdown()} method will be called to initiate a shutdown. Shutdown operations + * can fall into the following categories:

+ * + *

The {@link Future} returned by the the shutdown call will be completed to indicate the shutdown operation is completed. + * Some shutdown operations may be instantaneous and always return a completed future. + *

+ * Graceful shutdown is typically orchestrated by the doStop methods of Server or ContextHandler (for a full or partial + * shutdown respectively). + *

*/ public interface Graceful { - public Future shutdown(); + Future shutdown(); - public boolean isShutdown(); + boolean isShutdown(); - public static class Shutdown implements Graceful + /** + * A utility Graceful that uses a {@link FutureCallback} to indicate if shutdown is completed. + * By default the {@link FutureCallback} is returned as already completed, but the {@link #newShutdownCallback()} method + * can be overloaded to return a non-completed callback that will require a {@link Callback#succeeded()} or + * {@link Callback#failed(Throwable)} call to be completed. + */ + class Shutdown implements Graceful { private final AtomicReference _shutdown = new AtomicReference<>(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java index 0c1302b0bab..dafc7ad0bb2 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java @@ -42,6 +42,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class SuspendResumeTest @@ -192,8 +193,7 @@ public class SuspendResumeTest assertNull(clientSocket.error); assertNull(serverSocket.error); - // suspend the client so that no read events occur - SuspendToken suspendToken = clientSocket.session.suspend(); - suspendToken.resume(); + // suspend after closed throws ISE + assertThrows(IllegalStateException.class, () -> clientSocket.session.suspend()); } }