diff --git a/lib/ldclient-rb/impl/store_client_wrapper.rb b/lib/ldclient-rb/impl/store_client_wrapper.rb index 92c64e72..7698b43f 100644 --- a/lib/ldclient-rb/impl/store_client_wrapper.rb +++ b/lib/ldclient-rb/impl/store_client_wrapper.rb @@ -23,6 +23,7 @@ def initialize(store, store_update_sink, logger) @mutex = Mutex.new # Covers the following variables @last_available = true + @stopped = false # @type [LaunchDarkly::Impl::RepeatingTask, nil] @poller = nil end @@ -53,12 +54,14 @@ def initialized? def stop @store.stop - @mutex.synchronize do - return if @poller.nil? - @poller.stop + poller = @mutex.synchronize do + @stopped = true + task = @poller @poller = nil + task end + poller&.stop end def monitoring_enabled? @@ -87,12 +90,12 @@ def monitoring_enabled? @store_update_sink.update_status(status) if available - @mutex.synchronize do - return if @poller.nil? - - @poller.stop + poller = @mutex.synchronize do + task = @poller @poller = nil + task end + poller&.stop return end @@ -102,6 +105,9 @@ def monitoring_enabled? task = Impl::RepeatingTask.new(0.5, 0, -> { self.check_availability }, @logger, 'LD/StoreWrapper#check_availability') @mutex.synchronize do + # A read can fail after stop, and a poller started then would never be stopped. + next if @stopped + @poller = task @poller.start end diff --git a/spec/impl/store_client_wrapper_spec.rb b/spec/impl/store_client_wrapper_spec.rb index b0086992..43ba7745 100644 --- a/spec/impl/store_client_wrapper_spec.rb +++ b/spec/impl/store_client_wrapper_spec.rb @@ -78,6 +78,69 @@ module Impl expect(statuses[1].available).to be true end end + + it "can stop while the availability poller is running" do + sink = double + store = double + checking = Concurrent::Event.new + + allow(store).to receive(:stop) + allow(store).to receive(:monitoring_enabled?).and_return(true) + allow(store).to receive(:all).and_raise(StandardError.new('read error')) + allow(sink).to receive(:update_status) + # Hold the poller's thread inside its availability check, so that stop has to wait + # for a thread that still needs the lock stop holds. + allow(store).to receive(:available?) do + checking.set + sleep 0.25 + true + end + + wrapper = FeatureStoreClientWrapper.new(store, sink, $null_log) + + begin + wrapper.all(:features) + raise "all should have raised exception" + rescue StandardError + # Ignored. The failed read starts the availability poller. + end + + expect(checking.wait(2)).to be true + + stopped = Concurrent::Event.new + Thread.new do + wrapper.stop + stopped.set + end + + expect(stopped.wait(5)).to be true + end + + it "does not start the availability poller after stop" do + sink = double + store = double + checks = Concurrent::AtomicFixnum.new(0) + + allow(store).to receive(:stop) + allow(store).to receive(:monitoring_enabled?).and_return(true) + allow(store).to receive(:all).and_raise(StandardError.new('read error')) + allow(sink).to receive(:update_status) + allow(store).to receive(:available?) { checks.increment; true } + + wrapper = FeatureStoreClientWrapper.new(store, sink, $null_log) + wrapper.stop + + begin + wrapper.all(:features) + raise "all should have raised exception" + rescue StandardError + # Ignored. On a running wrapper this would start the poller. + end + + # The poller is the only caller of available?, so it never ran. + sleep 1 + expect(checks.value).to eq 0 + end end end end