From 212e05042687e484548d2e1e5c45641a652565bf Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 23 Jun 2026 12:35:19 +0000 Subject: [PATCH 1/9] fix race condition between status check and provider lookup Signed-off-by: NeaguGeorgiana23 --- openfeature/client_api.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/openfeature/client_api.h b/openfeature/client_api.h index b3c7e34..730896b 100644 --- a/openfeature/client_api.h +++ b/openfeature/client_api.h @@ -113,7 +113,16 @@ template ClientAPI::EvaluateFlag( ValueType default_value, const std::optional& ctx, ProviderCallable provider_call) { - ProviderStatus status = GetProviderStatus(); + std::shared_ptr manager = + provider_repository_.GetFeatureProviderStatusManager(domain_); + if (!manager) { + return std::make_unique( + default_value, Reason::kError, std::nullopt, FlagMetadata(), + ErrorCode::kProviderFatal, + "Provider status manager not found for domain"); + } + + ProviderStatus status = manager->GetStatus(); if (status == ProviderStatus::kNotReady) { return std::make_unique( default_value, Reason::kError, std::nullopt, FlagMetadata(), From c327b5cb05395174ffe7ea7015cb73b4e46bf1a6 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 23 Jun 2026 12:50:33 +0000 Subject: [PATCH 2/9] addressed CodeRabbit review Signed-off-by: NeaguGeorgiana23 --- openfeature/client_api.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openfeature/client_api.h b/openfeature/client_api.h index 730896b..50b7d78 100644 --- a/openfeature/client_api.h +++ b/openfeature/client_api.h @@ -134,8 +134,7 @@ std::unique_ptr ClientAPI::EvaluateFlag( ErrorCode::kProviderFatal, "Provider is in fatal error state"); } - std::shared_ptr provider = - provider_repository_.GetProvider(domain_); + std::shared_ptr provider = manager->GetProvider(); if (!provider) { return std::make_unique( default_value, Reason::kError, std::nullopt, FlagMetadata(), From 2be23bae86f57792dbc96f07bbd336976b060f9f Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Wed, 24 Jun 2026 09:26:59 +0000 Subject: [PATCH 3/9] apply gemini suggestions Signed-off-by: NeaguGeorgiana23 --- openfeature/client_api.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfeature/client_api.h b/openfeature/client_api.h index 50b7d78..3a9a0df 100644 --- a/openfeature/client_api.h +++ b/openfeature/client_api.h @@ -118,7 +118,7 @@ std::unique_ptr ClientAPI::EvaluateFlag( if (!manager) { return std::make_unique( default_value, Reason::kError, std::nullopt, FlagMetadata(), - ErrorCode::kProviderFatal, + ErrorCode::kGeneral, "Provider status manager not found for domain"); } From 18f55bea5912048197ed692ef8f6abf17c295448 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Wed, 24 Jun 2026 09:26:59 +0000 Subject: [PATCH 4/9] apply gemini suggestions Signed-off-by: NeaguGeorgiana23 --- openfeature/client_api.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openfeature/client_api.h b/openfeature/client_api.h index 50b7d78..d55e424 100644 --- a/openfeature/client_api.h +++ b/openfeature/client_api.h @@ -118,8 +118,7 @@ std::unique_ptr ClientAPI::EvaluateFlag( if (!manager) { return std::make_unique( default_value, Reason::kError, std::nullopt, FlagMetadata(), - ErrorCode::kProviderFatal, - "Provider status manager not found for domain"); + ErrorCode::kGeneral, "Provider status manager not found for domain"); } ProviderStatus status = manager->GetStatus(); From 134157b74094cf535b474d84e2100adebf60ecde Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Thu, 25 Jun 2026 08:12:52 +0000 Subject: [PATCH 5/9] add tests Signed-off-by: NeaguGeorgiana23 --- test/client_api_test.cpp | 86 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index 19f36f9..ce2bda9 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -3,8 +3,12 @@ #include #include +#include +#include +#include #include #include +#include #include "absl/status/status.h" #include "mocks/mock_feature_provider.h" @@ -381,3 +385,85 @@ TEST_F(ClientAPITest, EvaluateFlagProceedsWhenProviderInStaleState) { EXPECT_TRUE(client.GetBooleanValue("flag", false)); } + +TEST_F(ClientAPITest, ParallelProviderSwapRaceCondition) { + std::string domain = "race-domain"; + ClientAPI client(repo_, domain); + std::atomic evaluate{true}; + std::atomic running{true}; + + // Continuously evaluate flags when allowed. + std::thread evaluation_thread([&]() { + while (running) { + if (evaluate) { + client.GetBooleanValue("flag", false); + } else { + std::this_thread::sleep_for(std::chrono::microseconds(10)); + } + } + }); + + // Continuously swap providers. + std::thread swap_thread([&]() { + int iterations = 50; + for (int i = 0; i < iterations && running; ++i) { + auto not_ready_provider = + std::make_shared>(); + + auto init_called = std::make_shared>(); + auto proceed_init = std::make_shared>(); + auto init_finished = std::make_shared>(); + std::shared_future proceed_future = + proceed_init->get_future().share(); + + EXPECT_CALL(*not_ready_provider, Init(_)) + .WillOnce( + testing::Invoke([init_called, proceed_future, init_finished]( + const EvaluationContext&) -> absl::Status { + init_called->set_value(); + proceed_future.wait(); + init_finished->set_value(); + return absl::OkStatus(); + })); + + EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); + EXPECT_CALL(*not_ready_provider, Shutdown()) + .Times(testing::AtMost(1)) + .WillOnce(Return(absl::OkStatus())); + + repo_.SetProvider(domain, not_ready_provider, + EvaluationContext::Builder().build(), false); + + init_called->get_future().wait(); + + std::this_thread::sleep_for(std::chrono::microseconds(100)); + + evaluate = false; + std::this_thread::sleep_for(std::chrono::microseconds(10)); + + proceed_init->set_value(); + init_finished->get_future().wait(); + + auto ready_provider = std::make_shared>(); + ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); + ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) + .WillByDefault(testing::Invoke( + [](std::string_view, bool, const EvaluationContext&) + -> absl::StatusOr> { + return std::make_unique( + true, Reason::kTargetingMatch, std::nullopt, + FlagMetadata()); + })); + + repo_.SetProvider(domain, ready_provider, + EvaluationContext::Builder().build(), true); + evaluate = true; + std::this_thread::sleep_for(std::chrono::microseconds(100)); + } + running = false; + }); + + swap_thread.join(); + running = false; + evaluation_thread.join(); +} \ No newline at end of file From 8b8ba18dd85a69c1a09c5b73a78a4c54a31e75cd Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Thu, 25 Jun 2026 08:12:52 +0000 Subject: [PATCH 6/9] add tests Signed-off-by: NeaguGeorgiana23 --- test/client_api_test.cpp | 86 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index 19f36f9..939d073 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -3,8 +3,12 @@ #include #include +#include +#include +#include #include #include +#include #include "absl/status/status.h" #include "mocks/mock_feature_provider.h" @@ -381,3 +385,85 @@ TEST_F(ClientAPITest, EvaluateFlagProceedsWhenProviderInStaleState) { EXPECT_TRUE(client.GetBooleanValue("flag", false)); } + +TEST_F(ClientAPITest, ParallelProviderSwapRaceCondition) { + std::string domain = "race-domain"; + ClientAPI client(repo_, domain); + std::atomic evaluate{true}; + std::atomic running{true}; + + // Continuously evaluate flags when allowed. + std::thread evaluation_thread([&]() { + while (running) { + if (evaluate) { + client.GetBooleanValue("flag", false); + } else { + std::this_thread::sleep_for(std::chrono::microseconds(10)); + } + } + }); + + // Continuously swap providers. + std::thread swap_thread([&]() { + int iterations = 50; + for (int i = 0; i < iterations && running; ++i) { + auto not_ready_provider = + std::make_shared>(); + + auto init_called = std::make_shared>(); + auto proceed_init = std::make_shared>(); + auto init_finished = std::make_shared>(); + std::shared_future proceed_future = + proceed_init->get_future().share(); + + EXPECT_CALL(*not_ready_provider, Init(_)) + .WillOnce( + testing::Invoke([init_called, proceed_future, init_finished]( + const EvaluationContext&) -> absl::Status { + init_called->set_value(); + proceed_future.wait(); + init_finished->set_value(); + return absl::OkStatus(); + })); + + EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); + EXPECT_CALL(*not_ready_provider, Shutdown()) + .Times(testing::AtMost(1)) + .WillOnce(Return(absl::OkStatus())); + + repo_.SetProvider(domain, not_ready_provider, + EvaluationContext::Builder().build(), false); + + init_called->get_future().wait(); + + std::this_thread::sleep_for(std::chrono::microseconds(100)); + + evaluate = false; + std::this_thread::sleep_for(std::chrono::microseconds(10)); + + proceed_init->set_value(); + init_finished->get_future().wait(); + + auto ready_provider = std::make_shared>(); + ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); + ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) + .WillByDefault(testing::Invoke( + [](std::string_view, bool, const EvaluationContext&) + -> absl::StatusOr> { + return std::make_unique( + true, Reason::kTargetingMatch, std::nullopt, + FlagMetadata()); + })); + + repo_.SetProvider(domain, ready_provider, + EvaluationContext::Builder().build(), true); + evaluate = true; + std::this_thread::sleep_for(std::chrono::microseconds(100)); + } + running = false; + }); + + swap_thread.join(); + running = false; + evaluation_thread.join(); +} From bd7a6d67b3993ae516f8deacdd209a1b15dfd507 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Thu, 25 Jun 2026 08:20:44 +0000 Subject: [PATCH 7/9] fix linter Signed-off-by: NeaguGeorgiana23 --- test/client_api_test.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index c2fe6f9..939d073 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -3,16 +3,12 @@ #include #include -#include -#include -#include #include #include #include #include #include #include -#include #include "absl/status/status.h" #include "mocks/mock_feature_provider.h" From cbd0bc90125ea9a3cf4217645e145ffe50f26216 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Thu, 25 Jun 2026 08:32:02 +0000 Subject: [PATCH 8/9] fix test Signed-off-by: NeaguGeorgiana23 --- test/client_api_test.cpp | 106 ++++++++++++++------------------------- 1 file changed, 39 insertions(+), 67 deletions(-) diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index 939d073..4b12ded 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -389,81 +389,53 @@ TEST_F(ClientAPITest, EvaluateFlagProceedsWhenProviderInStaleState) { TEST_F(ClientAPITest, ParallelProviderSwapRaceCondition) { std::string domain = "race-domain"; ClientAPI client(repo_, domain); - std::atomic evaluate{true}; std::atomic running{true}; - // Continuously evaluate flags when allowed. + auto ready_provider = std::make_shared>(); + ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); + ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) + .WillByDefault(testing::Invoke( + [](std::string_view, bool, const EvaluationContext&) + -> absl::StatusOr> { + return std::make_unique( + true, Reason::kTargetingMatch, std::nullopt, FlagMetadata()); + })); + repo_.SetProvider(domain, ready_provider, + EvaluationContext::Builder().build(), true); + std::thread evaluation_thread([&]() { while (running) { - if (evaluate) { - client.GetBooleanValue("flag", false); - } else { - std::this_thread::sleep_for(std::chrono::microseconds(10)); - } + client.GetBooleanValue("flag", false); + std::this_thread::yield(); // Friendly to single-core CI runners } }); - // Continuously swap providers. - std::thread swap_thread([&]() { - int iterations = 50; - for (int i = 0; i < iterations && running; ++i) { - auto not_ready_provider = - std::make_shared>(); - - auto init_called = std::make_shared>(); - auto proceed_init = std::make_shared>(); - auto init_finished = std::make_shared>(); - std::shared_future proceed_future = - proceed_init->get_future().share(); - - EXPECT_CALL(*not_ready_provider, Init(_)) - .WillOnce( - testing::Invoke([init_called, proceed_future, init_finished]( - const EvaluationContext&) -> absl::Status { - init_called->set_value(); - proceed_future.wait(); - init_finished->set_value(); - return absl::OkStatus(); - })); - - EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); - EXPECT_CALL(*not_ready_provider, Shutdown()) - .Times(testing::AtMost(1)) - .WillOnce(Return(absl::OkStatus())); - - repo_.SetProvider(domain, not_ready_provider, - EvaluationContext::Builder().build(), false); - - init_called->get_future().wait(); - - std::this_thread::sleep_for(std::chrono::microseconds(100)); - - evaluate = false; - std::this_thread::sleep_for(std::chrono::microseconds(10)); - - proceed_init->set_value(); - init_finished->get_future().wait(); - - auto ready_provider = std::make_shared>(); - ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); - ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) - .WillByDefault(testing::Invoke( - [](std::string_view, bool, const EvaluationContext&) - -> absl::StatusOr> { - return std::make_unique( - true, Reason::kTargetingMatch, std::nullopt, - FlagMetadata()); - })); - - repo_.SetProvider(domain, ready_provider, - EvaluationContext::Builder().build(), true); - evaluate = true; - std::this_thread::sleep_for(std::chrono::microseconds(100)); - } - running = false; - }); + auto not_ready_provider = std::make_shared>(); + + auto init_called = std::make_shared>(); + auto proceed_init = std::make_shared>(); + std::shared_future proceed_future = proceed_init->get_future().share(); + + EXPECT_CALL(*not_ready_provider, Init(_)) + .WillOnce(testing::Invoke([init_called, proceed_future]( + const EvaluationContext&) -> absl::Status { + init_called->set_value(); + proceed_future.wait(); + return absl::OkStatus(); + })); + + EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); + EXPECT_CALL(*not_ready_provider, Shutdown()) + .Times(testing::AtMost(1)) + .WillOnce(Return(absl::OkStatus())); + + repo_.SetProvider(domain, not_ready_provider, + EvaluationContext::Builder().build(), false); + + init_called->get_future().wait(); - swap_thread.join(); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); running = false; evaluation_thread.join(); + proceed_init->set_value(); } From 0572daade099fa26624ce4a8be580476935ce1f0 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Thu, 25 Jun 2026 08:32:02 +0000 Subject: [PATCH 9/9] fix test Signed-off-by: NeaguGeorgiana23 --- test/client_api_test.cpp | 107 +++++++++++++++------------------------ 1 file changed, 40 insertions(+), 67 deletions(-) diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index 939d073..5008f36 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -46,6 +46,7 @@ class ClientAPITest : public ::testing::Test { }; constexpr int kUnknownExceptionError = 43; +constexpr int kSleepTimeMs = 10; // Test that the constructor correctly sets the domain in the metadata. TEST_F(ClientAPITest, ConstructorSetsDomainMetadata) { @@ -389,81 +390,53 @@ TEST_F(ClientAPITest, EvaluateFlagProceedsWhenProviderInStaleState) { TEST_F(ClientAPITest, ParallelProviderSwapRaceCondition) { std::string domain = "race-domain"; ClientAPI client(repo_, domain); - std::atomic evaluate{true}; std::atomic running{true}; - // Continuously evaluate flags when allowed. + auto ready_provider = std::make_shared>(); + ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); + ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) + .WillByDefault(testing::Invoke( + [](std::string_view, bool, const EvaluationContext&) + -> absl::StatusOr> { + return std::make_unique( + true, Reason::kTargetingMatch, std::nullopt, FlagMetadata()); + })); + repo_.SetProvider(domain, ready_provider, + EvaluationContext::Builder().build(), true); + std::thread evaluation_thread([&]() { while (running) { - if (evaluate) { - client.GetBooleanValue("flag", false); - } else { - std::this_thread::sleep_for(std::chrono::microseconds(10)); - } + client.GetBooleanValue("flag", false); + std::this_thread::yield(); // Friendly to single-core CI runners } }); - // Continuously swap providers. - std::thread swap_thread([&]() { - int iterations = 50; - for (int i = 0; i < iterations && running; ++i) { - auto not_ready_provider = - std::make_shared>(); - - auto init_called = std::make_shared>(); - auto proceed_init = std::make_shared>(); - auto init_finished = std::make_shared>(); - std::shared_future proceed_future = - proceed_init->get_future().share(); - - EXPECT_CALL(*not_ready_provider, Init(_)) - .WillOnce( - testing::Invoke([init_called, proceed_future, init_finished]( - const EvaluationContext&) -> absl::Status { - init_called->set_value(); - proceed_future.wait(); - init_finished->set_value(); - return absl::OkStatus(); - })); - - EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); - EXPECT_CALL(*not_ready_provider, Shutdown()) - .Times(testing::AtMost(1)) - .WillOnce(Return(absl::OkStatus())); - - repo_.SetProvider(domain, not_ready_provider, - EvaluationContext::Builder().build(), false); - - init_called->get_future().wait(); - - std::this_thread::sleep_for(std::chrono::microseconds(100)); - - evaluate = false; - std::this_thread::sleep_for(std::chrono::microseconds(10)); - - proceed_init->set_value(); - init_finished->get_future().wait(); - - auto ready_provider = std::make_shared>(); - ON_CALL(*ready_provider, Init(_)).WillByDefault(Return(absl::OkStatus())); - ON_CALL(*ready_provider, GetBooleanEvaluation(_, _, _)) - .WillByDefault(testing::Invoke( - [](std::string_view, bool, const EvaluationContext&) - -> absl::StatusOr> { - return std::make_unique( - true, Reason::kTargetingMatch, std::nullopt, - FlagMetadata()); - })); - - repo_.SetProvider(domain, ready_provider, - EvaluationContext::Builder().build(), true); - evaluate = true; - std::this_thread::sleep_for(std::chrono::microseconds(100)); - } - running = false; - }); + auto not_ready_provider = std::make_shared>(); + + auto init_called = std::make_shared>(); + auto proceed_init = std::make_shared>(); + std::shared_future proceed_future = proceed_init->get_future().share(); + + EXPECT_CALL(*not_ready_provider, Init(_)) + .WillOnce(testing::Invoke([init_called, proceed_future]( + const EvaluationContext&) -> absl::Status { + init_called->set_value(); + proceed_future.wait(); + return absl::OkStatus(); + })); + + EXPECT_CALL(*not_ready_provider, GetBooleanEvaluation(_, _, _)).Times(0); + EXPECT_CALL(*not_ready_provider, Shutdown()) + .Times(testing::AtMost(1)) + .WillOnce(Return(absl::OkStatus())); + + repo_.SetProvider(domain, not_ready_provider, + EvaluationContext::Builder().build(), false); + + init_called->get_future().wait(); - swap_thread.join(); + std::this_thread::sleep_for(std::chrono::milliseconds(kSleepTimeMs)); running = false; evaluation_thread.join(); + proceed_init->set_value(); }