Race condition in EndpointDiscoveryRefreshCache can trigger duplicate concurrent endpoint-discovery refresh calls

[Adityaj0]

Describe the bug

EndpointDiscoveryRefreshCache#returnCachedOrDefaultEndpoint decides whether to kick off a background refresh of an expired cached endpoint with an unguarded check-then-act:

// core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java
private URI returnCachedOrDefaultEndpoint(String key, EndpointDiscoveryEndpoint endpoint, EndpointDiscoveryRequest request) {
    ...
    if (endpoint.expirationTime().isBefore(Instant.now())) {
        cache.put(key, endpoint.toBuilder().expirationTime(Instant.now().plusSeconds(60)).build());
        refreshCacheAsync(request, key);
    }
    return endpoint.endpoint();
}

Neither the isBefore read nor the cache.put/refreshCacheAsync decision is guarded by any synchronization, CAS, or "refresh in flight" flag. This is inconsistent with the sibling branch a few lines above (the "no cached entry yet" case), which correctly uses cache.putIfAbsent as a compare-and-swap so only one caller wins and triggers discovery:

if (endpoint == null) {
    EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint);
    if (previousValue != null) {
        return previousValue.endpoint();
    }
    refreshCacheAsync(request, key);
    return tempEndpoint.endpoint();
}

Every thread that reads the same expired cache entry before any of them writes back independently decides it's expired and independently calls refreshCacheAsync, firing duplicate calls against the real endpoint-discovery service API for the same cache key — exactly at the moment of highest concurrent load, right when an entry expires.

Expected Behavior

Only one background refresh call should be triggered per cache-key expiration, regardless of how many threads concurrently observe the expired entry — matching the guarantee already provided for the "no cached entry yet" case.

Current Behavior

Multiple threads racing on the same just-expired entry can each independently call refreshCacheAsync, resulting in duplicate concurrent calls to the endpoint-discovery API for the same key.

Reproduction Steps

Since this class's control flow only depends on java.net.URI/java.time.Instant/ConcurrentHashMap, I extracted a byte-for-byte faithful copy of EndpointDiscoveryRefreshCache and EndpointDiscoveryEndpoint (only EndpointDiscoveryRequest/EndpointDiscoveryCacheLoader/EndpointDiscoveryFailedException were stubbed with matching signatures to avoid pulling in the rest of sdk-core's dependency graph) into a standalone harness, compiled and run with plain javac/java (Java 21).

The harness primes the cache with an already-expired entry (via reflection into the private cache field, to simulate the exact moment an entry expires), then releases 50 threads simultaneously to call get() against it, counting how many times the background discovery call actually fires. Expected: 1. Observed against the current code (5 runs):

Concurrent callers: 50
Background discovery (refresh) calls fired: 2
Concurrent callers: 50
Background discovery (refresh) calls fired: 2
Concurrent callers: 50
Background discovery (refresh) calls fired: 1
Concurrent callers: 50
Background discovery (refresh) calls fired: 2
Concurrent callers: 50
Background discovery (refresh) calls fired: 1

Under real network latency for the discovery call (vs. this instant in-process stub), the race window is much wider and the duplicate-call count would be higher for a burst of concurrent requests hitting a just-expired key.

Possible Solution

Guard the refresh decision with cache.replace(key, oldValue, newValue) — a compare-and-swap against the exact stale value each caller read — so only the thread that actually wins the race replaces the entry and triggers the refresh. I have a fix (verified against the same harness — repeated runs now consistently show exactly 1 refresh call) and will open a PR shortly.

Version

Reproduced against current master (commit e377d53f, 2.53.2-SNAPSHOT).