484167 - GOAWAY frames aren't handling disconnects appropriately on Client.

Fixed by overriding onClose() to listen for GOAWAY frames, and acting
appropriately.
This commit is contained in:
Simone Bordet 2015-12-11 12:25:54 +01:00
parent e7d8980952
commit c3889873f6
3 changed files with 69 additions and 26 deletions

View File

@ -32,6 +32,7 @@ import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.HTTP2ClientConnectionFactory;
import org.eclipse.jetty.http2.frames.GoAwayFrame;
import org.eclipse.jetty.io.ClientConnectionFactory;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
@ -107,37 +108,15 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements
final HttpDestination destination = (HttpDestination)context.get(HTTP_DESTINATION_CONTEXT_KEY);
@SuppressWarnings("unchecked")
final Promise<Connection> connection = (Promise<Connection>)context.get(HTTP_CONNECTION_PROMISE_CONTEXT_KEY);
final Promise<Connection> connectionPromise = (Promise<Connection>)context.get(HTTP_CONNECTION_PROMISE_CONTEXT_KEY);
Session.Listener listener = new Session.Listener.Adapter()
{
@Override
public void onFailure(Session session, Throwable failure)
{
destination.abort(failure);
}
};
final Promise<Session> promise = new Promise<Session>()
{
@Override
public void succeeded(Session session)
{
connection.succeeded(newHttpConnection(destination, session));
}
@Override
public void failed(Throwable failure)
{
connection.failed(failure);
}
};
SessionListenerPromise listenerPromise = new SessionListenerPromise(destination, connectionPromise);
SslContextFactory sslContextFactory = null;
if (HttpScheme.HTTPS.is(destination.getScheme()))
sslContextFactory = httpClient.getSslContextFactory();
client.connect(sslContextFactory, address, listener, promise, context);
client.connect(sslContextFactory, address, listenerPromise, listenerPromise, context);
}
@Override
@ -154,4 +133,42 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements
{
return new HttpConnectionOverHTTP2(destination, session);
}
private class SessionListenerPromise extends Session.Listener.Adapter implements Promise<Session>
{
private final HttpDestination destination;
private final Promise<Connection> promise;
private HttpConnectionOverHTTP2 connection;
public SessionListenerPromise(HttpDestination destination, Promise<Connection> promise)
{
this.destination = destination;
this.promise = promise;
}
@Override
public void succeeded(Session session)
{
connection = newHttpConnection(destination, session);
promise.succeeded(connection);
}
@Override
public void failed(Throwable failure)
{
promise.failed(failure);
}
@Override
public void onClose(Session session, GoAwayFrame frame)
{
connection.close();
}
@Override
public void onFailure(Session session, Throwable failure)
{
connection.abort(failure);
}
}
}

View File

@ -69,7 +69,7 @@ public class HttpConnectionOverHTTP2 extends HttpConnection
abort(new AsynchronousCloseException());
}
private void abort(Throwable failure)
protected void abort(Throwable failure)
{
for (HttpChannel channel : channels)
{

View File

@ -27,6 +27,8 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.Assert;
@ -93,4 +95,28 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
Assert.assertTrue(latch.await(2 * idleTimeout, TimeUnit.MILLISECONDS));
}
@Test
public void testServerIdleTimeout() throws Exception
{
start(new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
baseRequest.setHandled(true);
}
});
connector.setIdleTimeout(idleTimeout);
ContentResponse response1 = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response1.getStatus());
// Let the server idle timeout.
Thread.sleep(2 * idleTimeout);
// Make sure we can make another request successfully.
ContentResponse response2 = client.newRequest(newURI()).send();
Assert.assertEquals(HttpStatus.OK_200, response2.getStatus());
}
}