Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixes #504 - HTTP/2 client transport cannot send request after idle timeout. 

Made sure that the idle timeout mechanism notifies the destination
that the connection will close.

Also reviewed the close protocol to be: notify destination, then abort,
then close. In this way, HTTP/2 can send RST_STREAM before the
connection is closed.
This commit is contained in:
Simone Bordet 2016-04-11 19:14:34 +02:00
parent 11242ae1ec
commit f0a1ccf4d3
5 changed files with 157 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java
View File

@ -138,17 +138,16 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec
{
if (closed.compareAndSet(false, true))
{
// First close then abort, to be sure that the connection cannot be reused
// from an onFailure() handler or by blocking code waiting for completion.
getHttpDestination().close(this);
abort(failure);
getEndPoint().shutdownOutput();
if (LOG.isDebugEnabled())
LOG.debug("Shutdown {}", this);
getEndPoint().close();
if (LOG.isDebugEnabled())
LOG.debug("Closed {}", this);
abort(failure);
}
}

7
jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java
View File

@ -223,17 +223,16 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec
{
if (closed.compareAndSet(false, true))
{
// First close then abort, to be sure that the connection cannot be reused
// from an onFailure() handler or by blocking code waiting for completion.
getHttpDestination().close(this);
abort(failure);
getEndPoint().shutdownOutput();
if (LOG.isDebugEnabled())
LOG.debug("Shutdown {}", this);
getEndPoint().close();
if (LOG.isDebugEnabled())
LOG.debug("Closed {}", this);
abort(failure);
}
}

25
jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java
View File

@ -20,6 +20,8 @@ package org.eclipse.jetty.http2.client.http;
import java.nio.channels.AsynchronousCloseException;
import java.util.Set;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.jetty.client.HttpChannel;
import org.eclipse.jetty.client.HttpConnection;
@ -35,6 +37,7 @@ import org.eclipse.jetty.util.ConcurrentHashSet;
public class HttpConnectionOverHTTP2 extends HttpConnection
{
private final Set<HttpChannel> channels = new ConcurrentHashSet<>();
private final AtomicBoolean closed = new AtomicBoolean();
private final Session session;
public HttpConnectionOverHTTP2(HttpDestination destination, Session session)
@ -72,6 +75,15 @@ public class HttpConnectionOverHTTP2 extends HttpConnection
getHttpDestination().release(this);
}
@Override
public boolean onIdleTimeout(long idleTimeout)
{
boolean close = super.onIdleTimeout(idleTimeout);
if (close)
close(new TimeoutException("idle_timeout"));
return false;
}
@Override
public void close()
{
@ -80,11 +92,14 @@ public class HttpConnectionOverHTTP2 extends HttpConnection
protected void close(Throwable failure)
{
// First close then abort, to be sure that the connection cannot be reused
// from an onFailure() handler or by blocking code waiting for completion.
getHttpDestination().close(this);
session.close(ErrorCode.NO_ERROR.code, failure.getMessage(), Callback.NOOP);
abort(failure);
if (closed.compareAndSet(false, true))
{
getHttpDestination().close(this);
abort(failure);
session.close(ErrorCode.NO_ERROR.code, failure.getMessage(), Callback.NOOP);
}
}
private void abort(Throwable failure)

77
jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2Test.java
View File

@ -313,6 +313,83 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
}
@Test
public void testConnectionIdleTimeoutSendsResetFrame() throws Exception
{
long idleTimeout = 1000;
CountDownLatch resetLatch = new CountDownLatch(1);
start(new ServerSessionListener.Adapter()
{
@Override
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
return new Stream.Listener.Adapter()
{
@Override
public void onReset(Stream stream, ResetFrame frame)
{
resetLatch.countDown();
}
};
}
});
client.stop();
client.setIdleTimeout(idleTimeout);
client.start();
try
{
client.newRequest("localhost", connector.getLocalPort())
// Make sure the connection idle times out, not the stream.
.idleTimeout(2 * idleTimeout, TimeUnit.MILLISECONDS)
.send();
Assert.fail();
}
catch (ExecutionException e)
{
// Expected.
}
Assert.assertTrue(resetLatch.await(5, TimeUnit.SECONDS));
}
@Test
public void testRequestIdleTimeoutSendsResetFrame() throws Exception
{
CountDownLatch resetLatch = new CountDownLatch(1);
start(new ServerSessionListener.Adapter()
{
@Override
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
return new Stream.Listener.Adapter()
{
@Override
public void onReset(Stream stream, ResetFrame frame)
{
resetLatch.countDown();
}
};
}
});
try
{
long idleTimeout = 1000;
client.newRequest("localhost", connector.getLocalPort())
.idleTimeout(idleTimeout, TimeUnit.MILLISECONDS)
.send();
Assert.fail();
}
catch (ExecutionException e)
{
// Expected.
}
Assert.assertTrue(resetLatch.await(5, TimeUnit.SECONDS));
}
@Ignore
@Test
public void testExternalServer() throws Exception

64
tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientIdleTimeoutTest.java
View File

@ -52,8 +52,11 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
AsyncContext asyncContext = request.startAsync();
asyncContext.setTimeout(0);
if (target.equals("/timeout"))
{
AsyncContext asyncContext = request.startAsync();
asyncContext.setTimeout(0);
}
}
});
client.stop();
@ -61,13 +64,19 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
client.start();
final CountDownLatch latch = new CountDownLatch(1);
client.newRequest(newURI()).send(result ->
{
if (result.isFailed())
latch.countDown();
});
client.newRequest(newURI())
.path("/timeout")
.send(result ->
{
if (result.isFailed())
latch.countDown();
});
Assert.assertTrue(latch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
// Verify that after the timeout we can make another request.
ContentResponse response = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
}
@Test
@ -79,13 +88,17 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
AsyncContext asyncContext = request.startAsync();
asyncContext.setTimeout(0);
if (target.equals("/timeout"))
{
AsyncContext asyncContext = request.startAsync();
asyncContext.setTimeout(0);
}
}
});
final CountDownLatch latch = new CountDownLatch(1);
client.newRequest(newURI())
.path("/timeout")
.idleTimeout(idleTimeout, TimeUnit.MILLISECONDS)
.send(result ->
{
@ -94,10 +107,41 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
});
Assert.assertTrue(latch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
// Verify that after the timeout we can make another request.
ContentResponse response = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
}
@Test
public void testServerIdleTimeout() throws Exception
public void testIdleClientIdleTimeout() throws Exception
{
start(new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
}
});
client.stop();
client.setIdleTimeout(idleTimeout);
client.start();
// Make a first request to open a connection.
ContentResponse response = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
// Let the connection idle timeout.
Thread.sleep(2 * idleTimeout);
// Verify that after the timeout we can make another request.
response = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response.getStatus());
}
@Test
public void testIdleServerIdleTimeout() throws Exception
{
start(new AbstractHandler()
{