envoyproxy · isker · Aug 23, 2026
diff --git a/changelogs/current/bug_fixes/aws__fixed-signing-deadlock-during-server-init.rst b/changelogs/current/bug_fixes/aws__fixed-signing-deadlock-during-server-init.rst
@@ -0,0 +1,3 @@
+Fixed an initialization hang when an extension that gates server initialization
+sends an HTTP callout to a cluster whose AWS request signing filter resolves
+credentials asynchronously.
@@ -134,7 +134,8 @@ envoy_cc_library(
         ":metadata_fetcher_lib",
         ":utility_lib",
         "//envoy/common:time_interface",
-        "//source/common/common:cancel_wrapper_lib",
+        "//source/common/common:assert_lib",
+        "//source/common/common:thread_lib",
     ],
 )
 

@@ -105,7 +105,7 @@ void AssumeRoleCredentialsProvider::continueRefresh() {
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }

@@ -66,7 +66,7 @@ void ContainerCredentialsProvider::refresh() {
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }

@@ -110,7 +110,7 @@ void IAMRolesAnywhereCredentialsProvider::refresh() {
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }

@@ -47,7 +47,7 @@ void InstanceProfileCredentialsProvider::refresh() {
   continue_on_async_fetch_failure_reason_ = "Token fetch failed, falling back to IMDSv1";
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(token_req_message, Tracing::NullSpan::instance(), *this);
 }
@@ -71,7 +71,7 @@ void InstanceProfileCredentialsProvider::fetchInstanceRoleAsync(const std::strin
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }
@@ -116,7 +116,7 @@ void InstanceProfileCredentialsProvider::fetchCredentialFromInstanceRoleAsync(
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }

@@ -79,7 +79,7 @@ void WebIdentityCredentialsProvider::refresh() {
   };
 
   // mark credentials as pending while async completes
-  credentials_pending_.store(true);
+  setCredentialsPendingToAllThreads();
 
   metadata_fetcher_->fetch(message, Tracing::NullSpan::instance(), *this);
 }

@@ -168,8 +168,11 @@ using CredentialSubscriberCallbacksSharedPtr = std::shared_ptr<CredentialSubscri
 // Subscription model allowing CredentialsProviderChains to be notified of credential provider
 // updates. A credential provider chain will call credential_provider->subscribeToCredentialUpdates
 // to register itself for updates via onCredentialUpdate callback. When a credential provider has
-// successfully updated all threads with new credentials, via the setCredentialsToAllThreads method
-// it will notify all subscribers that credentials have been retrieved.
+// posted new credentials to all threads, via the setCredentialsToAllThreads method it will notify
+// all subscribers that credentials have been retrieved.
+//
+// Notification happens as soon as the update has been posted, not once every thread has applied it,
+// because the main thread may need credentials before worker threads are running at all.
 //
 // Subscription is only relevant for metadata credentials providers, as these are the only
 // credential providers that implement async credential retrieval functionality.

@@ -4,6 +4,9 @@
 
 #include "envoy/server/factory_context.h"
 
