From bc17929e625b0bb5cd35ad740dce6d1007c85982 Mon Sep 17 00:00:00 2001 From: Ivan Shkundalev Date: Thu, 27 Apr 2023 12:13:41 +0100 Subject: [PATCH] Fix the StackOverflowException in WaiterExecutor in case of large retries count. --- .../bugfix-AWSSDKforJavav2-04bf228.json | 6 +++ .../core/internal/waiters/WaiterExecutor.java | 54 +++++++++---------- .../internal/waiters/WaiterExecutorTest.java | 53 ++++++++++++++++++ 3 files changed, 83 insertions(+), 30 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSSDKforJavav2-04bf228.json create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutorTest.java diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-04bf228.json b/.changes/next-release/bugfix-AWSSDKforJavav2-04bf228.json new file mode 100644 index 000000000000..371b1d0d7b87 --- /dev/null +++ b/.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" +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutor.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutor.java index 8610c32e49f3..5377e0f04e59 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutor.java +++ b/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 execute(Supplier pollingFunction) { - return doExecute(pollingFunction, 0, System.currentTimeMillis()); - } - - WaiterResponse doExecute(Supplier 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 evaluate(Supplier pollingFunction, - Either responseOrException, - int attemptNumber, - long startTime) { - Optional> waiterAcceptor = executorHelper.firstWaiterAcceptorIfMatched(responseOrException); + while (true) { + attemptNumber++; - if (waiterAcceptor.isPresent()) { - WaiterState state = waiterAcceptor.get().waiterState(); - switch (state) { + Either polledResponse = pollResponse(pollingFunction); + WaiterAcceptor 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 pollResponse(Supplier pollingFunction) { + try { + return Either.left(pollingFunction.get()); + } catch (Exception exception) { + return Either.right(exception); + } + } - throw executorHelper.noneMatchException(responseOrException); + private WaiterAcceptor firstWaiterAcceptor(Either responseOrException) { + return executorHelper.firstWaiterAcceptorIfMatched(responseOrException) + .orElseThrow(() -> executorHelper.noneMatchException(responseOrException)); } - private WaiterResponse maybeRetry(Supplier pollingFunction, int attemptNumber, long startTime) { + private void waitToRetry(int attemptNumber, long startTime) { Either nextDelayOrUnretryableException = executorHelper.nextDelayOrUnretryableException(attemptNumber, startTime); @@ -97,6 +92,5 @@ private WaiterResponse maybeRetry(Supplier pollingFunction, int attemptNum Thread.currentThread().interrupt(); throw SdkClientException.create("The thread got interrupted", e); } - return doExecute(pollingFunction, attemptNumber, startTime); } } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutorTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/waiters/WaiterExecutorTest.java new file mode 100644 index 000000000000..2df65de46e0b --- /dev/null +++ b/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 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); + } +} \ No newline at end of file