fix: deliver provider events in order, outside handler locks

lib/open_feature/sdk/configuration.rb

@@ -34,6 +34,7 @@ def initialize
         @provider_state_registry = ProviderStateRegistry.new
         @client_handlers = {}
         @client_handlers_mutex = Mutex.new
+        @dispatch_key = :"openfeature_dispatch_#{object_id}"
       end
 
       def add_hooks(*new_hooks)
@@ -60,6 +61,8 @@ def remove_handler(event_type, handler)
 
       # @api private
       def add_client_handler(client, event_type, handler)
+        @event_dispatcher.validate_handler!(event_type, handler)
+
         @client_handlers_mutex.synchronize do
           @client_handlers[client] ||= Hash.new { |h, k| h[k] = [] }
           @client_handlers[client][event_type] << handler
@@ -202,7 +205,7 @@ def set_provider_internal(provider, domain:, wait_for_init:)
           attempt.done.pop
           raise attempt.error || incomplete_init_error(provider, "initialization did not complete") unless attempt.state == :succeeded
 
-          dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY)
+          dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY, token: attempt)
         end
       ensure
         # Fail an attempt no init took over (e.g. a Timeout hit first), or it would stay running forever
@@ -215,7 +218,7 @@ def init_provider(provider, context, domain:, attempt:, raise_on_error: false)
 
         # 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)
+        dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY, token: attempt) 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)
@@ -228,8 +231,7 @@ def init_provider(provider, context, domain:, attempt:, raise_on_error: false)
         finish_init_attempt(attempt, :failed, error)
 
         dispatch_provider_event(provider, ProviderEvent::PROVIDER_ERROR,
-          error_code: error_code,
-          message: e.message)
+          {error_code: error_code, message: e.message}, token: attempt)
 
         raise error if raise_on_error
       ensure
@@ -260,25 +262,56 @@ def finish_init_attempt(attempt, state, error = nil)
         true
       end
 
-      def dispatch_provider_event(provider, event_type, details = {})
-        # Under the replacement lock, so a late event can't re-track a replaced or shut-down provider
-        @provider_mutex.synchronize do
-          return unless provider_bound?(provider)
+      # An event for a provider already dispatching on this call stack is queued behind the current one
+      def dispatch_provider_event(provider, event_type, details = {}, token: nil)
+        # The SDK's lifecycle events (with a token) record status when sent, so set_provider_and_wait returns
+        # with it even from the provider's own handler (1.1.2.4); provider events record it when delivered (5.3.5)
+        return if token && !accept_provider_event(provider, event_type, details, token)
 
-          @provider_state_registry.update_state_from_event(provider, event_type, details)
+        queues = (Thread.current[@dispatch_key] ||= {}.compare_by_identity)
+        if (queue = queues[provider])
+          queue << [event_type, details.dup, token]
+          return
         end
 
-        provider_name = extract_provider_name(provider)
+        queue = queues[provider] = [[event_type, details, token]]
+        begin
+          deliver_provider_event(provider, *queue.shift)
+          until queue.empty?
+            begin
+              deliver_provider_event(provider, *queue.shift)
+            rescue => e
+              # Its own emitter already returned, so log rather than fail the event that was dispatching
+              @logger&.error("Queued provider event failed: #{e.message}")
+            end
+          end
+        ensure
+          queues.delete(provider)
+          Thread.current[@dispatch_key] = nil if queues.empty?
+        end
+      end
 
-        event_details = {
-          provider_name: provider_name
-        }.merge(details)
+      def deliver_provider_event(provider, event_type, details, token)
+        return unless accept_provider_event(provider, event_type, details, token, record_status: token.nil?)
 
+        event_details = {provider_name: extract_provider_name(provider)}.merge(details)
         run_handlers_for_provider(provider, event_type, event_details)
       end
 
       private
 
+      # Under the replacement lock, so a late event can't re-track a replaced or shut-down provider
+      def accept_provider_event(provider, event_type, details, token, record_status: true)
+        @provider_mutex.synchronize do
+          return false unless provider_bound?(provider)
+          # An SDK READY/ERROR from an earlier registration of this provider is stale
+          return false unless token.nil? || @init_attempts[provider].equal?(token)
+
+          @provider_state_registry.update_state_from_event(provider, event_type, details) if record_status
+          true
+        end
+      end
+
       def provider_bound?(provider)
         @providers.each_value.any? { |p| p.equal?(provider) }
       end
