From 92427ed5db10065df314c0cc77b224d9fcf31965 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sun, 14 Jan 2024 18:28:25 -0500 Subject: [PATCH 1/3] shutdown outside of the handler because, i think but haven't totally verified, rufus will create mutexts sometimes which is not allowed in a signal handler --- example-app/README.md | 9 +++++++++ exe/clock | 1 + lib/ruby-clock.rb | 23 +++++++++++++++++++---- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/example-app/README.md b/example-app/README.md index a365c5a..587397c 100644 --- a/example-app/README.md +++ b/example-app/README.md @@ -7,3 +7,12 @@ To run: bundle bundle exec clock ``` + +To test invocation of existing signal handlers, put this code at the very top of exe/clock: + +```ruby +Signal.trap('INT') do + puts "This is a well-behaving INT handler from outside of ruby-clock" + exit +end +``` diff --git a/exe/clock b/exe/clock index 6ca230f..7e55fc5 100755 --- a/exe/clock +++ b/exe/clock @@ -4,6 +4,7 @@ require 'ruby-clock' require "ruby-clock/dsl" RubyClock.detect_and_load_rails_app require 'rufus_monkeypatch' +RubyClock.instance.listen_for_shutdown RubyClock.instance.listen_to_signals RubyClock.instance.prepare_rake RubyClock.instance.schedule.pause diff --git a/lib/ruby-clock.rb b/lib/ruby-clock.rb index d7ea7b1..43af89d 100644 --- a/lib/ruby-clock.rb +++ b/lib/ruby-clock.rb @@ -15,6 +15,7 @@ class RubyClock include RubyClock::AroundActions attr_accessor :on_error, :around_trigger_code_location + attr_accessor :should_shutdown, :old_shutdown_handler def initialize set_up_around_actions @@ -24,6 +25,22 @@ def wait_seconds ENV['RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS']&.to_i || 29 end + def listen_for_shutdown + Thread.new do + loop do + sleep 1 + if should_shutdown + shutdown + if old_shutdown_handler + old_shutdown_handler.call + else + exit + end + end + end + end + end + def shutdown puts "Shutting down ruby-clock. Waiting #{wait_seconds} seconds for jobs to finish..." schedule.shutdown(wait: wait_seconds) @@ -34,12 +51,10 @@ def listen_to_signals signals = %w[INT TERM] signals.each do |signal| old_handler = Signal.trap(signal) do - shutdown if old_handler.respond_to?(:call) - old_handler.call - else - exit + self.old_shutdown_handler = old_handler end + self.should_shutdown = true end end puts "RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS is set to #{wait_seconds}" From 526117bc8649f8c589fc1505a5183cdff21cc7ef Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sun, 14 Jan 2024 19:08:01 -0500 Subject: [PATCH 2/3] wip --- example-app/README.md | 8 +++++--- exe/clock | 1 - lib/ruby-clock.rb | 34 ++++++++++++++-------------------- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/example-app/README.md b/example-app/README.md index 587397c..2b16df8 100644 --- a/example-app/README.md +++ b/example-app/README.md @@ -11,8 +11,10 @@ bundle exec clock To test invocation of existing signal handlers, put this code at the very top of exe/clock: ```ruby -Signal.trap('INT') do - puts "This is a well-behaving INT handler from outside of ruby-clock" - exit +%w[INT TERM].each do |signal| + Signal.trap(signal) do + puts "This is a well-behaving #{signal} handler from outside of ruby-clock" + exit + end end ``` diff --git a/exe/clock b/exe/clock index 7e55fc5..6ca230f 100755 --- a/exe/clock +++ b/exe/clock @@ -4,7 +4,6 @@ require 'ruby-clock' require "ruby-clock/dsl" RubyClock.detect_and_load_rails_app require 'rufus_monkeypatch' -RubyClock.instance.listen_for_shutdown RubyClock.instance.listen_to_signals RubyClock.instance.prepare_rake RubyClock.instance.schedule.pause diff --git a/lib/ruby-clock.rb b/lib/ruby-clock.rb index 43af89d..09ba55e 100644 --- a/lib/ruby-clock.rb +++ b/lib/ruby-clock.rb @@ -15,7 +15,6 @@ class RubyClock include RubyClock::AroundActions attr_accessor :on_error, :around_trigger_code_location - attr_accessor :should_shutdown, :old_shutdown_handler def initialize set_up_around_actions @@ -25,36 +24,31 @@ def wait_seconds ENV['RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS']&.to_i || 29 end - def listen_for_shutdown + def shutdown(old_handler=nil) Thread.new do - loop do - sleep 1 - if should_shutdown - shutdown - if old_shutdown_handler - old_shutdown_handler.call - else - exit - end - end + sleep 0.1 # wait for trap block to exit + puts "Shutting down ruby-clock. Waiting #{wait_seconds} seconds for jobs to finish..." + schedule.shutdown(wait: wait_seconds) + puts "...done 🐈️ 👋" + if old_handler + puts "handing off shutdown to another signal handler..." + old_handler.call + else + exit end end end - def shutdown - puts "Shutting down ruby-clock. Waiting #{wait_seconds} seconds for jobs to finish..." - schedule.shutdown(wait: wait_seconds) - puts "...done 🐈️ 👋" - end - def listen_to_signals signals = %w[INT TERM] signals.each do |signal| old_handler = Signal.trap(signal) do if old_handler.respond_to?(:call) - self.old_shutdown_handler = old_handler + shutdown(old_handler) + else + shutdown end - self.should_shutdown = true + puts "received #{signal}" end end puts "RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS is set to #{wait_seconds}" From 83c1632ee30f34791bb5da08580a361627e011c7 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Mon, 15 Jan 2024 12:56:18 -0500 Subject: [PATCH 3/3] comments, and flushing stdout --- lib/ruby-clock.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/ruby-clock.rb b/lib/ruby-clock.rb index 09ba55e..e5ac92d 100644 --- a/lib/ruby-clock.rb +++ b/lib/ruby-clock.rb @@ -24,6 +24,9 @@ def wait_seconds ENV['RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS']&.to_i || 29 end + # shutdown is done async in a Thread because signal handlers should + # not have mutex locks created within them, which I believe can happen in + # the rufus shutdown process def shutdown(old_handler=nil) Thread.new do sleep 0.1 # wait for trap block to exit @@ -48,7 +51,10 @@ def listen_to_signals else shutdown end - puts "received #{signal}" + + # keep this line here at the end, to serve as some degree of demonstration that + # the handler exited before shutdown begins + puts("received #{signal}") && STDOUT.flush end end puts "RUBY_CLOCK_SHUTDOWN_WAIT_SECONDS is set to #{wait_seconds}"