H3: Fix racy read from stream-less channel (#9761)

* #9655 introduce new Stream.Client.Listener.onNewStream() method to allow setting the channel's stream before sending any data to the network Signed-off-by: Ludovic Orban <[email protected]>
jetty · May 15, 2023 · 7ac49cd · 7ac49cd
1 parent 8eae62d
commit 7ac49cd
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 2 deletions.
diff --git a/...ttp3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java b/...ttp3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3SessionClient.java
@@ -106,6 +106,7 @@ private CompletableFuture<Stream> newRequest(long streamId, HeadersFrame frame,
             return promise;
 
         stream.setListener(listener);
+        stream.onOpen();
 
         stream.writeFrame(frame)
             .whenComplete((r, x) ->

diff --git a/...http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java b/...http3-client/src/main/java/org/eclipse/jetty/http3/client/internal/HTTP3StreamClient.java
@@ -42,6 +42,11 @@ public Stream.Client.Listener getListener()
         return listener;
     }
 
+    public void onOpen()
+    {
+        notifyNewStream();
+    }
+
     public void setListener(Stream.Client.Listener listener)
     {
         this.listener = listener;
@@ -65,6 +70,20 @@ else if (response.getStatus() == HttpStatus.EARLY_HINT_103)
         }
     }
 
+    private void notifyNewStream()
+    {
+        Stream.Client.Listener listener = getListener();
+        try
+        {
+            if (listener != null)
+                listener.onNewStream(this);
+        }
+        catch (Throwable x)
+        {
+            LOG.info("Failure while notifying listener {}", listener, x);
+        }
+    }
+
     private void notifyResponse(HeadersFrame frame)
     {
         Stream.Client.Listener listener = getListener();

diff --git a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java
@@ -138,6 +138,16 @@ public interface Client extends Stream
          */
         public interface Listener
         {
+            /**
+             * <p>Callback method invoked when a stream is created locally by
+             * {@link Session.Client#newRequest(HeadersFrame, Listener)}.</p>
+             *
+             * @param stream the newly created stream
+             */
+            public default void onNewStream(Stream.Client stream)
+            {
+            }
+
             /**
              * <p>Callback method invoked when a response is received.</p>
              * <p>To read response content, applications should call

diff --git a/...ort/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java b/...ort/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpReceiverOverHTTP3.java
@@ -59,9 +59,24 @@ protected void receive()
             return;
 
         if (notifySuccess)
+        {
             responseSuccess(exchange);
+        }
         else
-            getHttpChannel().getStream().demand();
+        {
+            Stream stream = getHttpChannel().getStream();
+            if (LOG.isDebugEnabled())
+                LOG.debug("Demanding from {} in {}", stream, this);
+            if (stream == null)
+                return;
+            stream.demand();
+        }
+    }
+
+    @Override
+    public void onNewStream(Stream.Client stream)
+    {
+        getHttpChannel().setStream(stream);
     }
 
     @Override

diff --git a/...sport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java b/...sport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpSenderOverHTTP3.java
@@ -138,7 +138,6 @@ protected void sendHeaders(HttpExchange exchange, ByteBuffer contentBuffer, bool
 
     private Stream onNewStream(Stream stream, HttpRequest request)
     {
-        getHttpChannel().setStream(stream);
         long idleTimeout = request.getIdleTimeout();
         if (idleTimeout > 0)
             ((HTTP3Stream)stream).setIdleTimeout(idleTimeout);

diff --git a/tests/test-http-client-transport/src/test/resources/jetty-logging.properties b/tests/test-http-client-transport/src/test/resources/jetty-logging.properties
@@ -7,6 +7,7 @@ org.eclipse.jetty.jmx.LEVEL=INFO
 org.eclipse.jetty.http2.hpack.LEVEL=INFO
 #org.eclipse.jetty.http2.client.LEVEL=DEBUG
 #org.eclipse.jetty.http3.LEVEL=DEBUG
+#org.eclipse.jetty.http3.client.LEVEL=DEBUG
 org.eclipse.jetty.http3.qpack.LEVEL=INFO
 #org.eclipse.jetty.quic.LEVEL=DEBUG
 org.eclipse.jetty.quic.quiche.LEVEL=INFO