Skip to content

Commit f770947

Browse files
authored
Merge pull request #14222 from lovesegfault/curl-based-s3-fix-race
fix(libstore): fix race condition in AWS credential provider caching
2 parents 4f585de + f0e1f65 commit f770947

File tree

1 file changed

+50
-41
lines changed

1 file changed

+50
-41
lines changed

src/libstore/aws-creds.cc

Lines changed: 50 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -118,48 +118,57 @@ AwsCredentials getAwsCredentials(const std::string & profile)
118118
// Get or create credential provider with caching
119119
std::shared_ptr<Aws::Crt::Auth::ICredentialsProvider> provider;
120120

121-
// Try to find existing provider
122-
credentialProviderCache.visit(profile, [&](const auto & pair) { provider = pair.second; });
123-
124-
if (!provider) {
125-
// Create new provider if not found
126-
debug(
127-
"[pid=%d] creating new AWS credential provider for profile '%s'",
128-
getpid(),
129-
profile.empty() ? "(default)" : profile.c_str());
130-
131-
try {
132-
initAwsCrt();
133-
134-
if (profile.empty()) {
135-
Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config;
136-
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
137-
provider = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChainDefault(config);
138-
} else {
139-
Aws::Crt::Auth::CredentialsProviderProfileConfig config;
140-
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
141-
// This is safe because the underlying C library will copy this string
142-
// c.f. https://github.com/awslabs/aws-c-auth/blob/main/source/credentials_provider_profile.c#L220
143-
config.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str());
144-
provider = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(config);
145-
}
146-
} catch (Error & e) {
147-
e.addTrace(
148-
{},
149-
"while creating AWS credentials provider for %s",
150-
profile.empty() ? "default profile" : fmt("profile '%s'", profile));
151-
throw;
152-
}
153-
154-
if (!provider) {
155-
throw AwsAuthError(
156-
"Failed to create AWS credentials provider for %s",
157-
profile.empty() ? "default profile" : fmt("profile '%s'", profile));
158-
}
121+
// Use try_emplace_and_cvisit for atomic get-or-create
122+
// This prevents race conditions where multiple threads create providers
123+
credentialProviderCache.try_emplace_and_cvisit(
124+
profile,
125+
nullptr, // Placeholder - will be replaced in f1 before any thread can see it
126+
[&](auto & kv) {
127+
// f1: Called atomically during insertion with non-const reference
128+
// Other threads are blocked until we finish, so nullptr is never visible
129+
debug(
130+
"[pid=%d] creating new AWS credential provider for profile '%s'",
131+
getpid(),
132+
profile.empty() ? "(default)" : profile.c_str());
159133

160-
// Insert into cache (try_emplace is thread-safe and won't overwrite if another thread added it)
161-
credentialProviderCache.try_emplace(profile, provider);
162-
}
134+
try {
135+
initAwsCrt();
136+
137+
if (profile.empty()) {
138+
Aws::Crt::Auth::CredentialsProviderChainDefaultConfig config;
139+
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
140+
kv.second = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderChainDefault(config);
141+
} else {
142+
Aws::Crt::Auth::CredentialsProviderProfileConfig config;
143+
config.Bootstrap = Aws::Crt::ApiHandle::GetOrCreateStaticDefaultClientBootstrap();
144+
// This is safe because the underlying C library will copy this string
145+
// c.f. https://github.com/awslabs/aws-c-auth/blob/main/source/credentials_provider_profile.c#L220
146+
config.ProfileNameOverride = Aws::Crt::ByteCursorFromCString(profile.c_str());
147+
kv.second = Aws::Crt::Auth::CredentialsProvider::CreateCredentialsProviderProfile(config);
148+
}
149+
150+
if (!kv.second) {
151+
throw AwsAuthError(
152+
"Failed to create AWS credentials provider for %s",
153+
profile.empty() ? "default profile" : fmt("profile '%s'", profile));
154+
}
155+
156+
provider = kv.second;
157+
} catch (Error & e) {
158+
// Exception during creation - remove the entry to allow retry
159+
credentialProviderCache.erase(profile);
160+
e.addTrace({}, "for AWS profile: %s", profile.empty() ? "(default)" : profile);
161+
throw;
162+
} catch (...) {
163+
// Non-Error exception - still need to clean up
164+
credentialProviderCache.erase(profile);
165+
throw;
166+
}
167+
},
168+
[&](const auto & kv) {
169+
// f2: Called if key already exists (const reference)
170+
provider = kv.second;
171+
});
163172

164173
return getCredentialsFromProvider(provider);
165174
}

0 commit comments

Comments
 (0)