Skip to content

Honour TERM received during supervisor boot before forking workers - #770

Open
rafael-pissardo wants to merge 1 commit into
rails:mainfrom
rafael-pissardo:fix/755-term-during-supervisor-boot
Open

Honour TERM received during supervisor boot before forking workers#770
rafael-pissardo wants to merge 1 commit into
rails:mainfrom
rafael-pissardo:fix/755-term-during-supervisor-boot

Conversation

@rafael-pissardo

@rafael-pissardo rafael-pissardo commented Jul 27, 2026

Copy link
Copy Markdown

Summary

  • Drain the supervisor signal queue before each child is forked, and shut down immediately when a stop signal was already queued during boot/start hooks
  • Re-arm TERM/INT/QUIT handlers as the first thing in forked children so a forwarded TERM is not handled by the inherited enqueue-only supervisor trap
  • Add a regression test that sends TERM while an on_start hook is still running and asserts workers never start or claim jobs

Fixes #755.

Test plan

  • TARGET_DB=sqlite bundle exec ruby -Itest test/integration/supervisor_boot_signal_test.rb
  • TARGET_DB=sqlite bundle exec ruby -Itest test/integration/forked_processes_lifecycle_test.rb
  • TARGET_DB=sqlite bundle exec ruby -Itest test/integration/lifecycle_hooks_test.rb
  • TARGET_DB=sqlite bundle exec ruby -Itest test/unit/fork_supervisor_test.rb
  • RuboCop on changed files

Signals queued while start hooks run were only drained in the supervise
loop, after children had already been forked. Those children still held
the inherited enqueue-only trap, so a forwarded TERM was dropped and
workers kept claiming jobs until SIGKILL. Drain the queue before each
fork, skip supervise when already stopped, and re-arm child handlers
immediately after fork.
@rosa
rosa force-pushed the fix/755-term-during-supervisor-boot branch from 9dc4913 to e775233 Compare August 20, 2026 11:16
# forwarded by the supervisor right after fork is not handled by the
# inherited enqueue-only supervisor trap (see #755).
fork do
register_signal_handlers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels redundant together with the same call to register_signal_handlers right after running the before_boot callbacks... (see #boot, which runs first thing as part of block.call when running as fork). I think one of those needs to go.

def start_processes
configuration.configured_processes.each { |configured_process| start_process(configured_process) }
configuration.configured_processes.each do |configured_process|
# Honour signals that arrived during boot / start hooks before forking

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would run for any supervisor even if it doesn't fork (AsyncSupervisor), so I don't think it belongs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Forked worker misses forwarded TERM, keeps claiming jobs until SIGKILL

2 participants