From 5e2f5656639de0e4500f03d175bf4f428d3b5011 Mon Sep 17 00:00:00 2001 From: Jose Colella Date: Thu, 22 Oct 2026 19:49:18 -0700 Subject: [PATCH 1/2] fix: release init waiters on replacement and same-thread re-entry - Replacing a provider whose init is still running marks that attempt failed and wakes anyone waiting on it, the same as shutdown does. - set_provider_and_wait called from inside the provider's own init (e.g. a handler of an event it emits) raises instead of waiting on itself forever. Signed-off-by: Jose Colella Co-Authored-By: Claude Opus 5.5 --- lib/open_feature/sdk/configuration.rb | 33 ++++++++++++------- sig/open_feature/sdk/configuration.rbs | 2 ++ .../specification/flag_evaluation_api_spec.rb | 28 ++++++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index fef6839d..b57def58 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -21,7 +21,7 @@ class Configuration attr_reader :logger # One per registration; +done+ is closed once init finishes, so waiters can pop on it - InitAttempt = Struct.new(:state, :error, :done) + InitAttempt = Struct.new(:state, :error, :done, :thread) private_constant :InitAttempt def initialize @@ -123,8 +123,7 @@ def reset @provider_state_registry.clear @provider_mutex.synchronize do @providers.clear - # Release anyone waiting on an init that shutdown cut short - @init_attempts.each_value { |attempt| attempt.done.close } + @init_attempts.each_value { |attempt| abandon_init_attempt(attempt) } @init_attempts.clear end @hooks.clear @@ -168,7 +167,7 @@ def set_provider_internal(provider, domain:, wait_for_init:) if old_provider && !old_provider.equal?(provider) needs_shutdown = !provider_bound?(old_provider) if needs_shutdown - @init_attempts.delete(old_provider) + abandon_init_attempt(@init_attempts.delete(old_provider)) @provider_state_registry.remove_provider(old_provider) old_provider.send(:detach) if old_provider.is_a?(Provider::EventEmitter) end @@ -194,15 +193,12 @@ def set_provider_internal(provider, domain:, wait_for_init:) end end elsif wait_for_init - # Already registered: wait for an init still in flight, then report its outcome + # Already registered: wait for an init still in flight, then report its outcome. + # From inside that init (e.g. a handler of an event it emits) the wait could never end. + raise incomplete_init_error(provider, "is still initializing on this thread") if attempt.state == :running && attempt.thread.equal?(Thread.current) + attempt.done.pop - unless attempt.state == :succeeded - raise attempt.error || ProviderInitializationError.new( - "Provider #{provider.class.name} initialization did not complete", - provider:, - error_code: Provider::ErrorCode::GENERAL - ) - end + raise attempt.error || incomplete_init_error(provider, "initialization did not complete") unless attempt.state == :succeeded dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) end @@ -212,6 +208,7 @@ def set_provider_internal(provider, domain:, wait_for_init:) end def init_provider(provider, context, domain:, attempt:, raise_on_error: false) + attempt.thread = Thread.current call_init(provider, context, domain) # Settle the attempt before handlers run, so a handler can register this provider again @@ -238,6 +235,18 @@ def init_provider(provider, context, domain:, attempt:, raise_on_error: false) finish_init_attempt(attempt, :failed) end + # Caller holds @provider_mutex: the provider was unbound or the API reset before its init settled + def abandon_init_attempt(attempt) + return unless attempt + + attempt.state = :failed if attempt.state == :running + attempt.done.close + end + + def incomplete_init_error(provider, reason) + ProviderInitializationError.new("Provider #{provider.class.name} #{reason}", provider:, error_code: Provider::ErrorCode::GENERAL) + end + def finish_init_attempt(attempt, state, error = nil) @provider_mutex.synchronize do return unless attempt.state == :running diff --git a/sig/open_feature/sdk/configuration.rbs b/sig/open_feature/sdk/configuration.rbs index 54154aca..ebb06d89 100644 --- a/sig/open_feature/sdk/configuration.rbs +++ b/sig/open_feature/sdk/configuration.rbs @@ -34,6 +34,8 @@ module OpenFeature def set_provider_internal: (untyped provider, domain: String?, wait_for_init: bool) -> void def init_provider: (untyped provider, EvaluationContext? context, domain: String?, attempt: untyped, ?raise_on_error: bool) -> void def finish_init_attempt: (untyped attempt, Symbol state, ?ProviderInitializationError? error) -> void + def abandon_init_attempt: (untyped attempt) -> void + def incomplete_init_error: (untyped provider, String reason) -> ProviderInitializationError def dispatch_provider_event: (untyped provider, String event_type, ?Hash[Symbol, untyped] details) -> void def provider_bound?: (untyped provider) -> bool def domain_scoped?: (untyped provider) -> bool diff --git a/spec/specification/flag_evaluation_api_spec.rb b/spec/specification/flag_evaluation_api_spec.rb index c2cc9588..a77373b8 100644 --- a/spec/specification/flag_evaluation_api_spec.rb +++ b/spec/specification/flag_evaluation_api_spec.rb @@ -370,6 +370,34 @@ def start_waiter(domain) ensure outcomes << :ok end + + specify "replacing a provider releases a waiter on its unfinished init" do + OpenFeature::SDK.set_provider(provider, domain: "a") + waiter = start_waiter("a") + + OpenFeature::SDK.set_provider(OpenFeature::SDK::Provider::NoOpProvider.new, domain: "a") + + expect(waiter.join(2)&.value).to be_a(OpenFeature::SDK::ProviderInitializationError) + ensure + outcomes << :ok + end + + specify "set_provider_and_wait from inside the provider's own init raises instead of deadlocking" do + emitting = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + def init(_evaluation_context = nil) = emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_STALE) + end.new + reentry = nil + OpenFeature::SDK.add_handler(OpenFeature::SDK::ProviderEvent::PROVIDER_STALE, lambda do |_| + OpenFeature::SDK.set_provider_and_wait(emitting) + rescue OpenFeature::SDK::ProviderInitializationError => e + reentry = e + end) + + setter = background { OpenFeature::SDK.set_provider_and_wait(emitting) } + + expect(setter.join(2)).to be(setter) + expect(reentry).to be_a(OpenFeature::SDK::ProviderInitializationError) + end end context "Requirement 1.1.2.4" do From 2099adf2b4d5c61eb9af2bab1b3f70c6be1757e3 Mon Sep 17 00:00:00 2001 From: Jose Colella Date: Fri, 23 Oct 2026 09:17:37 -0700 Subject: [PATCH 2/2] fix: fail set_provider_and_wait when its provider is replaced mid-init If another thread replaces or shuts down the provider while set_provider_and_wait is running its init, the call now raises ProviderInitializationError, matching what waiters on the same attempt get. The abandoned attempt also no longer dispatches READY. Signed-off-by: Jose Colella Co-Authored-By: Claude Opus 5.5 --- lib/open_feature/sdk/configuration.rb | 11 +++++++---- sig/open_feature/sdk/configuration.rbs | 2 +- spec/specification/flag_evaluation_api_spec.rb | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index b57def58..a48be5b6 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -187,6 +187,8 @@ def set_provider_internal(provider, domain:, wait_for_init:) if needs_init if wait_for_init init_provider(provider, context_for_init, domain:, attempt:, raise_on_error: true) + # Replaced or shut down while init ran: fail like the waiters on this attempt did + raise incomplete_init_error(provider, "initialization did not complete") unless attempt.state == :succeeded else init_thread = Thread.new do init_provider(provider, context_for_init, domain:, attempt:, raise_on_error: false) @@ -211,9 +213,9 @@ def init_provider(provider, context, domain:, attempt:, raise_on_error: false) attempt.thread = Thread.current call_init(provider, context, domain) - # Settle the attempt before handlers run, so a handler can register this provider again - finish_init_attempt(attempt, :succeeded) - dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) + # Settle the attempt before handlers run, so a handler can register this provider again. + # An abandoned attempt (provider replaced or shut down meanwhile) reports no READY. + dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) if finish_init_attempt(attempt, :succeeded) rescue => e # Spec 1.7.5: PROVIDER_FATAL from init makes the provider FATAL error_code = EvaluationError.code_for(e) @@ -249,12 +251,13 @@ def incomplete_init_error(provider, reason) def finish_init_attempt(attempt, state, error = nil) @provider_mutex.synchronize do - return unless attempt.state == :running + return false unless attempt.state == :running attempt.state = state attempt.error = error end attempt.done.close + true end def dispatch_provider_event(provider, event_type, details = {}) diff --git a/sig/open_feature/sdk/configuration.rbs b/sig/open_feature/sdk/configuration.rbs index ebb06d89..19dceb4d 100644 --- a/sig/open_feature/sdk/configuration.rbs +++ b/sig/open_feature/sdk/configuration.rbs @@ -33,7 +33,7 @@ module OpenFeature def reset: () -> void def set_provider_internal: (untyped provider, domain: String?, wait_for_init: bool) -> void def init_provider: (untyped provider, EvaluationContext? context, domain: String?, attempt: untyped, ?raise_on_error: bool) -> void - def finish_init_attempt: (untyped attempt, Symbol state, ?ProviderInitializationError? error) -> void + def finish_init_attempt: (untyped attempt, Symbol state, ?ProviderInitializationError? error) -> bool def abandon_init_attempt: (untyped attempt) -> void def incomplete_init_error: (untyped provider, String reason) -> ProviderInitializationError def dispatch_provider_event: (untyped provider, String event_type, ?Hash[Symbol, untyped] details) -> void diff --git a/spec/specification/flag_evaluation_api_spec.rb b/spec/specification/flag_evaluation_api_spec.rb index a77373b8..dcbfc278 100644 --- a/spec/specification/flag_evaluation_api_spec.rb +++ b/spec/specification/flag_evaluation_api_spec.rb @@ -382,6 +382,24 @@ def start_waiter(domain) outcomes << :ok end + specify "replacing a provider while set_provider_and_wait runs its init makes that call raise too" do + initiator = background do + OpenFeature::SDK.set_provider_and_wait(provider, domain: "a") + rescue OpenFeature::SDK::ProviderInitializationError => e + e + end + 200.times do + break if outcomes.num_waiting.positive? + sleep 0.01 + end + + OpenFeature::SDK.set_provider(OpenFeature::SDK::Provider::NoOpProvider.new, domain: "a") + outcomes << :ok + + expect(initiator.join(2)&.value).to be_a(OpenFeature::SDK::ProviderInitializationError) + expect(OpenFeature::SDK.configuration.provider_tracked?(provider)).to be(false) + end + specify "set_provider_and_wait from inside the provider's own init raises instead of deadlocking" do emitting = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do def init(_evaluation_context = nil) = emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_STALE)