diff options
4 files changed, 33 insertions, 19 deletions
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index e9f61ac181..f03f6dbc52 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -235,6 +235,8 @@ public class HttpChannel<T> implements HttpParser.RequestHandler<T>, Runnable public boolean handle() { LOG.debug("{} handle enter", this); + if(_state.isCompleted()) + return false; setCurrentHttpChannel(this); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index d070aeee67..a117b8e77f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -563,6 +563,14 @@ public class HttpChannelState } } + boolean isCompleted() + { + synchronized (this) + { + return _state == State.COMPLETED; + } + } + public boolean isAsyncStarted() { synchronized (this) diff --git a/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java b/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java index 45923b0c4f..08cd325474 100644 --- a/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java +++ b/jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java @@ -214,7 +214,7 @@ public class HttpTransportOverSPDY implements HttpTransport @Override public void completed() { - LOG.debug("Completed"); + LOG.debug("Completed {}", this); } private void reply(Stream stream, ReplyInfo replyInfo) @@ -263,7 +263,7 @@ public class HttpTransportOverSPDY implements HttpTransport { private final Queue<PushResource> queue = new ConcurrentArrayQueue<>(); private final Set<String> resources; - private boolean active; + private AtomicBoolean active = new AtomicBoolean(false); private PushResourceCoordinator(Set<String> resources) { @@ -272,6 +272,7 @@ public class HttpTransportOverSPDY implements HttpTransport private void coordinate() { + LOG.debug("Pushing resources: {}", resources); // Must send all push frames to the client at once before we // return from this method and send the main resource data for (String pushResource : resources) @@ -281,17 +282,15 @@ public class HttpTransportOverSPDY implements HttpTransport private void sendNextResourceData() { PushResource resource; - synchronized (this) + if(active.compareAndSet(false, true)) { - if (active) - return; resource = queue.poll(); if (resource == null) return; - active = true; + LOG.debug("Opening new push channel for: {}", resource); + HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders()); + pushChannel.requestStart(resource.getPushRequestHeaders(), true); } - HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders()); - pushChannel.requestStart(resource.getPushRequestHeaders(), true); } private HttpChannelOverSPDY newHttpChannelOverSPDY(Stream pushStream, Fields pushRequestHeaders) @@ -329,6 +328,13 @@ public class HttpTransportOverSPDY implements HttpTransport }); } + private void complete() + { + if(!active.compareAndSet(true, false)) + LOG.warn("complete() called and active==false? That smells like a concurrency bug!", new IllegalStateException()); + sendNextResourceData(); + } + private Fields createRequestHeaders(Fields.Field scheme, Fields.Field host, Fields.Field uri, String pushResourcePath) { final Fields newRequestHeaders = new Fields(requestHeaders, false); @@ -358,15 +364,6 @@ public class HttpTransportOverSPDY implements HttpTransport } return pushHeaders; } - - private void complete() - { - synchronized (this) - { - active = false; - } - sendNextResourceData(); - } } private static class PushResource @@ -389,5 +386,14 @@ public class HttpTransportOverSPDY implements HttpTransport { return pushRequestHeaders; } + + @Override + public String toString() + { + return "PushResource{" + + "pushStream=" + pushStream + + ", pushRequestHeaders=" + pushRequestHeaders + + '}'; + } } } diff --git a/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java b/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java index 5b501fef8e..d120166b08 100644 --- a/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java +++ b/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java @@ -60,7 +60,6 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StdErrLog; import org.junit.Assert; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import static org.hamcrest.CoreMatchers.is; @@ -338,7 +337,6 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest } @Test - @Ignore public void testPushResourceAreSentNonInterleaved() throws Exception { final CountDownLatch allExpectedPushesReceivedLatch = new CountDownLatch(4); |