aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Becker2013-07-04 09:45:56 (EDT)
committerThomas Becker2013-07-04 09:46:14 (EDT)
commitcb2eb030d18b6c92f3249794c02b100f0ee5cb33 (patch)
tree3b6f1be747cd40fc335b82af77a19e7e40cf7bb6
parentbce8eaabe02823dffc8243ea0eb9b14683eaf3ab (diff)
downloadorg.eclipse.jetty.project-cb2eb030d18b6c92f3249794c02b100f0ee5cb33.zip
org.eclipse.jetty.project-cb2eb030d18b6c92f3249794c02b100f0ee5cb33.tar.gz
org.eclipse.jetty.project-cb2eb030d18b6c92f3249794c02b100f0ee5cb33.tar.bz2
412318 HttpChannel fix multiple calls to _transport.completed() if handle() is called multiple times while the channel is COMPLETED
-rw-r--r--jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java2
-rw-r--r--jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java8
-rw-r--r--jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HttpTransportOverSPDY.java40
-rw-r--r--jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java2
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 e9f61ac..f03f6db 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 d070aee..a117b8e 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 45923b0..08cd325 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 5b501fe..d120166 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);