fix: synchronize provider registration (#136)

Signed-off-by: Manuel Schönlaub <[email protected]>
Co-authored-by: Max VelDink <[email protected]>

Author: mschoenlaub

lib/open_feature/sdk/configuration.rb

@@ -16,6 +16,7 @@ class Configuration
       def initialize
         @hooks = []
         @providers = {}
+        @provider_mutex = Mutex.new
       end
 
       def provider(domain: nil)
@@ -27,11 +28,13 @@ def provider(domain: nil)
       #   2. On the new provider, call `init`.
       #   3. Finally, set the internal provider to the new provider
       def set_provider(provider, domain: nil)
-        @providers[domain].shutdown if @providers[domain].respond_to?(:shutdown)
-
-        provider.init if provider.respond_to?(:init)
-
-        @providers[domain] = provider
+        @provider_mutex.synchronize do
+          @providers[domain].shutdown if @providers[domain].respond_to?(:shutdown)
+          provider.init if provider.respond_to?(:init)
+          new_providers = @providers.dup
+          new_providers[domain] = provider
+          @providers = new_providers
+        end
       end
     end
   end

spec/open_feature/sdk/configuration_spec.rb

@@ -38,5 +38,21 @@
         expect(configuration.provider(domain: "testing")).to be(provider)
       end
     end
+
+    context "when the provider is set concurrently" do
+      let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new }
+      it "does not not call shutdown hooks multiple times if multithreaded" do
+        providers = (0..2).map { OpenFeature::SDK::Provider::NoOpProvider.new }
+        providers.each { |provider| expect(provider).to receive(:init) }
+        providers[0, 2].each { |provider| expect(provider).to receive(:shutdown) }
+        configuration.set_provider(providers[0])
+
+        allow(providers[0]).to receive(:shutdown).once { sleep 0.5 }
+        background { configuration.set_provider(providers[1]) }
+        background { configuration.set_provider(providers[2]) }
+        yield_to_background
+        expect(configuration.provider).to be(providers[2])
+      end
+    end
   end
 end

spec/spec_helper.rb

@@ -8,6 +8,8 @@
 
 require "debug"
 
+Dir["./spec/support/**/*.rb"].sort.each { |f| require f }
+
 RSpec.configure do |config|
   # Enable flags like --only-failures and --next-failure
   config.example_status_persistence_file_path = ".rspec_status"

spec/support/background_helper.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module BackgroundHelper
+  attr_writer :threads
+
+  private
+
+  def background(&)
+    thread = Thread.new(&)
+    thread.report_on_exception = false
+    threads << thread
+    thread.join(0.1)
+    thread
+  end
+
+  def threads
+    @threads ||= []
+  end
+
+  def yield_to_background
+    threads.each(&:join)
+  end
+end
+
+RSpec.configure do |config|
+  config.after do
+    threads.each(&:kill)
+    self.threads = []
+  end
+  config.include(BackgroundHelper)
+end