+#include "source/common/common/assert.h"
+#include "source/common/common/thread.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace Common {
@@ -34,7 +37,6 @@ MetadataCredentialsProviderBase::MetadataCredentialsProviderBase(
 };
 
 MetadataCredentialsProviderBase::~MetadataCredentialsProviderBase() {
-  cancel_credentials_update_callback_();
   if (metadata_fetcher_) {
     metadata_fetcher_->cancel();
   }
@@ -68,7 +70,15 @@ void MetadataCredentialsProviderBase::credentialsRetrievalError() {
   handleFetchDone();
 }
 
-bool MetadataCredentialsProviderBase::credentialsPending() { return credentials_pending_; }
+bool MetadataCredentialsProviderBase::credentialsPending() {
+  if (!tls_slot_->currentThreadRegistered()) {
+    ASSERT(false, "AWS credentials provider queried from a thread with no thread local storage");
+    return true;
+  }
+  auto cache = tls_slot_->get();
+  ASSERT(cache.has_value());
+  return !cache.has_value() || cache->credentials_pending_;
+}
 
 Credentials MetadataCredentialsProviderBase::getCredentials() {
   return *(*tls_slot_)->credentials_.get();
@@ -140,28 +150,82 @@ void MetadataCredentialsProviderBase::setCredentialsToAllThreads(
 
   CredentialsConstSharedPtr shared_credentials = std::move(creds);
   if (tls_slot_ && !tls_slot_->isShutdown()) {
+    // A weak_ptr rather than a raw `this`, so that a completion callback still queued when the
+    // provider goes away becomes a no-op instead of a use-after-free.
+    std::weak_ptr<MetadataCredentialsProviderBase> weak_self = weak_from_this();
+
+    // Set the credentials and clear the pending flag as a single update, so that no thread can
+    // observe one without the other. This writes the main thread's slot synchronously and posts the
+    // same update to every registered worker dispatcher.
     tls_slot_->runOnAllThreads(
-        /* Set the credentials */ [shared_credentials](
-                                      OptRef<ThreadLocalCredentialsCache>
-                                          obj) { obj->credentials_ = shared_credentials; },
-        /* Notify waiting signers on completion of credential setting above */
-        CancelWrapper::cancelWrapped(
-            [this]() {
-              credentials_pending_.store(false);
-              std::list<std::weak_ptr<CredentialSubscriberCallbacks>> subscribers_copy;
-              {
-                Thread::LockGuard guard(mu_);
-                subscribers_copy = credentials_subscribers_;
-              }
-              for (auto& weak_cb : subscribers_copy) {
-                if (auto cb = weak_cb.lock()) {
-                  ENVOY_LOG(debug, "Notifying subscriber of credential update");
-                  cb->onCredentialUpdate();
-                }
-              }
-            },
-            &cancel_credentials_update_callback_));
+        [shared_credentials](OptRef<ThreadLocalCredentialsCache> obj) {
+          obj->credentials_ = shared_credentials;
+          obj->credentials_pending_ = false;
+        },
+        // Notify a second time once every worker has applied the update. Between the immediate
+        // notification below and a worker applying its update, that worker still reads
+        // `credentials_pending_ == true` from its own slot, so it can queue a pending callback
+        // after the immediate notification has already drained the queue. This notification is
+        // what wakes such a callback; without it the request stalls until the next successful
+        // refresh.
+        [weak_self]() {
+          if (auto self = weak_self.lock()) {
+            self->notifySubscribers();
+          }
+        });
+
+    // Notify waiting signers from this thread as well, rather than relying only on the
+    // all-threads-complete callback above. That callback does not run until every registered worker
+    // dispatcher has handled the posted update, and worker dispatchers do not start running until
+    // `startWorkers()`, which waits on server initialization. The main thread might like to use
+    // credentials before that point, and waiting for the workers would deadlock. (For example, a
+    // dynamic modules bootstrap extension might want to make an HTTP callout before it signals
+    // server init is complete.)
+    //
+    // For subscribers that post their wakeup to their own dispatcher (the AWS request signing and
+    // Lambda filters do), notifying here is ordered correctly: the credential update above is
+    // posted to each worker first, and dispatcher post queues are FIFO, so a worker applies the
+    // update before it runs the wakeup that reads it. Subscribers that instead run their callback
+    // inline rely on this being the main thread, whose slot `runOnAllThreads` has already updated.
+    //
+    // Note that unlike the completion callback, this notification runs on the caller's stack, which
+    // for a credential refresh is inside MetadataFetcher::onSuccess()/onMetadataError() and ahead
+    // of handleFetchDone(). No subscriber re-enters the provider today, but one that did would see
+    // a half-finished refresh.
+    notifySubscribers();
+  }
+}
+
+void MetadataCredentialsProviderBase::notifySubscribers() {
+  std::list<std::weak_ptr<CredentialSubscriberCallbacks>> subscribers_copy;
+  {
+    Thread::LockGuard guard(mu_);
+    subscribers_copy = credentials_subscribers_;
+  }
+  for (auto& weak_cb : subscribers_copy) {
+    if (auto cb = weak_cb.lock()) {
+      ENVOY_LOG(debug, "Notifying subscriber of credential update");
+      cb->onCredentialUpdate();
+    }
+  }
+}
+
+void MetadataCredentialsProviderBase::setCredentialsPendingToAllThreads() {
+  // The dedup below reads the main thread's slot, so it is only meaningful on the main thread. Not
+  // relying on the assertion inside runOnAllThreads(), because the dedup can return before ever
+  // reaching it.
+  ASSERT_IS_MAIN_OR_TEST_THREAD();
+  if (!tls_slot_ || tls_slot_->isShutdown()) {
+    return;
+  }
+  // The main thread's slot is written synchronously by runOnAllThreads, so it always holds the most
+  // recently initiated update. If it already says pending then every other thread has the same
+  // update applied or queued, and there is nothing to broadcast.
+  if ((*tls_slot_)->credentials_pending_) {
+    return;
   }
+  tls_slot_->runOnAllThreads(
+      [](OptRef<ThreadLocalCredentialsCache> obj) { obj->credentials_pending_ = true; });
 }
 
 CredentialSubscriberCallbacksHandlePtr

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "source/common/common/cancel_wrapper.h"
 #include "source/extensions/common/aws/aws_cluster_manager.h"
 #include "source/extensions/common/aws/credentials_provider.h"
 #include "source/extensions/common/aws/metadata_fetcher.h"
@@ -61,13 +60,17 @@ class MetadataCredentialsProviderBase
   subscribeToCredentialUpdates(CredentialSubscriberCallbacksSharedPtr cs);
 
 protected:
+  // Per-thread credential state. Both members are only ever touched on the thread that owns the
+  // slot: the main thread writes its own copy synchronously from setCredentialsToAllThreads and
+  // setCredentialsPendingToAllThreads, and every other thread applies the same update from its own
+  // dispatcher.
   struct ThreadLocalCredentialsCache : public ThreadLocal::ThreadLocalObject {
     ThreadLocalCredentialsCache() : credentials_(std::make_shared<Credentials>()) {};
 
     // The credentials object.
     CredentialsConstSharedPtr credentials_;
-    // Lock guard.
-    Thread::MutexBasicLockable lock_;
+    // Are credentials pending on this thread?
+    bool credentials_pending_{true};
   };
 
   // Set anonymous credentials to all threads, update stats and close async
@@ -81,9 +84,18 @@ class MetadataCredentialsProviderBase
   // Handle fetch done.
   void handleFetchDone();
 
-  // Set Credentials shared_ptr on all threads.
+  // Set Credentials shared_ptr on all threads, and mark credentials as no longer pending.
   void setCredentialsToAllThreads(CredentialsConstUniquePtr&& creds);
 
+  // Tell every subscriber that credentials have been retrieved. Called both from the thread that
+  // initiates a credential update and again once all threads have applied it; see
+  // setCredentialsToAllThreads. Subscriber notification is idempotent.
+  void notifySubscribers();
+
+  // Mark credentials as pending on all threads, ahead of an async credential fetch. Must be called
+  // on the main thread.
+  void setCredentialsPendingToAllThreads();
+
   virtual void refresh() PURE;
 
   Server::Configuration::ServerFactoryContext& context_;
@@ -122,12 +134,9 @@ class MetadataCredentialsProviderBase
   AwsClusterManagerPtr aws_cluster_manager_;
   // RAII handle for callbacks from AWS cluster manager
   AwsManagedClusterUpdateCallbacksHandlePtr callback_handle_;
-  // Are credentials pending?
-  std::atomic<bool> credentials_pending_ = true;
   Thread::MutexBasicLockable mu_;
   std::list<std::weak_ptr<CredentialSubscriberCallbacks>>
       credentials_subscribers_ ABSL_GUARDED_BY(mu_);
-  CancelWrapper::CancelFunction cancel_credentials_update_callback_ = []() {};
 };
 
 } // namespace Aws

