diff --git a/lib/ldclient-rb/impl/data_source/status_provider.rb b/lib/ldclient-rb/impl/data_source/status_provider.rb index ec55cd36..9e6bdd26 100644 --- a/lib/ldclient-rb/impl/data_source/status_provider.rb +++ b/lib/ldclient-rb/impl/data_source/status_provider.rb @@ -2,6 +2,7 @@ require "concurrent" require "forwardable" +require "ldclient-rb/impl/util" require "ldclient-rb/interfaces" module LaunchDarkly @@ -19,18 +20,24 @@ class StatusProviderV2 extend Forwardable def_delegators :@status_broadcaster, :add_listener, :remove_listener + MONOTONIC_CLOCK = -> { Impl::Util.monotonic_seconds } + private_constant :MONOTONIC_CLOCK + # # Creates a new status provider. # # @param status_broadcaster [LaunchDarkly::Impl::Broadcaster] Broadcaster for status changes + # @param clock [#call] returns seconds on a monotonic clock; injectable for tests # - def initialize(status_broadcaster) + def initialize(status_broadcaster, clock: MONOTONIC_CLOCK) @status_broadcaster = status_broadcaster + @clock = clock @status = LaunchDarkly::Interfaces::DataSource::Status.new( LaunchDarkly::Interfaces::DataSource::Status::INITIALIZING, Time.now, nil ) + @state_since_monotonic = @clock.call @lock = Concurrent::ReadWriteLock.new end @@ -41,8 +48,29 @@ def status end end + # + # The current status together with the seconds the data source has spent + # in its current state, read atomically under one lock acquisition so the + # duration can never be paired with a stale state. + # + # The duration is measured on the monotonic clock, so a wall-clock step + # cannot distort it. `status.state_since` remains the wall-clock time + # reported to applications; this duration is the value to use when it + # decides behavior. + # + # @private + # @return [Array(LaunchDarkly::Interfaces::DataSource::Status, Float)] + # + def status_and_seconds_in_state + @lock.with_read_lock do + [@status, @clock.call - @state_since_monotonic] + end + end + # (see LaunchDarkly::Interfaces::DataSource::UpdateSink#update_status) def update_status(new_state, new_error) + return if new_state.nil? + status_to_broadcast = nil @lock.with_write_lock do @@ -62,7 +90,12 @@ def update_status(new_state, new_error) # No change if state is the same and no error return if new_state == old_status.state && new_error.nil? - new_since = new_state == old_status.state ? @status.state_since : Time.now + if new_state == old_status.state + new_since = @status.state_since + else + new_since = Time.now + @state_since_monotonic = @clock.call + end new_error = @status.last_error if new_error.nil? @status = LaunchDarkly::Interfaces::DataSource::Status.new( diff --git a/lib/ldclient-rb/impl/data_source/stream.rb b/lib/ldclient-rb/impl/data_source/stream.rb index c7b4cb2d..0a94ef25 100644 --- a/lib/ldclient-rb/impl/data_source/stream.rb +++ b/lib/ldclient-rb/impl/data_source/stream.rb @@ -32,6 +32,7 @@ def initialize(sdk_key, config, diagnostic_accumulator = nil) @stopped = Concurrent::AtomicBoolean.new(false) @ready = Concurrent::Event.new @connection_attempt_start_time = 0 + @connection_attempt_started_monotonic = nil end def initialized? @@ -184,13 +185,16 @@ def key_for_path(kind, path) def log_connection_started @connection_attempt_start_time = Impl::Util::current_time_millis + @connection_attempt_started_monotonic = Impl::Util.monotonic_seconds end def log_connection_result(is_success) - if !@diagnostic_accumulator.nil? && @connection_attempt_start_time > 0 - @diagnostic_accumulator.record_stream_init(@connection_attempt_start_time, !is_success, - Impl::Util::current_time_millis - @connection_attempt_start_time) - @connection_attempt_start_time = 0 + if !@diagnostic_accumulator.nil? && !@connection_attempt_started_monotonic.nil? + # The monotonic stamp is both the "attempt in flight" sentinel and the + # duration base; the wall-clock value is only the reported timestamp. + duration_millis = ((Impl::Util.monotonic_seconds - @connection_attempt_started_monotonic) * 1_000).to_i + @diagnostic_accumulator.record_stream_init(@connection_attempt_start_time, !is_success, duration_millis) + @connection_attempt_started_monotonic = nil end end end diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index 15994d7e..b83d2fb1 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -459,10 +459,12 @@ def consume_synchronizer_results(synchronizer, check_recovery: false) break if update == "quit" if update == "check" - # Check condition periodically - current_status = @data_source_status_provider.status - return SyncResult::RECOVER if check_recovery && recovery_condition(current_status) - return SyncResult::FALLBACK if fallback_condition(current_status) + # Check condition periodically. One atomic read pairs the status with + # its monotonic time-in-state, so a verdict can never pair a stale + # state with a fresh duration. + current_status, seconds_in_state = @data_source_status_provider.status_and_seconds_in_state + return SyncResult::RECOVER if check_recovery && recovery_condition(current_status, seconds_in_state) + return SyncResult::FALLBACK if fallback_condition(current_status, seconds_in_state) end next end @@ -514,13 +516,14 @@ def record_environment_id(environment_id) # Determine if we should fallback to the next synchronizer. # # @param status [LaunchDarkly::Interfaces::DataSource::Status] Current data source status + # @param seconds_in_state [Float] monotonic seconds spent in status.state # @return [Boolean] true if fallback condition is met # - def fallback_condition(status) + def fallback_condition(status, seconds_in_state) interrupted_at_runtime = status.state == LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED && - Time.now - status.state_since > 60 # 1 minute + seconds_in_state > 60 # 1 minute cannot_initialize = status.state == LaunchDarkly::Interfaces::DataSource::Status::INITIALIZING && - Time.now - status.state_since > 10 # 10 seconds + seconds_in_state > 10 # 10 seconds interrupted_at_runtime || cannot_initialize end @@ -529,11 +532,12 @@ def fallback_condition(status) # Determine if we should recover to the primary synchronizer. # # @param status [LaunchDarkly::Interfaces::DataSource::Status] Current data source status + # @param seconds_in_state [Float] monotonic seconds spent in status.state # @return [Boolean] true if recovery condition is met (healthy for too long) # - def recovery_condition(status) + def recovery_condition(status, seconds_in_state) status.state == LaunchDarkly::Interfaces::DataSource::Status::VALID && - Time.now - status.state_since > 300 # 5 minutes + seconds_in_state > 300 # 5 minutes end # diff --git a/lib/ldclient-rb/impl/data_system/streaming.rb b/lib/ldclient-rb/impl/data_system/streaming.rb index 78c74d63..765d35a1 100644 --- a/lib/ldclient-rb/impl/data_system/streaming.rb +++ b/lib/ldclient-rb/impl/data_system/streaming.rb @@ -47,6 +47,7 @@ def initialize(sdk_key, http_config, initial_reconnect_delay, config) @stopped = Concurrent::Event.new @diagnostic_accumulator = nil @connection_attempt_start_time = 0 + @connection_attempt_started_monotonic = nil end # @@ -118,7 +119,7 @@ def sync(ss) next unless update log_connection_result(true) - @connection_attempt_start_time = 0 + @connection_attempt_started_monotonic = nil yield update @@ -366,16 +367,18 @@ def stop private def log_connection_started @connection_attempt_start_time = Impl::Util.current_time_millis + @connection_attempt_started_monotonic = Impl::Util.monotonic_seconds end private def log_connection_result(is_success) return unless @diagnostic_accumulator - return unless @connection_attempt_start_time > 0 + return if @connection_attempt_started_monotonic.nil? - current_time = Impl::Util.current_time_millis - elapsed = current_time - @connection_attempt_start_time - @diagnostic_accumulator.record_stream_init(@connection_attempt_start_time, !is_success, elapsed >= 0 ? elapsed : 0) - @connection_attempt_start_time = 0 + # The monotonic stamp is both the "attempt in flight" sentinel and the + # duration base; the wall-clock value is only the reported timestamp. + elapsed = ((Impl::Util.monotonic_seconds - @connection_attempt_started_monotonic) * 1_000).to_i + @diagnostic_accumulator.record_stream_init(@connection_attempt_start_time, !is_success, elapsed) + @connection_attempt_started_monotonic = nil end end diff --git a/lib/ldclient-rb/impl/expiring_cache.rb b/lib/ldclient-rb/impl/expiring_cache.rb index 97ea87e1..a9d6b2cc 100644 --- a/lib/ldclient-rb/impl/expiring_cache.rb +++ b/lib/ldclient-rb/impl/expiring_cache.rb @@ -1,3 +1,4 @@ +require "ldclient-rb/impl/util" module LaunchDarkly module Impl @@ -7,10 +8,17 @@ module Impl # * made thread-safe # * removed many unused methods # * reading a key does not reset its expiration time, only writing + # * expiration is measured on a monotonic clock, so wall-clock steps cannot + # retain entries past their TTL or evict them early class ExpiringCache - def initialize(max_size, ttl) + MONOTONIC_CLOCK = -> { Impl::Util.monotonic_seconds } + private_constant :MONOTONIC_CLOCK + + # @param clock [#call] returns seconds on a monotonic clock; injectable for tests + def initialize(max_size, ttl, clock: MONOTONIC_CLOCK) @max_size = max_size @ttl = ttl + @clock = clock @data_lru = {} @data_ttl = {} @lock = Mutex.new @@ -31,7 +39,7 @@ def []=(key, val) @data_ttl.delete(key) @data_lru[key] = val - @data_ttl[key] = Time.now.to_f + @data_ttl[key] = @clock.call if @data_lru.size > @max_size key, _ = @data_lru.first # hashes have a FIFO ordering in Ruby @@ -63,7 +71,7 @@ def clear private def ttl_evict - ttl_horizon = Time.now.to_f - @ttl + ttl_horizon = @clock.call - @ttl key, time = @data_ttl.first until time.nil? || time > ttl_horizon diff --git a/lib/ldclient-rb/impl/migrations/migrator.rb b/lib/ldclient-rb/impl/migrations/migrator.rb index e680e25b..af49e270 100644 --- a/lib/ldclient-rb/impl/migrations/migrator.rb +++ b/lib/ldclient-rb/impl/migrations/migrator.rb @@ -270,7 +270,7 @@ def initialize(logger, origin, fn, tracker, measure_latency, measure_errors, pay # @return [LaunchDarkly::Migrations::OperationResult] # def run() - start = Time.now + start = Impl::Util.monotonic_seconds begin result = @fn.call(@payload) @@ -279,7 +279,7 @@ def run() result = LaunchDarkly::Result.fail("'#{origin}' operation raised an exception", e) end - @tracker.latency(@origin, (Time.now - start) * 1_000) if @measure_latency + @tracker.latency(@origin, (Impl::Util.monotonic_seconds - start) * 1_000) if @measure_latency @tracker.error(@origin) if @measure_errors && !result.success? @tracker.invoked(@origin) diff --git a/lib/ldclient-rb/impl/repeating_task.rb b/lib/ldclient-rb/impl/repeating_task.rb index f11e4dc2..50264a7a 100644 --- a/lib/ldclient-rb/impl/repeating_task.rb +++ b/lib/ldclient-rb/impl/repeating_task.rb @@ -39,13 +39,15 @@ def start @stop_event.wait(@start_delay) unless @start_delay.nil? || @start_delay == 0 until @stopped.value do - started_at = Time.now + started_at = Impl::Util.monotonic_seconds begin @task.call rescue => e Impl::Util.log_exception(@logger, "Uncaught exception from repeating task", e) end - delta = @interval - (Time.now - started_at) + # The run must be measured on the monotonic clock: a wall-clock step would + # make this wait negative (immediate re-run) or stretch it by the step size. + delta = @interval - (Impl::Util.monotonic_seconds - started_at) @stop_event.wait(delta) if delta > 0 end end diff --git a/lib/ldclient-rb/impl/util.rb b/lib/ldclient-rb/impl/util.rb index 6748295a..4e0e8f87 100644 --- a/lib/ldclient-rb/impl/util.rb +++ b/lib/ldclient-rb/impl/util.rb @@ -12,6 +12,17 @@ def self.current_time_millis (Time.now.to_f * 1000).to_i end + # + # Seconds on the monotonic clock. Use this for measuring intervals and + # durations; use the wall clock only for values reported outside the + # process. + # + # @return [Float] + # + def self.monotonic_seconds + Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) + end + def self.default_http_headers(sdk_key, config) ret = { "Authorization" => sdk_key, "User-Agent" => "RubyClient/" + LaunchDarkly::VERSION } diff --git a/spec/impl/data_source/status_provider_spec.rb b/spec/impl/data_source/status_provider_spec.rb new file mode 100644 index 00000000..97f22fb0 --- /dev/null +++ b/spec/impl/data_source/status_provider_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require "ldclient-rb/impl/data_source/status_provider" + +require "timecop" + +require "spec_helper" + +module LaunchDarkly + module Impl + module DataSource + describe StatusProviderV2 do + let(:broadcaster) { double("broadcaster", broadcast: nil) } + let(:valid) { LaunchDarkly::Interfaces::DataSource::Status::VALID } + let(:interrupted) { LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED } + let(:initializing) { LaunchDarkly::Interfaces::DataSource::Status::INITIALIZING } + + def make_error + LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( + LaunchDarkly::Interfaces::DataSource::ErrorInfo::UNKNOWN, 0, "boom", Time.now + ) + end + + it "keeps the public state_since as a wall-clock time" do + provider = StatusProviderV2.new(broadcaster) + provider.update_status(valid, nil) + + expect(provider.status.state_since).to be_a(Time) + end + + it "measures time in state on the monotonic clock and resets it on state change" do + now = 1_000.0 + provider = StatusProviderV2.new(broadcaster, clock: -> { now }) + + now = 1_030.0 + _, seconds = provider.status_and_seconds_in_state + expect(seconds).to eq 30.0 + + provider.update_status(valid, nil) + status, seconds = provider.status_and_seconds_in_state + expect(status.state).to eq valid + expect(seconds).to eq 0.0 + + now = 1_045.0 + _, seconds = provider.status_and_seconds_in_state + expect(seconds).to eq 15.0 + end + + it "does not reset the duration when the state is unchanged" do + now = 1_000.0 + provider = StatusProviderV2.new(broadcaster, clock: -> { now }) + provider.update_status(valid, nil) + since = provider.status.state_since + + now = 1_020.0 + provider.update_status(valid, make_error) + + now = 1_050.0 + status, seconds = provider.status_and_seconds_in_state + expect(status.state).to eq valid + expect(status.state_since).to eq since + expect(seconds).to eq 50.0 + end + + it "keeps both stamps when INTERRUPTED during INITIALIZING is coerced to INITIALIZING" do + now = 1_000.0 + provider = StatusProviderV2.new(broadcaster, clock: -> { now }) + since = provider.status.state_since + + now = 1_030.0 + provider.update_status(interrupted, make_error) + + status, seconds = provider.status_and_seconds_in_state + expect(status.state).to eq initializing + expect(status.state_since).to eq since + expect(seconds).to eq 30.0 + end + + it "keeps both stamps when a return to INITIALIZING is coerced to the current state" do + now = 1_000.0 + provider = StatusProviderV2.new(broadcaster, clock: -> { now }) + provider.update_status(valid, nil) + since = provider.status.state_since + + now = 1_030.0 + provider.update_status(initializing, make_error) + + status, seconds = provider.status_and_seconds_in_state + expect(status.state).to eq valid + expect(status.state_since).to eq since + expect(seconds).to eq 30.0 + end + + it "ignores a nil state" do + now = 1_000.0 + provider = StatusProviderV2.new(broadcaster, clock: -> { now }) + provider.update_status(valid, nil) + since = provider.status.state_since + + now = 1_020.0 + provider.update_status(nil, make_error) + + status, seconds = provider.status_and_seconds_in_state + expect(status.state).to eq valid + expect(status.state_since).to eq since + expect(seconds).to eq 20.0 + end + + it "is unaffected by wall-clock steps" do + provider = StatusProviderV2.new(broadcaster) + provider.update_status(valid, nil) + + Timecop.freeze(Time.now + 3600) do + _, seconds = provider.status_and_seconds_in_state + expect(seconds).to be < 60 + end + end + end + end + end +end diff --git a/spec/impl/data_source/stream_spec.rb b/spec/impl/data_source/stream_spec.rb index 12080697..11da2714 100644 --- a/spec/impl/data_source/stream_spec.rb +++ b/spec/impl/data_source/stream_spec.rb @@ -146,6 +146,23 @@ def with_connect_handler(processor) processor.send(:log_connection_result, true) end + it "measures duration on the monotonic clock, unaffected by wall-clock steps" do + diagnostic_accumulator = double("DiagnosticAccumulator") + + processor = subject.new("sdk_key", config, diagnostic_accumulator) + + expect(diagnostic_accumulator).to receive(:record_stream_init) do |_timestamp, _failed, duration| + # A backward wall-clock step used to produce a negative duration here. + expect(duration).to be >= 0 + expect(duration).to be < 1000 + end + + processor.send(:log_connection_started) + Timecop.freeze(Time.now - 120) do + processor.send(:log_connection_result, true) + end + end + it "only logs once per connection attempt" do diagnostic_accumulator = double("DiagnosticAccumulator") expect(diagnostic_accumulator).to receive(:record_stream_init).once diff --git a/spec/impl/data_system/fdv2_conditions_spec.rb b/spec/impl/data_system/fdv2_conditions_spec.rb new file mode 100644 index 00000000..67b312b1 --- /dev/null +++ b/spec/impl/data_system/fdv2_conditions_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "ldclient-rb" + +require "spec_helper" + +module LaunchDarkly + module Impl + module DataSystem + describe FDv2 do + # The condition predicates are pure functions of (status, seconds_in_state), + # so no construction is needed. + subject { described_class.allocate } + + def status(state) + LaunchDarkly::Interfaces::DataSource::Status.new(state, Time.now, nil) + end + + states = LaunchDarkly::Interfaces::DataSource::Status + + describe "fallback_condition" do + [ + [states::INTERRUPTED, 59, false], + [states::INTERRUPTED, 60, false], # strictly greater than + [states::INTERRUPTED, 61, true], + [states::INITIALIZING, 9, false], + [states::INITIALIZING, 10, false], # strictly greater than + [states::INITIALIZING, 11, true], + [states::VALID, 10_000, false], + [states::OFF, 10_000, false], + ].each do |state, seconds, expected| + it "is #{expected} for #{state} at #{seconds}s" do + expect(subject.send(:fallback_condition, status(state), seconds)).to be expected + end + end + end + + describe "recovery_condition" do + [ + [states::VALID, 299, false], + [states::VALID, 300, false], # strictly greater than + [states::VALID, 301, true], + [states::INTERRUPTED, 10_000, false], + [states::INITIALIZING, 10_000, false], + [states::OFF, 10_000, false], + ].each do |state, seconds, expected| + it "is #{expected} for #{state} at #{seconds}s" do + expect(subject.send(:recovery_condition, status(state), seconds)).to be expected + end + end + end + end + end + end +end diff --git a/spec/impl/data_system/streaming_synchronizer_spec.rb b/spec/impl/data_system/streaming_synchronizer_spec.rb index b1ff865a..25fd370f 100644 --- a/spec/impl/data_system/streaming_synchronizer_spec.rb +++ b/spec/impl/data_system/streaming_synchronizer_spec.rb @@ -422,6 +422,23 @@ def initialize(type, data = nil) synchronizer.send(:log_connection_result, true) end + it "measures duration on the monotonic clock, unaffected by wall-clock steps" do + diagnostic_accumulator = double("DiagnosticAccumulator") + + synchronizer.set_diagnostic_accumulator(diagnostic_accumulator) + + expect(diagnostic_accumulator).to receive(:record_stream_init) do |_timestamp, _failed, duration| + # A backward wall-clock step used to produce a negative duration here. + expect(duration).to be >= 0 + expect(duration).to be < 1000 + end + + synchronizer.send(:log_connection_started) + Timecop.freeze(Time.now - 120) do + synchronizer.send(:log_connection_result, true) + end + end + it "only logs once per connection attempt" do diagnostic_accumulator = double("DiagnosticAccumulator") expect(diagnostic_accumulator).to receive(:record_stream_init).once diff --git a/spec/impl/expiring_cache_spec.rb b/spec/impl/expiring_cache_spec.rb index c5bb51de..8f01eda1 100644 --- a/spec/impl/expiring_cache_spec.rb +++ b/spec/impl/expiring_cache_spec.rb @@ -6,20 +6,13 @@ module LaunchDarkly describe Impl::ExpiringCache do subject { Impl::ExpiringCache } - before(:each) do - Timecop.freeze(Time.now) - end - - after(:each) do - Timecop.return - end - it "evicts entries based on TTL" do - c = subject.new(3, 300) + now = 1000.0 + c = subject.new(3, 300, clock: -> { now }) c[:a] = 1 c[:b] = 2 - Timecop.freeze(Time.now + 330) + now += 330 c[:c] = 3 @@ -42,15 +35,16 @@ module LaunchDarkly end it "resets TTL on put" do - c = subject.new(3, 300) + now = 1000.0 + c = subject.new(3, 300, clock: -> { now }) c[:a] = 1 c[:b] = 2 - Timecop.freeze(Time.now + 250) + now += 250 c[:a] = 1.5 - Timecop.freeze(Time.now + 100) + now += 100 c[:c] = 3 @@ -86,6 +80,20 @@ module LaunchDarkly expect(c[:c]).to eq 3 expect(c[:d]).to eq 4 end + + it "is unaffected by wall-clock steps" do + c = subject.new(3, 300) + c[:a] = 1 + + # A forward wall step used to evict everything at once; a backward step + # used to retain entries past their TTL. The cache no longer reads the + # wall clock at all. + Timecop.freeze(Time.now + 3600) do + c[:b] = 2 + + expect(c[:a]).to eq 1 + expect(c[:b]).to eq 2 + end + end end end - diff --git a/spec/impl/migrations/migrator_spec.rb b/spec/impl/migrations/migrator_spec.rb index a8bb2595..0645e7eb 100644 --- a/spec/impl/migrations/migrator_spec.rb +++ b/spec/impl/migrations/migrator_spec.rb @@ -1,6 +1,8 @@ require 'ldclient-rb/interfaces' require 'ldclient-rb/impl/migrations/migrator' +require 'timecop' + require "events_test_util" require "mock_components" require "model_builders" @@ -700,6 +702,31 @@ def default_builder(client) end end end + + describe Executor do + it "measures latency on the monotonic clock, unaffected by wall-clock steps" do + recorded = nil + tracker = double("tracker") + allow(tracker).to receive(:latency) { |_origin, ms| recorded = ms } + allow(tracker).to receive(:invoked) + + fn = ->(_) { + Timecop.freeze(Time.now + 3600) + LaunchDarkly::Result.success(nil) + } + + executor = Executor.new($null_log, LaunchDarkly::Migrations::ORIGIN_OLD, fn, tracker, true, false, nil) + begin + executor.run + ensure + Timecop.return + end + + expect(recorded).to be >= 0 + # An hour of wall-clock step must not leak into the measured latency. + expect(recorded).to be < 1_000 + end + end end end end diff --git a/spec/impl/repeating_task_spec.rb b/spec/impl/repeating_task_spec.rb index a754103b..e11dda6f 100644 --- a/spec/impl/repeating_task_spec.rb +++ b/spec/impl/repeating_task_spec.rb @@ -1,6 +1,7 @@ require "ldclient-rb/impl/repeating_task" require "concurrent/atomics" +require "timecop" require "spec_helper" @@ -59,6 +60,34 @@ def null_logger expect(no_more_items).to be true end + it "schedules the next run from the monotonic clock, unaffected by wall-clock steps" do + runs = Queue.new + stepped = false + task = RepeatingTask.new(10, 0, + -> { + unless stepped + stepped = true + # Freeze before signaling the main thread. Cross-thread Timecop is + # safe here only because #stop joins the worker, so this freeze + # always completes before the ensure's Timecop.return below. + Timecop.freeze(Time.now + 3600) + end + runs << :ran + }, + null_logger, "test") + begin + task.start + expect(runs.pop).to eq :ran + # Before the monotonic fix, the forward wall step made the computed wait + # negative, so the next run fired immediately instead of after the interval. + sleep(0.3) + expect(runs).to be_empty + ensure + task.stop + Timecop.return + end + end + it "stops promptly when stopped during a long start delay" do ran = Concurrent::Event.new task = RepeatingTask.new(10, 10, -> { ran.set }, null_logger, "test") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 83f40de9..e6eef45d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ require "simplecov" if ENV['LD_ENABLE_CODE_COVERAGE'] == '1' require "ldclient-rb" +require "timecop" $null_log = ::Logger.new($stdout) $null_log.level = ::Logger::FATAL @@ -56,4 +57,9 @@ def update(status) end config.before(:each) do end + config.after(:each) do + # Safety net: a spec that leaks a frozen or travelled clock would silently + # corrupt the rest of a randomized run. + Timecop.return + end end