fix: Stop the store availability poller outside the lock

[jsonbailey]

BEGIN_COMMIT_OVERRIDE
fix: Prevent close from hanging after a persistent store read fails
fix: Prevent the store availability poller from outliving the client
END_COMMIT_OVERRIDE

Symptom

LDClient#close never returns if a persistent store read has raised at any point
beforehand. The process hangs.

The store does not have to be broken at the time of the call. A single transient error
earlier in the process — a Redis blip, a DynamoDB throttle — is enough.

Cause

A failed store read calls update_availability(false), which starts a RepeatingTask
that polls the store every 0.5 seconds (store_client_wrapper.rb:101-106).

FeatureStoreClientWrapper#stop then took @mutex and called @poller.stop while still
holding it. RepeatingTask#stop waits for the worker thread (repeating_task.rb:66), and
that worker is in check_availability → update_availability(true), which needs the same
@mutex. The main thread waits for the worker; the worker waits for the mutex.

Confirmed with thread backtraces — both threads report status="sleep":

--- MAIN (calling stop)
    repeating_task.rb:66       Thread#join
    repeating_task.rb:66       RepeatingTask#stop
    store_client_wrapper.rb:59 block in FeatureStoreClientWrapper#stop
    store_client_wrapper.rb:56 Thread::Mutex#synchronize          <- holds @mutex
    store_client_wrapper.rb:56 FeatureStoreClientWrapper#stop

--- POLLER WORKER
    store_client_wrapper.rb:78  Thread::Mutex#synchronize         <- blocked on @mutex
    store_client_wrapper.rb:78  FeatureStoreClientWrapper#update_availability
    store_client_wrapper.rb:112 FeatureStoreClientWrapper#check_availability
    store_client_wrapper.rb:102 block in FeatureStoreClientWrapper#update_availability
    repeating_task.rb:44        block in RepeatingTask#start

The whole cycle is contained in one wrapper instance: the worker's stack bottoms out in the
earlier update_availability(false) call that created the task.

Impact beyond the hang

FDv1#stop never gets past @store_wrapper.stop (data_system/fdv1.rb:95), so
@shared_executor.shutdown does not run. Back in LDClient#close, neither
@event_processor.stop nor @big_segment_store_manager.stop runs, so buffered events are
lost as well.

Fix

Take the poller out of @mutex, then stop it outside the lock. The mutex still guards
@poller; it is released before the wait.

Applied in both places that stop the poller. The second one, in update_availability's
"available again" branch, runs on the poller's own thread, where RepeatingTask#stop
returns early on its @worker != Thread.current guard — so it does not deadlock today. It
is the same hazard from any other thread, and leaving one site in the old shape invites the
bug back.

One behavior note for review: this drops the return if @poller.nil? early-out in favour of
poller&.stop. The two are equivalent — in the original that return exited the whole
method, and here a nil poller no-ops and falls through to the same return.

Tests

spec/impl/store_client_wrapper_spec.rb gains "can stop while the availability poller is
running". It fails a timeout assertion rather than blocking, because a hang in CI is worse
than a failure.

Verified load-bearing: with only the source change reverted, the spec fails. Restored, it
passes.

rspec spec 1091 examples / 0 failures, rubocop clean.

Related

• SDK-3183

Python has the same structure but escapes the deadlock, because its RepeatingTask.stop()
signals without waiting. It pays for that with a poller that is never stopped at all.
Tracked separately as SDK-3182.

---

Note

Overview
Fixes LDClient#close hanging when a persistent store read had failed earlier: FeatureStoreClientWrapper#stop no longer calls RepeatingTask#stop while holding @mutex, avoiding deadlock with the availability poller thread in update_availability.

stop now sets @stopped, clears @poller under the lock, then stops the task outside the lock. The same extract-then-stop pattern applies when the store becomes available again. update_availability(false) skips starting a new poller if @stopped is already true, so late failing reads after shutdown cannot leak a background task.

Specs add coverage for stopping during an active poller (with a timeout so CI does not hang) and for not starting the poller after stop.

^{Reviewed by Cursor Bugbot for commit 40ded5d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 08c05e1. Configure here.}

[jsonbailey]

Good catch — the race is real and I have fixed it in 40ded5d. Two notes on scope, though, because the finding is narrower than it reads.

The leak is not introduced by this PR. There is no terminal state on the wrapper on main either: stop clears @poller but nothing prevents a later failure from creating a new one. The shortest path on unmodified main is simpler than the interleaving described — call close() while the store is healthy (no poller exists, so stop returns at the @poller.nil? check), then let any store read raise. A poller is created that nothing will ever stop.

I verified this by running the new regression spec against unmodified origin/main: it fails there too (expected: 0). So releasing the mutex earlier does not create the leak; it removes the deadlock that used to mask this particular interleaving of it.

The fix is a stopped flag, not a re-ordering. Re-taking the mutex around poller.stop would reintroduce the deadlock this PR exists to fix, and would still not stop a poller created after stop returns. Instead stop records that it ran, and the creation site refuses to start one:

poller = @mutex.synchronize do
  @stopped = true
  task = @poller
  @poller = nil
  task
end
poller&.stop

@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

That closes the described race and the pre-existing one, and makes stop idempotent.

Test: does not start the availability poller after stop asserts behaviourally — available? is only ever called by the poller, so a count of zero after a failed read proves none was started. Verified load-bearing: removing only the next if @stopped guard fails it.

rspec spec 1092 examples / 0 failures, rubocop clean.