fix: old provider not shutting down when the new provider fails to initialize (#98)

## This PR
Ensures that a displaced feature provider is always shut down when a new
provider is set, preventing resource leaks when the new provider fails
to initialize.
Previously, the old provider was removed from the internal repository
map immediately upon setting a new one, but `Shutdown()` was only called
if the new provider successfully initialized. If the new provider's
`Init()` failed, the old provider remained active but unreferenced,
leaking its resources.

### Related Issues
Fixes #69

---------

Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>

Author: NeaguGeorgiana23

openfeature/provider_repository.cpp

@@ -1,5 +1,6 @@
 #include "openfeature/provider_repository.h"
 
+#include <algorithm>
 #include <iostream>
 
 #include "absl/status/statusor.h"
@@ -30,9 +31,9 @@ ProviderRepository::GetFeatureProviderStatusManager(
     return default_manager_;
   }
 
-  auto it = provider_manager_.find(std::string(domain));
-  if (it != provider_manager_.end()) {
-    return it->second;
+  auto provider_manager_it = provider_manager_.find(std::string(domain));
+  if (provider_manager_it != provider_manager_.end()) {
+    return provider_manager_it->second;
   }
 
   return default_manager_;
@@ -49,32 +50,30 @@ std::shared_ptr<FeatureProvider> ProviderRepository::GetProvider(
   return nullptr;
 }
 
-void ProviderRepository::SetProvider(std::shared_ptr<FeatureProvider> provider,
-                                     const EvaluationContext& ctx,
-                                     bool wait_for_init) {
+void ProviderRepository::SetProvider(
+    const std::shared_ptr<FeatureProvider>& provider,
+    const EvaluationContext& ctx, bool wait_for_init) {
   if (!provider) {
-    std::cerr << "Provider cannot be null" << std::endl;
+    std::cerr << "Provider cannot be null\n";
     return;
   }
-  PrepareAndInitializeProvider(std::nullopt, std::move(provider), ctx,
-                               wait_for_init);
+  PrepareAndInitializeProvider(std::nullopt, provider, ctx, wait_for_init);
 }
 
-void ProviderRepository::SetProvider(std::string_view domain,
-                                     std::shared_ptr<FeatureProvider> provider,
-                                     const EvaluationContext& ctx,
-                                     bool wait_for_init) {
+void ProviderRepository::SetProvider(
+    std::string_view domain, const std::shared_ptr<FeatureProvider>& provider,
+    const EvaluationContext& ctx, bool wait_for_init) {
   if (!provider) {
-    std::cerr << "Provider cannot be null" << std::endl;
+    std::cerr << "Provider cannot be null\n";
     return;
   }
 
   if (domain.empty()) {
-    SetProvider(std::move(provider), ctx, wait_for_init);
+    SetProvider(provider, ctx, wait_for_init);
     return;
   }
 
-  PrepareAndInitializeProvider(std::string(domain), std::move(provider), ctx,
+  PrepareAndInitializeProvider(std::string(domain), provider, ctx,
                                wait_for_init);
 }
 
@@ -90,7 +89,7 @@ ProviderStatus ProviderRepository::GetProviderStatus(
 
 void ProviderRepository::Shutdown() {
   {
-    std::lock_guard<std::mutex> lock(threads_mutex_);
+    std::scoped_lock lock(threads_mutex_);
     for (std::thread& thread : initialization_threads_) {
       if (thread.joinable()) {
         thread.join();
@@ -128,9 +127,9 @@ void ProviderRepository::Shutdown() {
 }
 
 void ProviderRepository::PrepareAndInitializeProvider(
-    const std::optional<std::string> domain,
-    std::shared_ptr<FeatureProvider> new_provider, const EvaluationContext& ctx,
-    bool wait_for_init) {
+    const std::optional<std::string>& domain,
+    const std::shared_ptr<FeatureProvider>& new_provider,
+    const EvaluationContext& ctx, bool wait_for_init) {
   std::shared_ptr<FeatureProviderStatusManager> new_status_manager;
   std::shared_ptr<FeatureProviderStatusManager> old_status_manager;
 
@@ -144,7 +143,7 @@ void ProviderRepository::PrepareAndInitializeProvider(
           FeatureProviderStatusManager::Create(new_provider);
       if (!manager.ok()) {
         std::cerr << "Failed to create FeatureProviderStatusManager: "
-                  << manager.status() << std::endl;
+                  << manager.status() << "\n";
         return;
       }
       new_status_manager = std::move(manager.value());
@@ -154,9 +153,9 @@ void ProviderRepository::PrepareAndInitializeProvider(
 
     if (domain) {
       // Setting a named provider.
-      auto it = provider_manager_.find(domain.value());
-      if (it != provider_manager_.end()) {
-        old_status_manager = it->second;
+      auto provider_manager_it = provider_manager_.find(domain.value());
+      if (provider_manager_it != provider_manager_.end()) {
+        old_status_manager = provider_manager_it->second;
       }
       provider_manager_[domain.value()] = new_status_manager;
     } else {
@@ -169,7 +168,7 @@ void ProviderRepository::PrepareAndInitializeProvider(
   if (wait_for_init) {
     InitializeProvider(new_status_manager, old_status_manager, ctx);
   } else {
-    std::lock_guard<std::mutex> lock(threads_mutex_);
+    std::scoped_lock lock(threads_mutex_);
     initialization_threads_.emplace_back(
         [this, new_status_manager, old_status_manager, ctx] {
           InitializeProvider(new_status_manager, old_status_manager, ctx);
@@ -178,20 +177,18 @@ void ProviderRepository::PrepareAndInitializeProvider(
 }
 
 void ProviderRepository::InitializeProvider(
-    std::shared_ptr<FeatureProviderStatusManager> new_status_manager,
-    std::shared_ptr<FeatureProviderStatusManager> old_status_manager,
+    const std::shared_ptr<FeatureProviderStatusManager>& new_status_manager,
+    const std::shared_ptr<FeatureProviderStatusManager>& old_status_manager,
     const EvaluationContext& ctx) {
   if (new_status_manager->GetStatus() == ProviderStatus::kNotReady) {
     new_status_manager->Init(ctx);
   }
 
-  if (new_status_manager->GetStatus() == ProviderStatus::kReady) {
-    ShutdownOldProvider(old_status_manager);
-  }
+  ShutdownOldProvider(old_status_manager);
 }
 
 void ProviderRepository::ShutdownOldProvider(
-    std::shared_ptr<FeatureProviderStatusManager> old_status_manager) {
+    const std::shared_ptr<FeatureProviderStatusManager>& old_status_manager) {
   if (old_status_manager) {
     std::shared_lock<std::shared_mutex> lock(repo_mutex_);
 
@@ -221,12 +218,9 @@ bool ProviderRepository::IsStatusManagerRegistered(
   if (default_manager_ == manager) {
     return true;
   }
-  for (const auto& pair : provider_manager_) {
-    if (pair.second == manager) {
-      return true;
-    }
-  }
-  return false;
+  return std::any_of(
+      provider_manager_.begin(), provider_manager_.end(),
+      [&manager](const auto& pair) { return pair.second == manager; });
 }
 
 }  // namespace openfeature

openfeature/provider_repository.h

@@ -43,13 +43,13 @@ class ProviderRepository {
       std::string_view domain = "") const;
 
   // Set the default provider.
-  void SetProvider(std::shared_ptr<FeatureProvider> provider,
-                   const EvaluationContext& ctx, bool waitForInit);
+  void SetProvider(const std::shared_ptr<FeatureProvider>& provider,
+                   const EvaluationContext& ctx, bool wait_for_init);
 
   // Add a provider for a domain.
   void SetProvider(std::string_view domain,
-                   std::shared_ptr<FeatureProvider> provider,
-                   const EvaluationContext& ctx, bool waitForInit);
+                   const std::shared_ptr<FeatureProvider>& provider,
+                   const EvaluationContext& ctx, bool wait_for_init);
 
   // Fetch the status of a provider for a domain.
   // If the domain is not set, return the default provider status.
@@ -63,17 +63,17 @@ class ProviderRepository {
 
  private:
   void PrepareAndInitializeProvider(
-      const std::optional<std::string> domain,
-      std::shared_ptr<FeatureProvider> new_provider,
-      const EvaluationContext& ctx, bool waitForInit);
+      const std::optional<std::string>& domain,
+      const std::shared_ptr<FeatureProvider>& new_provider,
+      const EvaluationContext& ctx, bool wait_for_init);
 
   void InitializeProvider(
-      std::shared_ptr<FeatureProviderStatusManager> new_status_manager,
-      std::shared_ptr<FeatureProviderStatusManager> old_status_manager,
+      const std::shared_ptr<FeatureProviderStatusManager>& new_status_manager,
+      const std::shared_ptr<FeatureProviderStatusManager>& old_status_manager,
       const EvaluationContext& ctx);
 
   void ShutdownOldProvider(
-      std::shared_ptr<FeatureProviderStatusManager> old_status_manager);
+      const std::shared_ptr<FeatureProviderStatusManager>& old_status_manager);
 
   std::shared_ptr<FeatureProviderStatusManager>
   GetExistingStatusManagerForProvider(