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

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);
 

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)

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 +
+                    '}';
+        }
     }
 }

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);