Fix the StackOverflowException in WaiterExecutor in case of large retries count. (#3956)

Author: flittev

.changes/next-release/bugfix-AWSSDKforJavav2-04bf228.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "flittev",
+    "description": "`WaiterExecutor` recursive implementation changed to iterative"
+}

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutor.java

@@ -16,14 +16,12 @@
 package software.amazon.awssdk.core.internal.waiters;
 
 import java.util.List;
-import java.util.Optional;
 import java.util.function.Supplier;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.annotations.ThreadSafe;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.core.waiters.WaiterAcceptor;
 import software.amazon.awssdk.core.waiters.WaiterResponse;
-import software.amazon.awssdk.core.waiters.WaiterState;
 import software.amazon.awssdk.utils.Either;
 import software.amazon.awssdk.utils.Validate;
 
@@ -45,45 +43,42 @@ public WaiterExecutor(WaiterConfiguration configuration,
     }
 
     WaiterResponse<T> execute(Supplier<T> pollingFunction) {
-        return doExecute(pollingFunction, 0, System.currentTimeMillis());
-    }
-
-    WaiterResponse<T> doExecute(Supplier<T> pollingFunction, int attemptNumber, long startTime) {
-        attemptNumber++;
-        T response;
-        try {
-            response = pollingFunction.get();
-        } catch (Exception exception) {
-            return evaluate(pollingFunction, Either.right(exception), attemptNumber, startTime);
-        }
-
-        return evaluate(pollingFunction, Either.left(response), attemptNumber, startTime);
-    }
+        int attemptNumber = 0;
+        long startTime = System.currentTimeMillis();
 
-    private WaiterResponse<T> evaluate(Supplier<T> pollingFunction,
-                                       Either<T, Throwable> responseOrException,
-                                       int attemptNumber,
-                                       long startTime) {
-        Optional<WaiterAcceptor<? super T>> waiterAcceptor = executorHelper.firstWaiterAcceptorIfMatched(responseOrException);
+        while (true) {
+            attemptNumber++;
 
-        if (waiterAcceptor.isPresent()) {
-            WaiterState state = waiterAcceptor.get().waiterState();
-            switch (state) {
+            Either<T, Throwable> polledResponse = pollResponse(pollingFunction);
+            WaiterAcceptor<? super T> waiterAcceptor = firstWaiterAcceptor(polledResponse);
+            switch (waiterAcceptor.waiterState()) {
                 case SUCCESS:
-                    return executorHelper.createWaiterResponse(responseOrException, attemptNumber);
+                    return executorHelper.createWaiterResponse(polledResponse, attemptNumber);
                 case RETRY:
-                    return maybeRetry(pollingFunction, attemptNumber, startTime);
+                    waitToRetry(attemptNumber, startTime);
+                    break;
                 case FAILURE:
-                    throw executorHelper.waiterFailureException(waiterAcceptor.get());
+                    throw executorHelper.waiterFailureException(waiterAcceptor);
                 default:
                     throw new UnsupportedOperationException();
             }
         }
+    }
+
+    private Either<T, Throwable> pollResponse(Supplier<T> pollingFunction) {
+        try {
+            return Either.left(pollingFunction.get());
+        } catch (Exception exception) {
+            return Either.right(exception);
+        }
+    }
 
-        throw executorHelper.noneMatchException(responseOrException);
+    private WaiterAcceptor<? super T> firstWaiterAcceptor(Either<T, Throwable> responseOrException) {
+        return executorHelper.firstWaiterAcceptorIfMatched(responseOrException)
+                             .orElseThrow(() -> executorHelper.noneMatchException(responseOrException));
     }
 
-    private WaiterResponse<T> maybeRetry(Supplier<T> pollingFunction, int attemptNumber, long startTime) {
+    private void waitToRetry(int attemptNumber, long startTime) {
         Either<Long, SdkClientException> nextDelayOrUnretryableException =
             executorHelper.nextDelayOrUnretryableException(attemptNumber, startTime);
 
@@ -97,6 +92,5 @@ private WaiterResponse<T> maybeRetry(Supplier<T> pollingFunction, int attemptNum
             Thread.currentThread().interrupt();
             throw SdkClientException.create("The thread got interrupted", e);
         }
-        return doExecute(pollingFunction, attemptNumber, startTime);
     }
 }

core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutorTest.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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.
+ */
+
+package software.amazon.awssdk.core.internal.waiters;
+
+import java.util.Arrays;
+import java.util.concurrent.atomic.LongAdder;
+import org.junit.jupiter.api.Test;
+import org.testng.Assert;
+import software.amazon.awssdk.core.retry.backoff.BackoffStrategy;
+import software.amazon.awssdk.core.waiters.WaiterAcceptor;
+import software.amazon.awssdk.core.waiters.WaiterOverrideConfiguration;
+
+class WaiterExecutorTest {
+    @Test
+    void largeMaxAttempts() {
+
+        int expectedAttempts = 10_000;
+
+        WaiterOverrideConfiguration conf =
+            WaiterOverrideConfiguration.builder()
+                                       .maxAttempts(expectedAttempts)
+                                       .backoffStrategy(BackoffStrategy.none())
+                                       .build();
+
+        WaiterExecutor<Integer> sut =
+            new WaiterExecutor<>(new WaiterConfiguration(conf),
+                                 Arrays.asList(
+                                     WaiterAcceptor.retryOnResponseAcceptor(c -> c < expectedAttempts),
+                                     WaiterAcceptor.successOnResponseAcceptor(c -> c == expectedAttempts)
+                                 ));
+
+        LongAdder attemptCounter = new LongAdder();
+        sut.execute(() -> {
+            attemptCounter.increment();
+            return attemptCounter.intValue();
+        });
+
+        Assert.assertEquals(attemptCounter.intValue(), expectedAttempts);
+    }
+}