From f3d097cb1cf14bffd7438a412590b4fbeb0afc39 Mon Sep 17 00:00:00 2001 From: "Ben Sheldon [he/him]" Date: Sun, 28 May 2023 22:20:34 -0700 Subject: [PATCH] Allow TimerTask to be safely restarted after shutdown and avoid duplicate tasks Creates a new @running object on shutdown while the ScheduledTask is checking the previously set @running object which will remain false --- lib/concurrent-ruby/concurrent/timer_task.rb | 8 +++++--- spec/concurrent/timer_task_spec.rb | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/concurrent-ruby/concurrent/timer_task.rb b/lib/concurrent-ruby/concurrent/timer_task.rb index fadb6140b..866282f6d 100644 --- a/lib/concurrent-ruby/concurrent/timer_task.rb +++ b/lib/concurrent-ruby/concurrent/timer_task.rb @@ -278,24 +278,26 @@ def ns_initialize(opts, &task) # @!visibility private def ns_shutdown_execution @running.make_false + @running = Concurrent::AtomicBoolean.new(false) super end # @!visibility private def ns_kill_execution @running.make_false + @running = Concurrent::AtomicBoolean.new(false) super end # @!visibility private def schedule_next_task(interval = execution_interval) - ScheduledTask.execute(interval, args: [Concurrent::Event.new], &method(:execute_task)) + ScheduledTask.execute(interval, args: [Concurrent::Event.new, @running], &method(:execute_task)) nil end # @!visibility private - def execute_task(completion) - return nil unless @running.true? + def execute_task(completion, continue_running) + return nil unless continue_running.true? _success, value, reason = @executor.execute(self) if completion.try? self.value = value diff --git a/spec/concurrent/timer_task_spec.rb b/spec/concurrent/timer_task_spec.rb index 44cc3e22e..f5086e1be 100644 --- a/spec/concurrent/timer_task_spec.rb +++ b/spec/concurrent/timer_task_spec.rb @@ -1,5 +1,6 @@ require_relative 'concern/dereferenceable_shared' require_relative 'concern/observable_shared' +require 'concurrent/atomic/atomic_fixnum' require 'concurrent/timer_task' module Concurrent @@ -94,12 +95,24 @@ def trigger_observable(observable) end context '#shutdown' do - it 'returns true on success' do task = TimerTask.execute(run_now: false) { nil } sleep(0.1) expect(task.shutdown).to be_truthy end + + it 'will cancel pre-shutdown task even if restarted to avoid double-runs' do + counter = Concurrent::AtomicFixnum.new(0) + task = TimerTask.execute(execution_interval: 0.2, run_now: true) { counter.increment } + sleep 0.05 + expect(counter.value).to eq 1 + + task.shutdown + task.execute + + sleep 0.25 + expect(counter.value).to eq 3 + end end end