Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AttributeMap should not close executor service in JDK19 #4395

Closed
scrocquesel opened this issue Sep 5, 2023 · 4 comments
Closed

AttributeMap should not close executor service in JDK19 #4395

scrocquesel opened this issue Sep 5, 2023 · 4 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@scrocquesel
Copy link
Contributor

scrocquesel commented Sep 5, 2023

Describe the bug

With JDK19+, the first loop in the AttributeMap::close method could lead to deadlock waiting for the executor service to terminate in the same thread that should stop the executor service.

public void close() {
attributes.values().forEach(v -> IoUtils.closeIfCloseable(v, null));
attributes.values().forEach(this::shutdownIfExecutorService);
}

Starting JDK 19+, ExecutorService now implements AutoCloseable. I know this version is not yet supported but excluding explicitely ExecutorService from the first loop will avoid issues in the future and let users move to newer JDK version

Expected Behavior

I should be able to Close the Client and terminate the overriden executor service in this order in the same thread.

Current Behavior

The actual code lead to a deadlock

Reproduction Steps

// provides a custom executor service
ClientOverrideConfiguration.builder().scheduledExecutorService(scheduledExecutorService);

// close the client, then shutdown the given executor
client.close(); // will call scheduledExecutorService.awaitTermination();
scheduledExecutorService.shutdown(); // never reached

Possible Solution

  • Explicitely exclude ExecutorService from autocloseable loop in AttributeMap::close
  • Do not delegate call to the wrapped implementation in UnmanagedScheduledExecutorService::awaitTermination

Additional Information/Context

Stack trace of the origin of the deadlock

 java.base@20.0.2/jdk.internal.misc.Unsafe.park(Native Method)
    java.base@20.0.2/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:269)
    org.jboss.threads.EnhancedQueueExecutor.awaitTermination(EnhancedQueueExecutor.java:920)
    software.amazon.awssdk.utils.ScheduledExecutorUtils$UnmanagedScheduledExecutorService.awaitTermination(ScheduledExecutorUtils.java:85)
    java.base@20.0.2/java.util.concurrent.ExecutorService.close(ExecutorService.java:417)
    software.amazon.awssdk.utils.IoUtils.closeQuietly(IoUtils.java:70)
    software.amazon.awssdk.utils.IoUtils.closeIfCloseable(IoUtils.java:87)
    software.amazon.awssdk.utils.AttributeMap.lambda$close$0(AttributeMap.java:87)
    software.amazon.awssdk.utils.AttributeMap$$Lambda$4721/0x00000008021f8690.accept(Unknown Source)
    java.base@20.0.2/java.util.HashMap$Values.forEach(HashMap.java:1073)
    software.amazon.awssdk.utils.AttributeMap.close(AttributeMap.java:87)
    software.amazon.awssdk.core.client.config.SdkClientConfiguration.close(SdkClientConfiguration.java:79)
    software.amazon.awssdk.core.internal.http.HttpClientDependencies.close(HttpClientDependencies.java:80)
    software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient.close(AmazonSyncHttpClient.java:75)
    software.amazon.awssdk.core.internal.handler.BaseSyncClientHandler.close(BaseSyncClientHandler.java:88)
    software.amazon.awssdk.services.s3.DefaultS3Client.close(DefaultS3Client.java:10525)

AWS Java SDK version used

2.20.129

JDK version used

19+

Operating System and version

linux

@scrocquesel scrocquesel added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 5, 2023
@debora-ito debora-ito added needs-review This issue or PR needs review from the team. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 6, 2023
@debora-ito
Copy link
Member

@scrocquesel thank you for reaching out, we'll review the PR #4404.

@debora-ito debora-ito added p2 This is a standard priority issue and removed needs-review This issue or PR needs review from the team. labels Sep 11, 2023
@tiny-george
Copy link

Same here. I have a Quarkus application, and tests are hanging forever in Jenkins because we have Java 20. Also running tests from the IDE you can detect the issue because you need to terminate the tests manually.

@debora-ito
Copy link
Member

Fixed via PR #4649, released in SDK version 2.21.15.

Copy link

github-actions bot commented Dec 5, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants