Bound an exchange by one request timeout

client/src/main/java/org/asynchttpclient/AsyncHttpClientConfig.java

@@ -109,6 +109,10 @@ public interface AsyncHttpClientConfig {
 
     /**
      * Return the maximum time an {@link AsyncHttpClient} waits until the response is completed.
+     * <p>
+     * By default this bounds each attempt within an exchange rather than the exchange as a whole: a redirect, a
+     * retry and an auth replay each start it again, so a chain of n hops may run for n times this value. Set
+     * {@link #isUseAbsoluteRequestDeadline()} to bound the exchange instead.
      *
      * @return the maximum time an {@link AsyncHttpClient} waits until the response is completed.
      */
@@ -142,6 +146,30 @@ default boolean isUseEventLoopTimeouts() {
         return false;
     }
 
+    /**
+     * Whether {@link #getRequestTimeout()} is a deadline for the whole exchange rather than for each attempt
+     * within it.
+     * <p>
+     * A redirect, a retry and an auth replay all continue the same exchange on the same response future, but
+     * each builds its own timeout state. Anchoring the deadline on that state gives every hop a fresh budget,
+     * which is why a five-redirect chain can legitimately take six times the configured timeout today. Enabling
+     * this anchors it on when the exchange was submitted instead, so a later hop gets whatever is left and the
+     * caller's total wait is bounded by the one value.
+     * <p>
+     * Off by default because turning it on shortens exchanges that rely on the per-attempt behaviour. A caller
+     * working to an end-to-end budget wants it on; {@link Request#getUseAbsoluteRequestDeadline()} sets it for a
+     * single request.
+     * <p>
+     * As with every option on this interface, the {@code org.asynchttpclient.useAbsoluteRequestDeadline}
+     * property is read by {@link DefaultAsyncHttpClientConfig.Builder}, not here: an implementation of this
+     * interface that does not override this method gets {@code false} whatever the property says.
+     *
+     * @return {@code true} to treat the request timeout as a deadline for the whole exchange
+     */
+    default boolean isUseAbsoluteRequestDeadline() {
+        return false;
+    }
+
     /**
      * Is HTTP redirect enabled
      *

client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClientConfig.java

@@ -63,6 +63,7 @@
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultEnabledProtocols;
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultExpiredCookieEvictionDelay;
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFailedIpCooldownEnabled;
+import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultUseAbsoluteRequestDeadline;
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFailedIpCooldownPeriod;
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFilterInsecureCipherSuites;
 import static org.asynchttpclient.config.AsyncHttpClientConfigDefaults.defaultFollowRedirect;
@@ -139,6 +140,7 @@ public class DefaultAsyncHttpClientConfig implements AsyncHttpClientConfig {
     private final int maxRequestRetry;
     private final LoadBalance loadBalance;
     private final boolean failedIpCooldownEnabled;
+    private final boolean useAbsoluteRequestDeadline;
     private final Duration failedIpCooldownPeriod;
     private final boolean disableUrlEncodingForBoundRequests;
     private final boolean useLaxCookieEncoder;
@@ -245,6 +247,7 @@ private DefaultAsyncHttpClientConfig(// http
                                          int maxRequestRetry,
                                          LoadBalance loadBalance,
                                          boolean failedIpCooldownEnabled,
+                                         boolean useAbsoluteRequestDeadline,
                                          Duration failedIpCooldownPeriod,
                                          boolean disableUrlEncodingForBoundRequests,
                                          boolean useLaxCookieEncoder,
@@ -351,6 +354,7 @@ private DefaultAsyncHttpClientConfig(// http
         this.maxRequestRetry = maxRequestRetry;
         this.loadBalance = loadBalance;
         this.failedIpCooldownEnabled = failedIpCooldownEnabled;
+        this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
         this.failedIpCooldownPeriod = failedIpCooldownPeriod;
         this.disableUrlEncodingForBoundRequests = disableUrlEncodingForBoundRequests;
         this.useLaxCookieEncoder = useLaxCookieEncoder;
@@ -522,6 +526,11 @@ public boolean isFailedIpCooldownEnabled() {
         return failedIpCooldownEnabled;
     }
 
+    @Override
+    public boolean isUseAbsoluteRequestDeadline() {
+        return useAbsoluteRequestDeadline;
+    }
+
     @Override
     public Duration getFailedIpCooldownPeriod() {
         return failedIpCooldownPeriod;
@@ -946,6 +955,7 @@ public static class Builder {
         private int maxRequestRetry = defaultMaxRequestRetry();
         private LoadBalance loadBalance = defaultLoadBalance();
         private boolean failedIpCooldownEnabled = defaultFailedIpCooldownEnabled();
+        private boolean useAbsoluteRequestDeadline = defaultUseAbsoluteRequestDeadline();
         private Duration failedIpCooldownPeriod = defaultFailedIpCooldownPeriod();
         private boolean disableUrlEncodingForBoundRequests = defaultDisableUrlEncodingForBoundRequests();
         private boolean useLaxCookieEncoder = defaultUseLaxCookieEncoder();
@@ -1055,6 +1065,7 @@ public Builder(AsyncHttpClientConfig config) {
             maxRequestRetry = config.getMaxRequestRetry();
             loadBalance = config.getLoadBalance();
             failedIpCooldownEnabled = config.isFailedIpCooldownEnabled();
+            useAbsoluteRequestDeadline = config.isUseAbsoluteRequestDeadline();
             failedIpCooldownPeriod = config.getFailedIpCooldownPeriod();
             disableUrlEncodingForBoundRequests = config.isDisableUrlEncodingForBoundRequests();
             useLaxCookieEncoder = config.isUseLaxCookieEncoder();
@@ -1255,6 +1266,17 @@ public Builder setFailedIpCooldownEnabled(boolean failedIpCooldownEnabled) {
             return this;
         }
 
+        /**
+         * @param useAbsoluteRequestDeadline whether the request timeout is a deadline for the whole exchange
+         *                                   rather than for each attempt within it; see
+         *                                   {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()}
+         * @return this
+         */
+        public Builder setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) {
+            this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
+            return this;
+        }
+
         /**
          * @param failedIpCooldownPeriod how long a failed IP is deprioritized before it is re-probed;
          *                               {@code null} resets to the default. Must not be negative; use
@@ -1773,6 +1795,7 @@ public DefaultAsyncHttpClientConfig build() {
                     maxRequestRetry,
                     loadBalance,
                     failedIpCooldownEnabled,
+                    useAbsoluteRequestDeadline,
                     failedIpCooldownPeriod,
                     disableUrlEncodingForBoundRequests,
                     useLaxCookieEncoder,

client/src/main/java/org/asynchttpclient/DefaultRequest.java

@@ -63,6 +63,7 @@ public class DefaultRequest implements Request {
     private final @Nullable Realm realm;
     private final @Nullable File file;
     private final @Nullable Boolean followRedirect;
+    private final @Nullable Boolean useAbsoluteRequestDeadline;
     private final Duration requestTimeout;
     private final Duration readTimeout;
     private final long rangeOffset;
@@ -99,6 +100,49 @@ public DefaultRequest(String method,
                           @Nullable Charset charset,
                           ChannelPoolPartitioning channelPoolPartitioning,
                           NameResolver<InetAddress> nameResolver) {
+        this(method, uri, address, localAddress, headers, cookies, byteData, compositeByteData, stringData,
+                byteBufferData, byteBufData, streamData, bodyGenerator, formParams, bodyParts, virtualHost,
+                proxyServer, realm, file, followRedirect, requestTimeout, readTimeout, rangeOffset, charset,
+                channelPoolPartitioning, nameResolver, null);
+    }
+
+    /**
+     * Not public, and the parameter is trailing rather than beside {@code followRedirect}: the constructor above
+     * keeps the signature outside callers compile against, while this one stays free to grow. A public
+     * twenty-seven argument constructor would be pinned by revapi, and its parameter list would have to be kept
+     * in step with the one above by hand, with the compiler unable to help once the tail is all reference types.
+     * {@link RequestBuilderBase#build()} is the only caller.
+     *
+     * @param useAbsoluteRequestDeadline whether {@code requestTimeout} bounds the whole exchange rather than
+     *                                   each attempt within it, or null to defer to the client config
+     */
+    DefaultRequest(String method,
+                   Uri uri,
+                   @Nullable InetAddress address,
+                   @Nullable InetAddress localAddress,
+                   HttpHeaders headers,
+                   List<Cookie> cookies,
+                   byte @Nullable [] byteData,
+                   @Nullable List<byte[]> compositeByteData,
+                   @Nullable String stringData,
+                   @Nullable ByteBuffer byteBufferData,
+                   @Nullable ByteBuf byteBufData,
+                   @Nullable InputStream streamData,
+                   @Nullable BodyGenerator bodyGenerator,
+                   List<Param> formParams,
+                   List<Part> bodyParts,
+                   @Nullable String virtualHost,
+                   @Nullable ProxyServer proxyServer,
+                   @Nullable Realm realm,
+                   @Nullable File file,
+                   @Nullable Boolean followRedirect,
+                   @Nullable Duration requestTimeout,
+                   @Nullable Duration readTimeout,
+                   long rangeOffset,
+                   @Nullable Charset charset,
+                   ChannelPoolPartitioning channelPoolPartitioning,
+                   NameResolver<InetAddress> nameResolver,
+                   @Nullable Boolean useAbsoluteRequestDeadline) {
         this.method = method;
         this.uri = uri;
         this.address = address;
@@ -119,6 +163,7 @@ public DefaultRequest(String method,
         this.realm = realm;
         this.file = file;
         this.followRedirect = followRedirect;
+        this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
         this.requestTimeout = requestTimeout == null ? Duration.ZERO : requestTimeout;
         this.readTimeout = readTimeout == null ? Duration.ZERO : readTimeout;
         this.rangeOffset = rangeOffset;
@@ -232,6 +277,11 @@ public List<Part> getBodyParts() {
         return followRedirect;
     }
 
+    @Override
+    public @Nullable Boolean getUseAbsoluteRequestDeadline() {
+        return useAbsoluteRequestDeadline;
+    }
+
     @Override
     public Duration getRequestTimeout() {
         return requestTimeout;

client/src/main/java/org/asynchttpclient/Request.java

@@ -172,6 +172,17 @@ public interface Request {
     @Nullable
     Boolean getFollowRedirect();
 
+    /**
+     * Whether {@link #getRequestTimeout()} is a deadline for the whole exchange rather than for each attempt
+     * within it. See {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()}.
+     *
+     * @return the override, or null to use the config value
+     */
+    @Nullable
+    default Boolean getUseAbsoluteRequestDeadline() {
+        return null;
+    }
+
     /**
      * @return the request timeout. Non zero values means "override config value".
      */

client/src/main/java/org/asynchttpclient/RequestBuilderBase.java

@@ -87,6 +87,7 @@ public abstract class RequestBuilderBase<T extends RequestBuilderBase<T>> {
     protected @Nullable Realm realm;
     protected @Nullable File file;
     protected @Nullable Boolean followRedirect;
+    protected @Nullable Boolean useAbsoluteRequestDeadline;
     protected @Nullable Duration requestTimeout;
     protected @Nullable Duration readTimeout;
     protected long rangeOffset;
@@ -165,6 +166,7 @@ protected RequestBuilderBase(Request prototype, boolean disableUrlEncoding, bool
         realm = prototype.getRealm();
         file = prototype.getFile();
         followRedirect = prototype.getFollowRedirect();
+        useAbsoluteRequestDeadline = prototype.getUseAbsoluteRequestDeadline();
         requestTimeout = prototype.getRequestTimeout();
         readTimeout = prototype.getReadTimeout();
         rangeOffset = prototype.getRangeOffset();
@@ -598,6 +600,17 @@ public T setRealm(Realm realm) {
         return asDerivedType();
     }
 
+    /**
+     * @param useAbsoluteRequestDeadline whether this request's timeout is a deadline for the whole exchange
+     *                                   rather than for each attempt within it, overriding
+     *                                   {@link AsyncHttpClientConfig#isUseAbsoluteRequestDeadline()}
+     * @return {@code this}
+     */
+    public T setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) {
+        this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
+        return asDerivedType();
+    }
+
     public T setFollowRedirect(boolean followRedirect) {
         this.followRedirect = followRedirect;
         return asDerivedType();
@@ -685,7 +698,9 @@ private RequestBuilderBase<?> executeSignatureCalculator() {
         rb.realm = realm;
         rb.file = file;
         rb.followRedirect = followRedirect;
+        rb.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
         rb.requestTimeout = requestTimeout;
+        rb.readTimeout = readTimeout;
         rb.rangeOffset = rangeOffset;
         rb.charset = charset;
         rb.channelPoolPartitioning = channelPoolPartitioning;
@@ -755,6 +770,7 @@ public Request build() {
                 rb.rangeOffset,
                 rb.charset,
                 rb.channelPoolPartitioning,
-                rb.nameResolver);
+                rb.nameResolver,
+                rb.useAbsoluteRequestDeadline);
     }
 }

client/src/main/java/org/asynchttpclient/config/AsyncHttpClientConfigDefaults.java

@@ -63,6 +63,7 @@ public final class AsyncHttpClientConfigDefaults {
     public static final String MAX_REQUEST_RETRY_CONFIG = "maxRequestRetry";
     public static final String LOAD_BALANCE_CONFIG = "loadBalance";
     public static final String FAILED_IP_COOLDOWN_ENABLED_CONFIG = "failedIpCooldownEnabled";
+    public static final String USE_ABSOLUTE_REQUEST_DEADLINE_CONFIG = "useAbsoluteRequestDeadline";
     public static final String FAILED_IP_COOLDOWN_PERIOD_CONFIG = "failedIpCooldownPeriod";
     public static final String DISABLE_URL_ENCODING_FOR_BOUND_REQUESTS_CONFIG = "disableUrlEncodingForBoundRequests";
     public static final String USE_LAX_COOKIE_ENCODER_CONFIG = "useLaxCookieEncoder";
@@ -188,6 +189,10 @@ public static boolean defaultFailedIpCooldownEnabled() {
         return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + FAILED_IP_COOLDOWN_ENABLED_CONFIG);
     }
 
+    public static boolean defaultUseAbsoluteRequestDeadline() {
+        return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getBoolean(ASYNC_CLIENT_CONFIG_ROOT + USE_ABSOLUTE_REQUEST_DEADLINE_CONFIG);
+    }
+
     public static Duration defaultFailedIpCooldownPeriod() {
         return AsyncHttpClientConfigHelper.getAsyncHttpClientConfig().getDuration(ASYNC_CLIENT_CONFIG_ROOT + FAILED_IP_COOLDOWN_PERIOD_CONFIG);
     }

client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java

@@ -92,6 +92,10 @@ public final class NettyResponseFuture<V> implements ListenableFuture<V> {
             .newUpdater(NettyResponseFuture.class, Object.class, "partitionKeyLock");
 
     private final long start = unpreciseMillisTime();
+    // Wall clock is what start reports, and a deadline anchored on it for the length of a whole exchange would
+    // be moved by any clock correction that lands mid-chain: a step back would hand a later hop a budget it
+    // never had, a step forward would abort it on a healthy connection. Elapsed time is measured from here.
+    private final long startNanos = System.nanoTime();
     private final ChannelPoolPartitioning connectionPoolPartitioning;
     private final ConnectionSemaphore connectionSemaphore;
     // Not final: a filter replay can retarget this future at a different origin, reached through a
@@ -154,6 +158,9 @@ public final class NettyResponseFuture<V> implements ListenableFuture<V> {
     // future no longer takes, which is how a connection through a proxy comes to be offered as a direct one.
     // Volatile: the mutators run on the redirect and replay paths while reads happen on other threads.
     private volatile Object basePartitionKeyCache;
+    // Read when a TimeoutsHolder is built, which happens on the caller thread, an event loop or the timer
+    // thread depending on the path, so it is published rather than plain.
+    private volatile boolean useAbsoluteRequestDeadline;
 
     public NettyResponseFuture(Request originalRequest,
                                AsyncHandler<V> asyncHandler,
@@ -598,6 +605,14 @@ public long getStart() {
         return start;
     }
 
+    /**
+     * When this exchange was submitted, on a monotonic clock, for measuring how much of a deadline spanning the
+     * whole exchange it has spent. Comparable only with other {@link System#nanoTime()} readings.
+     */
+    public long getStartNanos() {
+        return startNanos;
+    }
+
     public Object getPartitionKey() {
         Object override = partitionKeyOverride;
         if (override != null) {
@@ -731,6 +746,21 @@ public void acquirePartitionLockLazily(boolean nonBlocking) throws IOException {
         }
     }
 
+    /**
+     * Whether this exchange's request timeout is a deadline for the exchange as a whole. Resolved once, from the
+     * request the caller submitted and the client config, and then kept here rather than re-read per hop: a
+     * redirect rebuilds the request from a hand-picked set of fields, so anything carried only on the request
+     * would silently revert to the config value partway through the exchange, which is exactly the case this
+     * setting exists for.
+     */
+    public boolean isUseAbsoluteRequestDeadline() {
+        return useAbsoluteRequestDeadline;
+    }
+
+    public void setUseAbsoluteRequestDeadline(boolean useAbsoluteRequestDeadline) {
+        this.useAbsoluteRequestDeadline = useAbsoluteRequestDeadline;
+    }
+
     public Realm getRealm() {
         return realm;
     }

client/src/main/java/org/asynchttpclient/netty/handler/intercept/Redirect30xInterceptor.java

@@ -139,7 +139,19 @@ public boolean exitAfterHandlingRedirect(Channel channel, NettyResponseFuture<?>
                         .setNameResolver(request.getNameResolver())
                         .setProxyServer(request.getProxyServer())
                         .setRealm(stripAuth ? null : request.getRealm())
-                        .setRequestTimeout(request.getRequestTimeout());
+                        .setRequestTimeout(request.getRequestTimeout())
+                        // Dropped here until now, so a per-request read timeout reverted to the config default
+                        // on every hop after the first.
+                        .setReadTimeout(request.getReadTimeout());
+
+                // Also dropped, which left the request disagreeing with the deadline the exchange was being
+                // held to: the future carries the flag, so a filter or a signature calculator reading the
+                // request saw per-attempt timeouts while the exchange was bounded as a whole. Only when it was
+                // set, since the setter takes a primitive and null means defer to the client config.
+                Boolean useAbsoluteRequestDeadline = request.getUseAbsoluteRequestDeadline();
+                if (useAbsoluteRequestDeadline != null) {
+                    requestBuilder.setUseAbsoluteRequestDeadline(useAbsoluteRequestDeadline);
+                }
 
                 if (stripAuth) {
                     future.setRealm(null);