@@ -105,11 +105,14 @@ envoy_cc_test(
     srcs = ["credentials_provider_test.cc"],
     rbe_pool = "6gig",
     deps = [
+        "//source/common/thread_local:thread_local_lib",
         "//source/extensions/common/aws:credentials_provider_interface",
         "//source/extensions/common/aws:utility_lib",
         "//source/extensions/common/aws/signers:sigv4_signer_impl_lib",
         "//test/extensions/common/aws:aws_mocks",
         "//test/mocks/server:factory_context_mocks",
         "//test/test_common:status_utility_lib",
+        "//test/test_common:utility_lib",
+        "@abseil-cpp//absl/synchronization",
     ],
 )
@@ -973,8 +973,10 @@ TEST_F(AssumeRoleCredentialsProviderTest, WithExternalId) {
   EXPECT_EQ("test-access-key", credentials.accessKeyId().value());
 }
 
-// Tests ASAN failure when cancel wrapper is not used
-TEST_F(AssumeRoleCredentialsProviderTest, CancelWrapperPreventsUseAfterFree) {
+// setCredentialsToAllThreads() leaves an all-threads-complete callback outstanding, which may run
+// after the provider is gone. It captures a weak_ptr, so firing it then must be a no-op rather than
+// a use-after-free (this test fails under ASAN if the callback captures the provider directly).
+TEST_F(AssumeRoleCredentialsProviderTest, DeferredCompletionCallbackSafeAfterDestruction) {
   std::function<void()> captured_callback;
 
   EXPECT_CALL(context_.thread_local_, runOnAllThreads(testing::_, testing::_))

@@ -769,8 +769,10 @@ not json
   delete (raw_metadata_fetcher_);
 }
 
-// Tests ASAN failure when cancel wrapper is not used
-TEST_F(InstanceProfileCredentialsProviderTest, CancelWrapperPreventsUseAfterFree) {
+// setCredentialsToAllThreads() leaves an all-threads-complete callback outstanding, which may run
+// after the provider is gone. It captures a weak_ptr, so firing it then must be a no-op rather than
+// a use-after-free (this test fails under ASAN if the callback captures the provider directly).
+TEST_F(InstanceProfileCredentialsProviderTest, DeferredCompletionCallbackSafeAfterDestruction) {
   std::function<void()> captured_callback;
 
   EXPECT_CALL(context_.thread_local_, runOnAllThreads(testing::_, testing::_))