From f5dc47d62acc74f2ed15e7a9e1fa04aa1328c177 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20Sch=C3=B6nlaub?= <manuel.schoenlaub@gmail.com>
Date: Thu, 2 May 2024 11:22:36 +0200
Subject: [PATCH] fix: provider switching should be threadsafe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Manuel Schönlaub <manuel.schoenlaub@gmail.com>
---
 lib/open_feature/sdk/configuration.rb       | 13 +++++----
 spec/open_feature/sdk/configuration_spec.rb | 16 +++++++++++
 spec/spec_helper.rb                         |  2 ++
 spec/support/background_helper.rb           | 31 +++++++++++++++++++++
 4 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 spec/support/background_helper.rb

diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb
index e782b74..321baa2 100644
--- a/lib/open_feature/sdk/configuration.rb
+++ b/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
diff --git a/spec/open_feature/sdk/configuration_spec.rb b/spec/open_feature/sdk/configuration_spec.rb
index 1c98b9b..73d1464 100644
--- a/spec/open_feature/sdk/configuration_spec.rb
+++ b/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
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 294b1f3..82c0c0c 100644
--- a/spec/spec_helper.rb
+++ b/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"
diff --git a/spec/support/background_helper.rb b/spec/support/background_helper.rb
new file mode 100644
index 0000000..dc417d9
--- /dev/null
+++ b/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