From cc1071fa053b0a87cff003b98d9594b657e38924 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Jul 2018 10:54:43 +0200 Subject: [PATCH] Fixes #2717 - Async requests are not considered when shutting down gracefully. Now using _requestStats instead of _dispatchedStats to check for requests completed when shutting down StatisticsHandler. Signed-off-by: Simone Bordet --- .../server/handler/StatisticsHandler.java | 2 +- .../server/handler/StatisticsHandlerTest.java | 88 +++++++++++++++---- 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java index 3bb9f43d34a9..38fee0bdc1b9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/StatisticsHandler.java @@ -584,7 +584,7 @@ public Future shutdown() FutureCallback shutdown=new FutureCallback(false); _shutdown.compareAndSet(null,shutdown); shutdown=_shutdown.get(); - if (_dispatchedStats.getCurrent()==0) + if (_requestStats.getCurrent()==0) shutdown.succeeded(); return shutdown; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java index c29f91f85eb9..fd593d623160 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/StatisticsHandlerTest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -31,6 +32,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.io.ConnectionStatistics; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; @@ -41,6 +43,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -85,7 +88,7 @@ public void testRequest() throws Exception _statsHandler.setHandler(new AbstractHandler() { @Override - public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException { request.setHandled(true); try @@ -97,7 +100,7 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, catch (Exception x) { Thread.currentThread().interrupt(); - throw (IOException)new IOException().initCause(x); + throw new IOException(x); } } }); @@ -182,7 +185,7 @@ public void testTwoRequests() throws Exception _statsHandler.setHandler(new AbstractHandler() { @Override - public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException { request.setHandled(true); try @@ -193,7 +196,7 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, catch (Exception x) { Thread.currentThread().interrupt(); - throw (IOException)new IOException().initCause(x); + throw new IOException(x); } } }); @@ -245,7 +248,7 @@ public void testSuspendResume() throws Exception _statsHandler.setHandler(new AbstractHandler() { @Override - public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws ServletException { request.setHandled(true); try @@ -306,22 +309,22 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, asyncHolder.get().addListener(new AsyncListener() { @Override - public void onTimeout(AsyncEvent event) throws IOException + public void onTimeout(AsyncEvent event) { } @Override - public void onStartAsync(AsyncEvent event) throws IOException + public void onStartAsync(AsyncEvent event) { } @Override - public void onError(AsyncEvent event) throws IOException + public void onError(AsyncEvent event) { } @Override - public void onComplete(AsyncEvent event) throws IOException + public void onComplete(AsyncEvent event) { try { @@ -375,7 +378,7 @@ public void testSuspendExpire() throws Exception _statsHandler.setHandler(new AbstractHandler() { @Override - public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws ServletException { request.setHandled(true); try @@ -428,23 +431,23 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, asyncHolder.get().addListener(new AsyncListener() { @Override - public void onTimeout(AsyncEvent event) throws IOException + public void onTimeout(AsyncEvent event) { event.getAsyncContext().complete(); } @Override - public void onStartAsync(AsyncEvent event) throws IOException + public void onStartAsync(AsyncEvent event) { } @Override - public void onError(AsyncEvent event) throws IOException + public void onError(AsyncEvent event) { } @Override - public void onComplete(AsyncEvent event) throws IOException + public void onComplete(AsyncEvent event) { try { @@ -491,7 +494,7 @@ public void testSuspendComplete() throws Exception _statsHandler.setHandler(new AbstractHandler() { @Override - public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException + public void handle(String path, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws ServletException { request.setHandled(true); try @@ -550,22 +553,22 @@ public void handle(String path, Request request, HttpServletRequest httpRequest, asyncHolder.get().addListener(new AsyncListener() { @Override - public void onTimeout(AsyncEvent event) throws IOException + public void onTimeout(AsyncEvent event) { } @Override - public void onStartAsync(AsyncEvent event) throws IOException + public void onStartAsync(AsyncEvent event) { } @Override - public void onError(AsyncEvent event) throws IOException + public void onError(AsyncEvent event) { } @Override - public void onComplete(AsyncEvent event) throws IOException + public void onComplete(AsyncEvent event) { try { @@ -601,6 +604,53 @@ public void onComplete(AsyncEvent event) throws IOException assertEquals(_statsHandler.getDispatchedTimeTotal(), _statsHandler.getDispatchedTimeMean(), 0.01); } + @Test + public void testAsyncRequestWithShutdown() throws Exception + { + long delay = 500; + CountDownLatch serverLatch = new CountDownLatch(1); + _statsHandler.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + { + AsyncContext asyncContext = request.startAsync(); + asyncContext.setTimeout(0); + new Thread(() -> + { + try + { + Thread.sleep(delay); + asyncContext.complete(); + } + catch (InterruptedException e) + { + response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + asyncContext.complete(); + } + }).start(); + serverLatch.countDown(); + } + }); + _server.start(); + + String request = "GET / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "\r\n"; + _connector.executeRequest(request); + + assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); + + Future shutdown = _statsHandler.shutdown(); + assertFalse(shutdown.isDone()); + + Thread.sleep(delay / 2); + assertFalse(shutdown.isDone()); + + Thread.sleep(delay); + assertTrue(shutdown.isDone()); + } + /** * This handler is external to the statistics handler and it is used to ensure that statistics handler's * handle() is fully executed before asserting its values in the tests, to avoid race conditions with the