@@ -325,22 +358,20 @@ def run_handlers_for_provider(provider, event_type, event_details)
         # Run global handlers (API-level, no domain filtering)
         @event_dispatcher.trigger_event(event_type, event_details)
 
-        # Run client handlers (domain-scoped)
-        @client_handlers_mutex.synchronize do
-          @client_handlers.each do |client, handlers_by_event|
+        # Run client handlers (domain-scoped) outside the lock, so they can add or remove handlers
+        handlers = @client_handlers_mutex.synchronize do
+          @client_handlers.flat_map do |client, handlers_by_event|
             # Check if this client should receive events from this provider
             client_provider = provider(domain: client.metadata.domain)
-            next unless client_provider&.equal?(provider)
-
-            # Trigger handlers for this client
-            handlers = handlers_by_event[event_type]
-            handlers.each do |handler|
-              handler.call(event_details)
-            rescue => e
-              @logger&.error("Client event handler failed: #{e.message}")
-            end
+            client_provider&.equal?(provider) ? handlers_by_event[event_type] : []
           end
         end
+
+        handlers.each do |handler|
+          handler.call(event_details)
+        rescue => e
+          @logger&.error("Client event handler failed: #{e.message}")
+        end
       end
 
       def run_immediate_handler(event_type, handler, client)
@@ -352,8 +383,8 @@ def run_immediate_handler(event_type, handler, client)
         }
 
         if client.nil?
-          # API-level handler: check all providers
-          @providers.each do |domain, provider|
+          # API-level handler: check each bound provider once, however many domains it's bound to
+          @providers.values.uniq(&:object_id).each do |provider|
             next unless provider
 
             provider_state = @provider_state_registry.get_state(provider)

lib/open_feature/sdk/event_dispatcher.rb

@@ -16,14 +16,18 @@ def initialize(logger = nil)
       end
 
       def add_handler(event_type, handler)
-        raise ArgumentError, "Invalid event type: #{event_type}" unless valid_event?(event_type)
-        raise ArgumentError, "Handler must respond to call" unless handler.respond_to?(:call)
+        validate_handler!(event_type, handler)
 
         @mutex.synchronize do
           @handlers[event_type] << handler
         end
       end
 
+      def validate_handler!(event_type, handler)
+        raise ArgumentError, "Invalid event type: #{event_type}" unless valid_event?(event_type)
+        raise ArgumentError, "Handler must respond to call" unless handler.respond_to?(:call)
+      end
+
       def remove_handler(event_type, handler)
         return unless valid_event?(event_type)
 

sig/open_feature/sdk/configuration.rbs

@@ -36,7 +36,9 @@ module OpenFeature
       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
+      def dispatch_provider_event: (untyped provider, String event_type, ?Hash[Symbol, untyped] details, ?token: untyped) -> void
+      def deliver_provider_event: (untyped provider, String event_type, Hash[Symbol, untyped]? details, untyped token) -> void
+      def accept_provider_event: (untyped provider, String event_type, Hash[Symbol, untyped]? details, untyped token, ?record_status: bool) -> bool
       def provider_bound?: (untyped provider) -> bool
       def domain_scoped?: (untyped provider) -> bool
       def validate_domain_scoped_binding!: (untyped provider, String? domain, bool scoped) -> void

sig/open_feature/sdk/event_dispatcher.rbs

@@ -6,6 +6,7 @@ module OpenFeature
       def initialize: (?untyped? logger) -> void
 
       def add_handler: (String event_type, _ToProc handler) -> void
+      def validate_handler!: (String event_type, untyped handler) -> void
       def remove_handler: (String event_type, _ToProc handler) -> void
       def remove_all_handlers: (String event_type) -> void
       def trigger_event: (String event_type, ?Hash[Symbol, untyped] event_details) -> void

spec/open_feature/sdk/configuration_spec.rb

@@ -408,6 +408,14 @@ def init(evaluation_context = nil, **opts) = @received = [evaluation_context, op
     end
   end
 
+  describe "nested event dispatch" do
+    it "leaves no per-thread state behind once dispatch finishes" do
+      provider = OpenFeature::SDK::Provider::InMemoryProvider.new
+
+      expect { configuration.set_provider_and_wait(provider) }.not_to change { Thread.current.keys.size }
+    end
+  end
+
   describe "event handler error logging" do
     it "logs error when event handler fails and logger is present" do
       provider = OpenFeature::SDK::Provider::NoOpProvider.new