From dcc8bfcd10d19176ee9d06dbd6cea1c1383e7e74 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 2 Aug 2014 00:01:56 +0200 Subject: [PATCH] Fixed a reentrancy issue that caused a stack overflow. The case was that shutdown was called, ShutdownFlusherEntry called flusher.close(), which called super.close(), which called onCompleteFailure(), which looped over the active items to fail them, calling again ShutdownFlusherEntry, which called again flusher.close(), etc. --- .../org/eclipse/jetty/http2/HTTP2Session.java | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 0b98d5c2b7d..dfdc4adc20f 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -588,11 +588,13 @@ public abstract class HTTP2Session implements ISession, Parser.Listener private final ByteBufferPool.Lease lease = new ByteBufferPool.Lease(generator.getByteBufferPool()); private final int maxGather; private final List active; + private final Queue complete; private Flusher(int maxGather) { this.maxGather = maxGather; this.active = new ArrayList<>(maxGather); + this.complete = new ArrayDeque<>(maxGather); } private void append(FlusherEntry entry) @@ -731,12 +733,19 @@ public abstract class HTTP2Session implements ISession, Parser.Listener public void succeeded() { lease.recycle(); + + // Transfer active items to avoid reentrancy. for (int i = 0; i < active.size(); ++i) + complete.add(active.get(i)); + active.clear(); + + // Drain the queue one by one to avoid reentrancy. + while (!complete.isEmpty()) { - FlusherEntry entry = active.get(i); + FlusherEntry entry = complete.poll(); entry.succeeded(); } - active.clear(); + super.succeeded(); } @@ -749,31 +758,40 @@ public abstract class HTTP2Session implements ISession, Parser.Listener @Override protected void onCompleteFailure(Throwable x) { - LOG.debug(x); + if (LOG.isDebugEnabled()) + LOG.debug(x); + lease.recycle(); + + // Transfer active items to avoid reentrancy. for (int i = 0; i < active.size(); ++i) + complete.add(active.get(i)); + active.clear(); + + // Drain the queue one by one to avoid reentrancy. + while (!complete.isEmpty()) { - FlusherEntry entry = active.get(i); + FlusherEntry entry = complete.poll(); entry.failed(x); } - active.clear(); } public void close() { + super.close(); + Queue queued; synchronized (queue) { - super.close(); queued = new ArrayDeque<>(queue); + queue.clear(); } if (LOG.isDebugEnabled()) LOG.debug("Closing, queued={}", queued.size()); - for (FlusherEntry item: queued) + for (FlusherEntry item : queued) closed(item); - } protected void closed(FlusherEntry item)