Skip to content

Commit

Permalink
fix: provider switching should be threadsafe
Browse files Browse the repository at this point in the history
Signed-off-by: Manuel Schönlaub <[email protected]>
  • Loading branch information
mschoenlaub committed May 2, 2024
1 parent 341f069 commit f5dc47d
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 5 deletions.
13 changes: 8 additions & 5 deletions lib/open_feature/sdk/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Configuration
def initialize
@hooks = []
@providers = {}
@provider_mutex = Mutex.new
end

def provider(domain: nil)
Expand All @@ -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
Expand Down
16 changes: 16 additions & 0 deletions spec/open_feature/sdk/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
31 changes: 31 additions & 0 deletions spec/support/background_helper.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f5dc47d

Please sign in to comment.