484262 - Race condition between GOAWAY disconnect and ability to make new request.

Fixed by making sure that when a peer received a GOAWAY frame, it
does not also notify the onFailure() callback.
This commit is contained in:
Simone Bordet 2015-12-15 15:35:04 +01:00
parent d4d9ceea86
commit 717fc7819d
2 changed files with 50 additions and 9 deletions

View File

@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http2.ErrorCode;
import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.http2.api.Stream;
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
@ -416,4 +417,46 @@ public class HTTP2Test extends AbstractTest
Assert.assertTrue(exchangeLatch2.await(5, TimeUnit.SECONDS));
Assert.assertEquals(0, session.getStreams().size());
}
@Test
public void testCleanGoAwayDoesNotTriggerFailureNotification() throws Exception
{
start(new ServerSessionListener.Adapter()
{
@Override
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
MetaData.Response metaData = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields());
HeadersFrame response = new HeadersFrame(stream.getId(), metaData, null, true);
stream.headers(response, Callback.NOOP);
// Close cleanly.
stream.getSession().close(ErrorCode.NO_ERROR.code, null, Callback.NOOP);
return null;
}
});
CountDownLatch closeLatch = new CountDownLatch(1);
CountDownLatch failureLatch = new CountDownLatch(1);
Session session = newClient(new Session.Listener.Adapter()
{
@Override
public void onClose(Session session, GoAwayFrame frame)
{
closeLatch.countDown();
}
@Override
public void onFailure(Session session, Throwable failure)
{
failureLatch.countDown();
}
});
MetaData.Request metaData = newRequest("GET", new HttpFields());
HeadersFrame request = new HeadersFrame(metaData, null, true);
session.newStream(request, new Promise.Adapter<>(), new Stream.Listener.Adapter());
// Make sure onClose() is called.
Assert.assertTrue(closeLatch.await(5, TimeUnit.SECONDS));
Assert.assertFalse(failureLatch.await(1, TimeUnit.SECONDS));
}
}

View File

@ -230,7 +230,7 @@ public class HTTP2Flusher extends IteratingCallback
if (actives.isEmpty())
{
if (isClosed())
abort(new ClosedChannelException());
fail(new ClosedChannelException(), true);
if (LOG.isDebugEnabled())
LOG.debug("Flushed {}", session);
@ -289,9 +289,6 @@ public class HTTP2Flusher extends IteratingCallback
@Override
protected void onCompleteFailure(Throwable x)
{
if (LOG.isDebugEnabled())
LOG.debug("Failed", x);
lease.recycle();
// Transfer active items to avoid reentrancy.
@ -306,10 +303,10 @@ public class HTTP2Flusher extends IteratingCallback
entry.failed(x);
}
abort(x);
fail(x, isClosed());
}
private void abort(Throwable x)
private void fail(Throwable x, boolean closed)
{
Queue<Entry> queued;
synchronized (this)
@ -319,12 +316,13 @@ public class HTTP2Flusher extends IteratingCallback
}
if (LOG.isDebugEnabled())
LOG.debug("Aborting, queued={}", queued.size());
LOG.debug("{}, queued={}", closed ? "Closing" : "Failing", queued.size());
for (Entry entry : queued)
closed(entry, x);
entry.failed(x);
session.abort(x);
if (!closed)
session.abort(x);
}
private void closed(Entry entry, Throwable failure)