480061 - HTTP/2 server doesn't send GOAWAY frame when shutting down.

Fixed by overriding Connection.close() and sending the GOAWAY from
there, which is triggered when the SelectorManager stops.

This allowed simplification of client code too.
This commit is contained in:
Simone Bordet 2015-10-19 11:17:22 +02:00
parent 570c751ec8
commit 3c26ffb227
5 changed files with 55 additions and 46 deletions

View File

@ -26,13 +26,9 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.Executor;
import org.eclipse.jetty.alpn.client.ALPNClientConnectionFactory;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.ISession;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.ClientConnectionFactory;
@ -43,7 +39,6 @@ import org.eclipse.jetty.io.MappedByteBufferPool;
import org.eclipse.jetty.io.SelectChannelEndPoint;
import org.eclipse.jetty.io.SelectorManager;
import org.eclipse.jetty.io.ssl.SslClientConnectionFactory;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.ssl.SslContextFactory;
@ -117,7 +112,6 @@ public class HTTP2Client extends ContainerLifeCycle
private Scheduler scheduler;
private ByteBufferPool bufferPool;
private ClientConnectionFactory connectionFactory;
private Queue<ISession> sessions;
private SelectorManager selector;
private int selectors = 1;
private long idleTimeout = 30000;
@ -150,12 +144,6 @@ public class HTTP2Client extends ContainerLifeCycle
});
}
if (sessions == null)
{
sessions = new ConcurrentLinkedQueue<>();
addBean(sessions);
}
if (selector == null)
{
selector = newSelectorManager();
@ -171,13 +159,6 @@ public class HTTP2Client extends ContainerLifeCycle
return new ClientSelectorManager(getExecutor(), getScheduler(), getSelectors());
}
@Override
protected void doStop() throws Exception
{
closeConnections();
super.doStop();
}
public Executor getExecutor()
{
return executor;
@ -329,23 +310,6 @@ public class HTTP2Client extends ContainerLifeCycle
channel.socket().setTcpNoDelay(true);
}
private void closeConnections()
{
for (ISession session : sessions)
session.close(ErrorCode.NO_ERROR.code, null, Callback.NOOP);
sessions.clear();
}
public boolean addSession(ISession session)
{
return sessions.offer(session);
}
public boolean removeSession(ISession session)
{
return sessions.remove(session);
}
private class ClientSelectorManager extends SelectorManager
{
private ClientSelectorManager(Executor executor, Scheduler scheduler, int selectors)

View File

@ -125,17 +125,9 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
super.onOpen();
}
@Override
public void onClose()
{
super.onClose();
client.removeSession(getSession());
}
@Override
public void succeeded()
{
client.addSession(getSession());
promise.succeeded(getSession());
}

View File

@ -37,7 +37,9 @@ import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.http2.api.Stream;
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
import org.eclipse.jetty.http2.frames.DataFrame;
import org.eclipse.jetty.http2.frames.GoAwayFrame;
import org.eclipse.jetty.http2.frames.HeadersFrame;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.Jetty;
@ -239,4 +241,48 @@ public class HTTP2Test extends AbstractTest
Assert.assertTrue(latch.await(5, TimeUnit.SECONDS));
}
@Test
public void testServerSendsGoAwayOnStop() throws Exception
{
start(new ServerSessionListener.Adapter());
CountDownLatch closeLatch = new CountDownLatch(1);
newClient(new Session.Listener.Adapter()
{
@Override
public void onClose(Session session, GoAwayFrame frame)
{
closeLatch.countDown();
}
});
Thread.sleep(1000);
server.stop();
Assert.assertTrue(closeLatch.await(5, TimeUnit.SECONDS));
}
@Test
public void testClientSendsGoAwayOnStop() throws Exception
{
CountDownLatch closeLatch = new CountDownLatch(1);
start(new ServerSessionListener.Adapter()
{
@Override
public void onClose(Session session, GoAwayFrame frame)
{
closeLatch.countDown();
}
});
newClient(new Session.Listener.Adapter());
Thread.sleep(1000);
client.stop();
Assert.assertTrue(closeLatch.await(5, TimeUnit.SECONDS));
}
}

View File

@ -28,6 +28,7 @@ import org.eclipse.jetty.io.AbstractConnection;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.ConcurrentArrayQueue;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@ -129,6 +130,14 @@ public class HTTP2Connection extends AbstractConnection
executionStrategy.execute();
}
@Override
public void close()
{
// We don't call super from here, otherwise we close the
// endPoint and we're not able to read or write anymore.
session.close(ErrorCode.NO_ERROR.code, "close", Callback.NOOP);
}
protected class HTTP2Producer implements ExecutionStrategy.Producer
{
private ByteBuffer buffer;

View File

@ -537,8 +537,6 @@ public abstract class HTTP2Session implements ISession, Parser.Listener
{
byte[] payload = reason == null ? null : reason.getBytes(StandardCharsets.UTF_8);
GoAwayFrame frame = new GoAwayFrame(lastStreamId.get(), error, payload);
if (LOG.isDebugEnabled())
LOG.debug("Sending {}", frame);
control(null, callback, frame);
return true;
}