8361249: PlainHttpConnection connection logic can be simplified

Reviewed-by: djelinski, vyazici, michaelm, jpai

Author: dfuch

src/java.net.http/share/classes/jdk/internal/net/http/PlainHttpConnection.java

@@ -36,12 +36,10 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReentrantLock;
-import java.util.function.Function;
 
 import jdk.internal.net.http.common.FlowTube;
 import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.MinimalFuture;
-import jdk.internal.net.http.common.TimeSource;
 import jdk.internal.net.http.common.Utils;
 
 /**
@@ -57,16 +55,9 @@ class PlainHttpConnection extends HttpConnection {
     private volatile boolean connected;
     private volatile boolean closed;
     private volatile ConnectTimerEvent connectTimerEvent;  // may be null
-    private volatile int unsuccessfulAttempts;
     private final ReentrantLock stateLock = new ReentrantLock();
     private final AtomicReference<Throwable> errorRef = new AtomicReference<>();
 
-    // Indicates whether a connection attempt has succeeded or should be retried.
-    // If the attempt failed, and shouldn't be retried, there will be an exception
-    // instead.
-    private enum ConnectState { SUCCESS, RETRY }
-
-
     /**
      * Returns a ConnectTimerEvent iff there is a connect timeout duration,
      * otherwise null.
@@ -115,10 +106,10 @@ Throwable getError(Throwable cause) {
     }
 
     final class ConnectEvent extends AsyncEvent {
-        private final CompletableFuture<ConnectState> cf;
+        private final CompletableFuture<Void> cf;
         private final Exchange<?> exchange;
 
-        ConnectEvent(CompletableFuture<ConnectState> cf, Exchange<?> exchange) {
+        ConnectEvent(CompletableFuture<Void> cf, Exchange<?> exchange) {
             this.cf = cf;
             this.exchange = exchange;
         }
@@ -147,15 +138,10 @@ public void handle() {
                 assert finished || exchange.multi.requestCancelled() : "Expected channel to be connected";
                 if (connectionOpened()) {
                     // complete async since the event runs on the SelectorManager thread
-                    cf.completeAsync(() -> ConnectState.SUCCESS, client().theExecutor());
+                    if (debug.on()) debug.log("%s has been connected asynchronously", label());
+                    cf.completeAsync(() -> null, client().theExecutor());
                 } else throw new ConnectException("Connection closed");
             } catch (Throwable e) {
-                if (canRetryConnect(e)) {
-                    unsuccessfulAttempts++;
-                    // complete async since the event runs on the SelectorManager thread
-                    cf.completeAsync(() -> ConnectState.RETRY, client().theExecutor());
-                    return;
-                }
                 Throwable t = getError(Utils.toConnectException(e));
                 // complete async since the event runs on the SelectorManager thread
                 client().theExecutor().execute( () -> cf.completeExceptionally(t));
@@ -174,7 +160,7 @@ public void abort(IOException ioe) {
 
     @Override
     public CompletableFuture<Void> connectAsync(Exchange<?> exchange) {
-        CompletableFuture<ConnectState> cf = new MinimalFuture<>();
+        CompletableFuture<Void> cf = new MinimalFuture<>();
         try {
             assert !connected : "Already connected";
             assert !chan.isBlocking() : "Unexpected blocking channel";
@@ -212,7 +198,8 @@ public CompletableFuture<Void> connectAsync(Exchange<?> exchange) {
             if (finished) {
                 if (debug.on()) debug.log("connect finished without blocking");
                 if (connectionOpened()) {
-                    cf.complete(ConnectState.SUCCESS);
+                    if (debug.on()) debug.log("%s has been connected", label());
+                    cf.complete(null);
                 } else throw getError(new ConnectException("connection closed"));
             } else {
                 if (debug.on()) debug.log("registering connect event");
@@ -232,8 +219,7 @@ public CompletableFuture<Void> connectAsync(Exchange<?> exchange) {
                     debug.log("Failed to close channel after unsuccessful connect");
             }
         }
-        return cf.handle((r,t) -> checkRetryConnect(r, t, exchange))
-                .thenCompose(Function.identity());
+        return cf;
     }
 
     boolean connectionOpened() {
@@ -254,42 +240,6 @@ boolean connectionOpened() {
         return !closed;
     }
 
-    /**
-     * On some platforms, a ConnectEvent may be raised and a ConnectionException
-     * may occur with the message "Connection timed out: no further information"
-     * before our actual connection timeout has expired. In this case, this
-     * method will be called with a {@code connect} state of {@code ConnectState.RETRY)}
-     * and we will retry once again.
-     * @param connect indicates whether the connection was successful or should be retried
-     * @param failed the failure if the connection failed
-     * @param exchange the exchange
-     * @return a completable future that will take care of retrying the connection if needed.
-     */
-    private CompletableFuture<Void> checkRetryConnect(ConnectState connect, Throwable failed, Exchange<?> exchange) {
-        // first check if the connection failed
-        if (failed != null) return MinimalFuture.failedFuture(failed);
-        // then check if the connection should be retried
-        if (connect == ConnectState.RETRY) {
-            int attempts = unsuccessfulAttempts;
-            assert attempts <= 1;
-            if (debug.on())
-                debug.log("Retrying connect after %d attempts", attempts);
-            return connectAsync(exchange);
-        }
-        // Otherwise, the connection was successful;
-        assert connect == ConnectState.SUCCESS;
-        return MinimalFuture.completedFuture(null);
-    }
-
-    private boolean canRetryConnect(Throwable e) {
-        if (!MultiExchange.RETRY_CONNECT) return false;
-        if (!(e instanceof ConnectException)) return false;
-        if (unsuccessfulAttempts > 0) return false;
-        ConnectTimerEvent timer = connectTimerEvent;
-        if (timer == null) return true;
-        return timer.deadline().isAfter(TimeSource.now());
-    }
-
     @Override
     public CompletableFuture<Void> finishConnect() {
         assert connected == false;