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 <simone.bordet@gmail.com>
This commit is contained in:
parent
9c15796403
commit
cc1071fa05
|
@ -584,7 +584,7 @@ public class StatisticsHandler extends HandlerWrapper implements Graceful
|
|||
FutureCallback shutdown=new FutureCallback(false);
|
||||
_shutdown.compareAndSet(null,shutdown);
|
||||
shutdown=_shutdown.get();
|
||||
if (_dispatchedStats.getCurrent()==0)
|
||||
if (_requestStats.getCurrent()==0)
|
||||
shutdown.succeeded();
|
||||
return shutdown;
|
||||
}
|
||||
|
|
|
@ -21,6 +21,7 @@ package org.eclipse.jetty.server.handler;
|
|||
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.ServletException;
|
|||
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 org.junit.Test;
|
|||
|
||||
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 class StatisticsHandlerTest
|
|||
_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 class StatisticsHandlerTest
|
|||
catch (Exception x)
|
||||
{
|
||||
Thread.currentThread().interrupt();
|
||||
throw (IOException)new IOException().initCause(x);
|
||||
throw new IOException(x);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
@ -182,7 +185,7 @@ public class StatisticsHandlerTest
|
|||
_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 class StatisticsHandlerTest
|
|||
catch (Exception x)
|
||||
{
|
||||
Thread.currentThread().interrupt();
|
||||
throw (IOException)new IOException().initCause(x);
|
||||
throw new IOException(x);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
@ -245,7 +248,7 @@ public class StatisticsHandlerTest
|
|||
_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 class StatisticsHandlerTest
|
|||
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 class StatisticsHandlerTest
|
|||
_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 class StatisticsHandlerTest
|
|||
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 class StatisticsHandlerTest
|
|||
_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 class StatisticsHandlerTest
|
|||
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 class StatisticsHandlerTest
|
|||
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<Void> 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
|
||||
|
|
Loading…
Reference in New Issue