Skip to content

Fix ADAPTIVE_V2 client close() hang when a queued request's callback … - #7517

Merged
davidh44 merged 2 commits into
masterfrom
hdavidh/fix-adaptive-retry-close-hang
Nov 5, 2026
Merged

davidh44 merged 2 commits into
masterfrom
hdavidh/fix-adaptive-retry-close-hang

Conversation

@davidh44

@davidh44 davidh44 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

…calls the same client

Motivation and Context

Fixes #7515. Since #7197 (2.52.0), closing a client that uses RetryMode.ADAPTIVE_V2 can spin forever at 100% CPU if a request queued in the adaptive rate limiter has a dependent that calls the same client.

RateLimiterTokenBucketStore.close() evicts every bucket through LruCache.evictAll(), which closes each value while holding listLock and before removing the entry from the map. Closing a bucket fails its queued requests on the closing thread, so their dependents run there too. If a dependent calls the same client, for example the abortMultipartUpload that multipart copy issues when a part fails, LruCache.get() re-enters listLock and keeps getting back the evicted entry that is still in the map. The loop never ends, and any other thread that touches the retry strategy, including Netty event loop threads recording a response, blocks on the lock that thread holds.

Modifications

  • RateLimiterTokenBucketStore: marks itself closed before evicting. After that, tokenBucketForScope() returns a pre-closed bucket without touching the cache. A bucket created in the race window as close starts is closed before the pre-closed bucket is returned.
  • LruCache: eviction marks the entry evicted, removes it from the map and LRU list under listLock, and closes the value only after releasing that lock. This applies to evictAll() and to size-based eviction in get().
  • RateLimiterTokenBucket: drains queued requests under its lock and fails them after releasing it. This covers close() and the paths where scheduling is rejected in acquireAsync() and doNotify().

Behavior changes

  • Once the retry strategy starts closing, new rate limiter lookups get a closed bucket instead of a new working one, so those requests fail right away. Callers see an SdkClientException caused by IllegalStateException: Rate limiter bucket is closed, the same failure requests already queued in the rate limiter get on close. A lookup that completed just before close started still gets the live bucket, which eviction then closes.
  • LruCache closes evicted values after releasing its internal list lock instead of while holding it, so different evicted values can now be closed at the same time. During evictAll(), a concurrent get() can now return a new entry while evicted values are still being closed, where before it waited for evictAll() to finish. This also applies to evictions in the S3 Express identity cache.

Testing

Added unit tests.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@dagnir dagnir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we incorporate the reproduction code from the issue as a test as well? We can put it in codegen-generated-classes-test

@davidh44
davidh44 merged commit af5bee1 into master Nov 5, 2026
10 of 11 checks passed
@davidh44
davidh44 deleted the hdavidh/fix-adaptive-retry-close-hang branch November 5, 2026 20:44
@davidh44
davidh44 restored the hdavidh/fix-adaptive-retry-close-hang branch November 5, 2026 20:44
@davidh44
davidh44 deleted the hdavidh/fix-adaptive-retry-close-hang branch November 5, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants