Fix hang caused by race condition in test for reset of kept-alive connection

Previously, a race between the server starting to reject requests
on a kept-alive connection and the request reaching the blocking
servlet could result in a response never being sent.

This commit updates the test to disable blocking once graceful
shutdown with an in-flight request has being. Awaitility is then used
to make a request on an idle kept-alive connection until it fails
due to the connection reset. This may not happen immediately due to
the aforementioned race.
pull/21440/head
Andy Wilkinson 5 years ago
parent 453ca01338
commit 86e6ec04b2

@ -66,6 +66,7 @@ import org.apache.jasper.servlet.JspServlet;
import org.apache.tomcat.JarScanFilter; import org.apache.tomcat.JarScanFilter;
import org.apache.tomcat.JarScanType; import org.apache.tomcat.JarScanType;
import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
@ -599,17 +600,21 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory
assertThat(keepAliveRequest.get()).isInstanceOf(HttpResponse.class); assertThat(keepAliveRequest.get()).isInstanceOf(HttpResponse.class);
Future<Object> request = initiateGetRequest(port, "/blocking"); Future<Object> request = initiateGetRequest(port, "/blocking");
blockingServlet.awaitQueue(); blockingServlet.awaitQueue();
blockingServlet.setBlocking(false);
this.webServer.shutDownGracefully((result) -> { this.webServer.shutDownGracefully((result) -> {
}); });
Object idleConnectionRequestResult = Awaitility.await().until(() -> {
Future<Object> idleConnectionRequest = initiateGetRequest(httpClient, port, "/blocking"); Future<Object> idleConnectionRequest = initiateGetRequest(httpClient, port, "/blocking");
blockingServlet.admitOne(); Object result = idleConnectionRequest.get();
Object response = request.get(); return result;
assertThat(response).isInstanceOf(HttpResponse.class); }, (result) -> result instanceof Exception);
Object idleConnectionRequestResult = idleConnectionRequest.get();
assertThat(idleConnectionRequestResult).isInstanceOfAny(SocketException.class, NoHttpResponseException.class); assertThat(idleConnectionRequestResult).isInstanceOfAny(SocketException.class, NoHttpResponseException.class);
if (idleConnectionRequestResult instanceof SocketException) { if (idleConnectionRequestResult instanceof SocketException) {
assertThat((SocketException) idleConnectionRequestResult).hasMessage("Connection reset"); assertThat((SocketException) idleConnectionRequestResult).hasMessage("Connection reset");
} }
blockingServlet.admitOne();
Object response = request.get();
assertThat(response).isInstanceOf(HttpResponse.class);
this.webServer.stop(); this.webServer.stop();
} }

@ -1445,12 +1445,15 @@ public abstract class AbstractServletWebServerFactoryTests {
private final BlockingQueue<CyclicBarrier> barriers = new ArrayBlockingQueue<>(10); private final BlockingQueue<CyclicBarrier> barriers = new ArrayBlockingQueue<>(10);
protected volatile boolean blocking = true;
public BlockingServlet() { public BlockingServlet() {
} }
@Override @Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
if (this.blocking) {
CyclicBarrier barrier = new CyclicBarrier(2); CyclicBarrier barrier = new CyclicBarrier(2);
this.barriers.add(barrier); this.barriers.add(barrier);
try { try {
@ -1463,6 +1466,7 @@ public abstract class AbstractServletWebServerFactoryTests {
throw new ServletException(ex); throw new ServletException(ex);
} }
} }
}
public void admitOne() { public void admitOne() {
try { try {
@ -1491,6 +1495,10 @@ public abstract class AbstractServletWebServerFactoryTests {
} }
} }
public void setBlocking(boolean blocking) {
this.blocking = blocking;
}
} }
static class BlockingAsyncServlet extends HttpServlet { static class BlockingAsyncServlet extends HttpServlet {

Loading…
Cancel
Save