Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Run signal handler blocks in the event loop

This fixes the following bug:

  [MASTER 965] INFO: Reloading einhorn (.../vendor/bundle/ruby/1.9.1/bin/einhorn)...
  .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:183:in `[]=': can't add a new key into hash during iteration (RuntimeError)
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:183:in `block in reload'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:180:in `fork'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:180:in `reload'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:289:in `reload_for_preload_upgrade'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:281:in `full_upgrade'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command/interface.rb:166:in `block in install_handlers'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn.rb:12:in `call'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn.rb:12:in `state'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn.rb:18:in `method_missing'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:15:in `block in modern_workers_with_state'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:14:in `select'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:14:in `modern_workers_with_state'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:20:in `acked_modern_workers_with_state'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:32:in `acked_unsignaled_modern_workers'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/worker_pool.rb:49:in `ack_count'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn/command.rb:311:in `cull'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/lib/einhorn.rb:327:in `run'
     from .../vendor/bundle/ruby/1.9.1/bundler/gems/einhorn-27f5313b9627/bin/einhorn:304:in `<top (required)>'
  • Loading branch information...
commit 5ba91d0741e8aa41e2f351484a5837df7298aba6 1 parent d0a8ce2
Greg Brockman gdb authored
Showing with 33 additions and 7 deletions.
  1. +17 −7 lib/einhorn/command/interface.rb
  2. +16 −0 lib/einhorn/event.rb
24 lib/einhorn/command/interface.rb
View
@@ -147,25 +147,25 @@ def self.default_pidfile(cmd_name=nil)
## Signals
def self.install_handlers
- Signal.trap("INT") do
+ trap_async("INT") do
Einhorn::Command.signal_all("USR2", Einhorn::WorkerPool.workers)
Einhorn::Command.stop_respawning
end
- Signal.trap("TERM") do
+ trap_async("TERM") do
Einhorn::Command.signal_all("TERM", Einhorn::WorkerPool.workers)
Einhorn::Command.stop_respawning
end
# Note that quit is a bit different, in that it will actually
# make Einhorn quit without waiting for children to exit.
- Signal.trap("QUIT") do
+ trap_async("QUIT") do
Einhorn::Command.signal_all("QUIT", Einhorn::WorkerPool.workers)
Einhorn::Command.stop_respawning
exit(1)
end
- Signal.trap("HUP") {Einhorn::Command.reload}
- Signal.trap("ALRM") {Einhorn::Command.full_upgrade}
- Signal.trap("CHLD") {Einhorn::Event.break_loop}
- Signal.trap("USR2") do
+ trap_async("HUP") {Einhorn::Command.reload}
+ trap_async("ALRM") {Einhorn::Command.full_upgrade}
+ trap_async("CHLD") {}
+ trap_async("USR2") do
Einhorn::Command.signal_all("USR2", Einhorn::WorkerPool.workers)
Einhorn::Command.stop_respawning
end
@@ -177,6 +177,16 @@ def self.install_handlers
end
end
+ def self.trap_async(signal, &blk)
+ Signal.trap(signal) do
+ # We try to do as little work in the signal handler as
+ # possible. This avoids potential races between e.g. iteration
+ # and mutation.
+ Einhorn::Event.break_loop
+ Einhorn::Event.register_signal_action(&blk)
+ end
+ end
+
def self.remove_handlers
%w{INT TERM QUIT HUP ALRM CHLD USR2}.each do |signal|
Signal.trap(signal, "DEFAULT")
16 lib/einhorn/event.rb
View
@@ -4,6 +4,7 @@ module Einhorn
module Event
@@loopbreak_reader = nil
@@loopbreak_writer = nil
+ @@signal_actions = []
@@readable = {}
@@writeable = {}
@@timers = {}
@@ -47,6 +48,10 @@ def self.restore_persistent_descriptors(persistent_descriptors)
end
end
+ def self.register_signal_action(&blk)
+ @@signal_actions << blk
+ end
+
def self.register_readable(reader)
@@readable[reader.to_io] ||= Set.new
@@readable[reader.to_io] << reader
@@ -95,6 +100,7 @@ def self.deregister_timer(timer)
end
def self.loop_once
+ run_signal_actions
run_selectables
run_timers
end
@@ -108,6 +114,16 @@ def self.timeout
end
end
+ def self.run_signal_actions
+ # Note thah @@signal_actions can be mutated in the signal
+ # handlers. Since it's just an array we push to/shift from, we
+ # can be sure there's no race (such as adding hash keys during
+ # iteration.)
+ while blk = @@signal_actions.shift
+ blk.call
+ end
+ end
+
def self.run_selectables
time = timeout
Einhorn.log_debug("Loop timeout is #{time.inspect}")

1 comment on commit 5ba91d0

Nelson Elhage
Owner

I have some vague FUD around whether even using the array like that is signal-safe, but it's probably fine. lgtm, I think.

Please sign in to comment.
Something went wrong with that request. Please try again.