Revert "412318 HttpChannel fix multiple calls to _transport.completed() if handle() is called multiple times while the channel is COMPLETED"

This reverts commit cb2eb030d18b6c92f3249794c02b100f0ee5cb33.

4 files changed, 19 insertions, 33 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 f03f6dbc52..e9f61ac181 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,8 +235,6 @@ 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 a117b8e77f..d070aeee67 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,14 +563,6 @@ 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 08cd325474..45923b0c4f 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 {}", this);
+        LOG.debug("Completed");
     }
 
     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 AtomicBoolean active = new AtomicBoolean(false);
+        private boolean active;
 
         private PushResourceCoordinator(Set<String> resources)
         {
@@ -272,7 +272,6 @@ 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)
@@ -282,15 +281,17 @@ public class HttpTransportOverSPDY implements HttpTransport
         private void sendNextResourceData()
         {
             PushResource resource;
-            if(active.compareAndSet(false, true))
+            synchronized (this)
             {
+                if (active)
+                    return;
                 resource = queue.poll();
                 if (resource == null)
                     return;
-                LOG.debug("Opening new push channel for: {}", resource);
-                HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders());
-                pushChannel.requestStart(resource.getPushRequestHeaders(), true);
+                active = true;
             }
+            HttpChannelOverSPDY pushChannel = newHttpChannelOverSPDY(resource.getPushStream(), resource.getPushRequestHeaders());
+            pushChannel.requestStart(resource.getPushRequestHeaders(), true);
         }
 
         private HttpChannelOverSPDY newHttpChannelOverSPDY(Stream pushStream, Fields pushRequestHeaders)
@@ -328,13 +329,6 @@ 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);
@@ -364,6 +358,15 @@ public class HttpTransportOverSPDY implements HttpTransport
             }
             return pushHeaders;
         }
+
+        private void complete()
+        {
+            synchronized (this)
+            {
+                active = false;
+            }
+            sendNextResourceData();
+        }
     }
 
     private static class PushResource
@@ -386,14 +389,5 @@ 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 d120166b08..5b501fef8e 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,6 +60,7 @@ 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;
@@ -337,6 +338,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest
     }
 
     @Test
+    @Ignore
     public void testPushResourceAreSentNonInterleaved() throws Exception
     {
         final CountDownLatch allExpectedPushesReceivedLatch = new CountDownLatch(4);