fix: Stop the store availability poller outside the lock

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

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