summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/mixlib/shellout/unix.rb33
-rw-r--r--spec/mixlib/shellout_spec.rb12
2 files changed, 14 insertions, 31 deletions
diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb
index 54f9fc3..dd22cbe 100644
--- a/lib/mixlib/shellout/unix.rb
+++ b/lib/mixlib/shellout/unix.rb
@@ -61,7 +61,7 @@ module Mixlib
return {} unless using_login?
entry = Etc.getpwuid(uid)
# According to `man su`, the set fields are:
- # $HOME, $SHELL, $USER, $LOGNAME, $PATH, and $IFS
+ # $HOME, $SHELL, $USER, $LOGNAME, $PATH, and $IFS
# Values are copied from "shadow" package in Ubuntu 14.10
{'HOME'=>entry.dir, 'SHELL'=>entry.shell, 'USER'=>entry.name, 'LOGNAME'=>entry.name, 'PATH'=>'/sbin:/bin:/usr/sbin:/usr/bin', 'IFS'=>"\t\n"}
end
@@ -104,7 +104,6 @@ module Mixlib
# CHEF-3390: Marshall.load on Ruby < 1.8.7p369 also has a GC bug related
# to Marshall.load, so try disabling GC first.
propagate_pre_exec_failure
- get_child_pgid
@status = nil
@result = nil
@@ -149,18 +148,6 @@ module Mixlib
private
- def get_child_pgid
- # The behavior of Process.getpgid (see also getpgid(2) ) when the
- # argument is the pid of a zombie isn't well specified. On Linux it
- # works, on OS X it returns ESRCH (which ruby turns into Errno::ESRCH).
- #
- # If the child dies very quickly, @child_pid may be a zombie, so handle
- # ESRCH here.
- @child_pgid = -Process.getpgid(@child_pid)
- rescue Errno::ESRCH, Errno::EPERM
- @child_pgid = nil
- end
-
def set_user
if user
Process.uid = uid
@@ -196,14 +183,10 @@ module Mixlib
Dir.chdir(cwd) if cwd
end
- # Process group id of the child. Returned as a negative value so you can
- # put it directly in arguments to kill, wait, etc.
- #
- # This may be nil if the child dies before the parent can query the
- # system for its pgid (on some systems it is an error to get the pgid of
- # a zombie).
+ # Since we call setsid the child_pgid will be the child_pid, set to negative here
+ # so it can be directly used in arguments to kill, wait, etc.
def child_pgid
- @child_pgid
+ -@child_pid
end
def initialize_ipc
@@ -337,10 +320,10 @@ module Mixlib
# support the "ONESHOT" optimization (where sh -c does exec without
# forking). To support cleaning up all the children, we need to
# ensure they're in a unique process group.
- # We cannot use setsid here since getpgid fails on AIX with EPERM
- # when parent and child have different sessions and the parent tries to get the process group,
- # hence we just create a new process group, and have the same session.
- Process.setpgrp
+ #
+ # We use setsid here to abandon our controlling tty and get a new session
+ # and process group that are set to the pid of the child process.
+ Process.setsid
configure_subprocess_file_descriptors
diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb
index 1765026..75d9821 100644
--- a/spec/mixlib/shellout_spec.rb
+++ b/spec/mixlib/shellout_spec.rb
@@ -1013,15 +1013,15 @@ describe Mixlib::ShellOut do
let(:cmd) { [ 'exit' ] }
it "handles ESRCH from getpgid of a zombie", :unix_only do
- Process.stub(:setpgrp) { exit!(4) }
+ Process.stub(:setsid) { exit!(4) }
- # there is a small race condition here if the child doesn't get
- # scheduled and call exit! before the parent can call getpgid, so run
- # this a few times to make sure we've created the reproduction case
- # correctly.
+ # we used to have race conditions if the child exited and zombied
+ # quickly which would cause an exception. we no longer call getpgrp()
+ # after setsid()/setpgrp() though so this race condition should no
+ # longer exist. still test 5 times for it though.
5.times do
s = Mixlib::ShellOut.new(cmd)
- s.run_command # should not raise Errno::ESRCH
+ s.run_command # should not raise Errno::ESRCH (or anything else)
end
end