You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2021/12/18 11:00:54 UTC

[httpcomponents-core] branch HTTPCORE-705 updated (95b156d -> e67e5b6)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch HTTPCORE-705
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git.


 discard 95b156d  HTTPCORE-705: ConnectionReuseStrategy to use protocol version of the response message by default and that of the execution context as a fallback
 discard 5d58f9f  Use subclass of ConnectionClosedException to signal request execution failures due to the connection being closed. Requests failed with this exception should generally be safe to re-execute
     new 5b06568  Use subclass of ConnectionClosedException to signal request execution failures due to the connection being closed. Requests failed with this exception should generally be safe to re-execute
     new e67e5b6  HTTPCORE-705: ConnectionReuseStrategy to use protocol version of the response message by default and that of the execution context as a fallback

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (95b156d)
            \
             N -- N -- N   refs/heads/HTTPCORE-705 (e67e5b6)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

[httpcomponents-core] 01/02: Use subclass of ConnectionClosedException to signal request execution failures due to the connection being closed. Requests failed with this exception should generally be safe to re-execute

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPCORE-705
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit 5b0656880c14b4584313ad1173256e515a8c772a
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Dec 17 12:11:15 2021 +0100

    Use subclass of ConnectionClosedException to signal request execution failures due to the connection being closed. Requests failed with this exception should generally be safe to re-execute
---
 .../impl/nio/AbstractH2StreamMultiplexer.java      |  6 +--
 .../hc/core5/http/RequestNotExecutedException.java | 56 ++++++++++++++++++++++
 .../hc/core5/http/nio/command/CommandSupport.java  | 27 ++++-------
 .../http/nio/command/RequestExecutionCommand.java  | 26 +++++++---
 4 files changed, 87 insertions(+), 28 deletions(-)

diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
index dc73741..2222f52 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
@@ -36,7 +36,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
-import java.util.concurrent.CancellationException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -55,6 +54,7 @@ import org.apache.hc.core5.http.HttpStreamResetException;
 import org.apache.hc.core5.http.HttpVersion;
 import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.ProtocolVersion;
+import org.apache.hc.core5.http.RequestNotExecutedException;
 import org.apache.hc.core5.http.config.CharCodingConfig;
 import org.apache.hc.core5.http.impl.BasicEndpointDetails;
 import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics;
@@ -1688,12 +1688,12 @@ abstract class AbstractH2StreamMultiplexer implements Identifiable, HttpConnecti
         }
 
         void cancel() {
-            reset(new CancellationException("HTTP/2 message exchange cancelled"));
+            reset(new RequestNotExecutedException());
         }
 
         boolean abort() {
             final boolean cancelled = channel.cancel();
-            handler.failed(new CancellationException("HTTP/2 message exchange cancelled"));
+            handler.failed(new RequestNotExecutedException());
             return cancelled;
         }
 
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/RequestNotExecutedException.java b/httpcore5/src/main/java/org/apache/hc/core5/http/RequestNotExecutedException.java
new file mode 100644
index 0000000..3f7e70a
--- /dev/null
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/RequestNotExecutedException.java
@@ -0,0 +1,56 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.core5.http;
+
+import org.apache.hc.core5.annotation.Internal;
+
+/**
+ * {@link ConnectionClosedException} subclass that signals requests cannot not be executed
+ * due to the connection being closed. Generally requests failed with this exception
+ * are safe to be re-executed.
+ */
+@Internal
+public class RequestNotExecutedException extends ConnectionClosedException {
+
+    /**
+     * Constructs a new RequestNotExecutedException with a default message.
+     */
+    public RequestNotExecutedException() {
+        super("Connection is closed");
+    }
+
+    /**
+     * Constructs a new RequestNotExecutedException with the specified detail message.
+     *
+     * @param message The exception detail message
+     */
+    public RequestNotExecutedException(final String message) {
+        super(message);
+    }
+
+}
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/CommandSupport.java b/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/CommandSupport.java
index c2facea..dd19edc 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/CommandSupport.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/CommandSupport.java
@@ -28,8 +28,7 @@
 package org.apache.hc.core5.http.nio.command;
 
 import org.apache.hc.core5.annotation.Internal;
