From 20392d98a15dbf5c203e7c0292e5526c07b33927 Mon Sep 17 00:00:00 2001 From: Michael Granger Date: Tue, 5 Nov 2013 11:56:13 -0800 Subject: [PATCH] A better fix for #357. This is a better fix for several reasons: - It's more portable; the arguments/options to 'ps' vary widely across platforms. - Killing children by forking yet more children (i.e., via backtick) is problematic when you're trying to kill processes because of resource starvation. - Processes should not signal processes other than their own children, especially in a generic task-runner like Foreman. If the signal should propagate, then the sub-process should propagate signals to its children itself. There's no way to know what cleanup or preparation is necessary for a clean shutdown (e.g., SIGINT/SIGTERM), and in what order grandchild processes should be shut down to properly release locks, close files, etc. - foreman-runner doesn't (shouldn't?) create grandchild processes; the last thing it does is `exec` the program it's launching, which will replace the program of the *current* process with the one being started up. --- lib/foreman/engine.rb | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index f54d145..91015f6 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -176,19 +176,14 @@ class Foreman::Engine # # @param [String] signal The signal to send to each process # - def kill_children(signal="SIGTERM") - if Foreman.windows? - @running.each do |pid, (process, index)| - system "sending #{signal} to #{name_for(pid)} at pid #{pid}" - begin - Process.kill(signal, pid) - rescue Errno::ESRCH, Errno::EPERM - end - end - else + def kill_children( signal="SIGTERM" ) + @running.each do |pid, (process, index)| + system "sending #{signal} to #{name_for(pid)} at pid #{pid}" begin - Process.kill signal, *@running.keys unless @running.empty? - rescue Errno::ESRCH, Errno::EPERM + Process.kill( signal, pid ) + rescue Errno::ESRCH, Errno::EPERM => err + system " %p when sending signal %p to pid %d: %s" % + [ err.class, signal, pid, err.message ] end end end @@ -198,14 +193,7 @@ class Foreman::Engine # @param [String] signal The signal to send # def killall(signal="SIGTERM") - if Foreman.windows? - kill_children(signal) - else - begin - Process.kill "-#{signal}", Process.pid - rescue Errno::ESRCH, Errno::EPERM - end - end + kill_children(signal) end # Get the process formation