-import org.apache.hc.core5.http.ConnectionClosedException;
-import org.apache.hc.core5.http.nio.AsyncClientExchangeHandler;
+import org.apache.hc.core5.http.RequestNotExecutedException;
 import org.apache.hc.core5.reactor.Command;
 import org.apache.hc.core5.reactor.IOSession;
 import org.apache.hc.core5.util.Args;
@@ -49,13 +48,8 @@ public final class CommandSupport {
         Args.notNull(ioSession, "I/O session");
         Command command;
         while ((command = ioSession.poll()) != null) {
-            if (command instanceof RequestExecutionCommand) {
-                final AsyncClientExchangeHandler exchangeHandler = ((RequestExecutionCommand) command).getExchangeHandler();
-                try {
-                    exchangeHandler.failed(ex);
-                } finally {
-                    exchangeHandler.releaseResources();
-                }
+            if (command instanceof ExecutableCommand) {
+                ((ExecutableCommand) command).failed(ex);
             } else {
                 command.cancel();
             }
@@ -69,16 +63,11 @@ public final class CommandSupport {
         Args.notNull(ioSession, "I/O session");
         Command command;
         while ((command = ioSession.poll()) != null) {
-            if (command instanceof RequestExecutionCommand) {
-                final AsyncClientExchangeHandler exchangeHandler = ((RequestExecutionCommand) command).getExchangeHandler();
-                try {
-                    if (!ioSession.isOpen()) {
-                        exchangeHandler.failed(new ConnectionClosedException());
-                    } else {
-                        exchangeHandler.cancel();
-                    }
-                } finally {
-                    exchangeHandler.releaseResources();
+            if (command instanceof ExecutableCommand) {
+                if (!ioSession.isOpen()) {
+                    ((ExecutableCommand) command).failed(new RequestNotExecutedException());
+                } else {
+                    command.cancel();
                 }
             } else {
                 command.cancel();
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/RequestExecutionCommand.java b/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/RequestExecutionCommand.java
index 05f0961..6da8e30 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/RequestExecutionCommand.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/nio/command/RequestExecutionCommand.java
@@ -27,8 +27,11 @@
 
 package org.apache.hc.core5.http.nio.command;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import org.apache.hc.core5.annotation.Internal;
 import org.apache.hc.core5.concurrent.CancellableDependency;
+import org.apache.hc.core5.http.RequestNotExecutedException;
 import org.apache.hc.core5.http.nio.AsyncClientExchangeHandler;
 import org.apache.hc.core5.http.nio.AsyncPushConsumer;
 import org.apache.hc.core5.http.nio.HandlerFactory;
@@ -47,6 +50,7 @@ public final class RequestExecutionCommand extends ExecutableCommand {
     private final HandlerFactory<AsyncPushConsumer> pushHandlerFactory;
     private final CancellableDependency cancellableDependency;
     private final HttpContext context;
+    private final AtomicBoolean failed;
 
     public RequestExecutionCommand(
             final AsyncClientExchangeHandler exchangeHandler,
@@ -57,6 +61,7 @@ public final class RequestExecutionCommand extends ExecutableCommand {
         this.pushHandlerFactory = pushHandlerFactory;
         this.cancellableDependency = cancellableDependency;
         this.context = context;
+        this.failed = new AtomicBoolean();
     }
 
     public RequestExecutionCommand(
@@ -91,17 +96,26 @@ public final class RequestExecutionCommand extends ExecutableCommand {
 
     @Override
     public void failed(final Exception ex) {
-        try {
-            exchangeHandler.failed(ex);
-        } finally {
-            exchangeHandler.releaseResources();
+        if (failed.compareAndSet(false, true)) {
+            try {
+                exchangeHandler.failed(ex);
+            } finally {
+                exchangeHandler.releaseResources();
+            }
         }
     }
 
     @Override
     public boolean cancel() {
-        exchangeHandler.cancel();
-        return true;
+        if (failed.compareAndSet(false, true)) {
+            try {
+                exchangeHandler.failed(new RequestNotExecutedException());
+                return true;
+            } finally {
+                exchangeHandler.releaseResources();
+            }
+        }
+        return false;
     }
 
 }

[httpcomponents-core] 02/02: HTTPCORE-705: ConnectionReuseStrategy to use protocol version of the response message by default and that of the execution context as a fallback

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPCORE-705
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit e67e5b6903e1005cecdf804c92da2ebce87741c3
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Dec 17 12:24:06 2021 +0100

    HTTPCORE-705: ConnectionReuseStrategy to use protocol version of the response message by default and that of the execution context as a fallback
---
 .../hc/core5/http/impl/DefaultConnectionReuseStrategy.java     |  2 +-
 .../hc/core5/http/impl/TestDefaultConnectionReuseStrategy.java | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/DefaultConnectionReuseStrategy.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/DefaultConnectionReuseStrategy.java
index 0231bcb..10ca317 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/DefaultConnectionReuseStrategy.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/DefaultConnectionReuseStrategy.java
@@ -133,7 +133,7 @@ public class DefaultConnectionReuseStrategy implements ConnectionReuseStrategy {
             headerIterator = response.headerIterator("Proxy-Connection");
         }
 
-        final ProtocolVersion ver = context.getProtocolVersion();
+        final ProtocolVersion ver = response.getVersion() != null ? response.getVersion() : context.getProtocolVersion();
         if (headerIterator.hasNext()) {
             if (ver.greaterEquals(HttpVersion.HTTP_1_1)) {
                 final Iterator<String> it = new BasicTokenIterator(headerIterator);
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/TestDefaultConnectionReuseStrategy.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/TestDefaultConnectionReuseStrategy.java
index 9252f7b..a197d46 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/TestDefaultConnectionReuseStrategy.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/TestDefaultConnectionReuseStrategy.java
@@ -100,8 +100,8 @@ public class TestDefaultConnectionReuseStrategy {
     @Test
     public void testExplicitKeepAlive() throws Exception {
         // Use HTTP 1.0
-        context.setProtocolVersion(HttpVersion.HTTP_1_0);
         final HttpResponse response = new BasicHttpResponse(200, "OK");
+        response.setVersion(HttpVersion.HTTP_1_0);
         response.addHeader("Content-Length", "10");
         response.addHeader("Connection", "keep-alive");
 
@@ -110,8 +110,8 @@ public class TestDefaultConnectionReuseStrategy {
 
     @Test
     public void testHTTP10Default() throws Exception {
-        context.setProtocolVersion(HttpVersion.HTTP_1_0);
         final HttpResponse response = new BasicHttpResponse(200, "OK");
+        response.setVersion(HttpVersion.HTTP_1_0);
         response.addHeader("Content-Length", "10");
         Assert.assertFalse(reuseStrategy.keepAlive(null, response, context));
     }
@@ -126,8 +126,8 @@ public class TestDefaultConnectionReuseStrategy {
     @Test
     public void testBrokenConnectionDirective1() throws Exception {
         // Use HTTP 1.0
-        context.setProtocolVersion(HttpVersion.HTTP_1_0);
         final HttpResponse response = new BasicHttpResponse(200, "OK");
+        response.setVersion(HttpVersion.HTTP_1_0);
         response.addHeader("Content-Length", "10");
         response.addHeader("Connection", "keep--alive");
         Assert.assertFalse(reuseStrategy.keepAlive(null, response, context));
@@ -136,8 +136,8 @@ public class TestDefaultConnectionReuseStrategy {
     @Test
     public void testBrokenConnectionDirective2() throws Exception {
         // Use HTTP 1.0
-        context.setProtocolVersion(HttpVersion.HTTP_1_0);
         final HttpResponse response = new BasicHttpResponse(200, "OK");
+        response.setVersion(HttpVersion.HTTP_1_0);
         response.addHeader("Content-Length", "10");
         response.addHeader("Connection", null);
         Assert.assertFalse(reuseStrategy.keepAlive(null, response, context));
@@ -225,8 +225,8 @@ public class TestDefaultConnectionReuseStrategy {
     @Test
     public void testNoContentResponseHttp10() throws Exception {
         // Use HTTP 1.0
-        context.setProtocolVersion(HttpVersion.HTTP_1_0);
         final HttpResponse response = new BasicHttpResponse(HttpStatus.SC_NO_CONTENT, "No Content");
+        response.setVersion(HttpVersion.HTTP_1_0);
         Assert.assertFalse(reuseStrategy.keepAlive(null, response, context));
     }