http://reductivelabs.com/trac/puppet/changeset/2385
http://reductivelabs.com/trac/puppet/ticket/565
Index: test/util/utiltest.rb
===================================================================
--- test/util/utiltest.rb	(revision 2377)
+++ test/util/utiltest.rb	(revision 2385)
@@ -277,11 +277,15 @@
         # Now try it with a single quote
         assert_nothing_raised do
             output = Puppet::Util.execute([command, "yay'test", "funtest"])
-            # output = Puppet::Util.execute(command)
-            
         end
         assert_equal("yay'test\nfuntest\n", output)
         
+        # Now make sure we can squelch output (#565)
+        assert_nothing_raised do
+            output = Puppet::Util.execute([command, "yay'test", "funtest"], :squelch => true)
+        end
+        assert_equal(nil, output)
+
         # Now test that we correctly fail if the command returns non-zero
         assert_raise(Puppet::ExecutionFailure) do
             out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"])
@@ -289,7 +293,7 @@
         
         # And that we can tell it not to fail
         assert_nothing_raised() do
-            out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], false)
+            out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], :failonfail => false)
         end
         
         if Process.uid == 0
@@ -298,7 +302,7 @@
             group = nonrootgroup
             file = tempfile()
             assert_nothing_raised do
-                Puppet::Util.execute(["touch", file], true, user.name, group.name)
+                Puppet::Util.execute(["touch", file], :uid => user.name, :gid => group.name)
             end
             assert(FileTest.exists?(file), "file was not created")
             assert_equal(user.uid, File.stat(file).uid, "uid was not set correctly")
@@ -308,6 +312,24 @@
             # assert_equal(group.gid, File.stat(file).gid,
             #    "gid was not set correctly")
         end
+        
+        # (#565) Test the case of patricide.
+        patricidecommand = tempfile()
+        File.open(patricidecommand, "w") { |f|
+            f.puts %{#!/bin/bash\n/bin/bash -c 'kill -TERM \$PPID' &;\n while [ 1 ]; do echo -n ''; done;\n}
+        }
+        File.chmod(0755, patricidecommand)
+        assert_nothing_raised do
+            output = Puppet::Util.execute([patricidecommand], :squelch => true)
+        end
+        assert_equal(nil, output)
+        # See what happens if we try and read the pipe to the command...
+        assert_raise(Puppet::ExecutionFailure) do
+            output = Puppet::Util.execute([patricidecommand])
+        end
+        assert_nothing_raised do
+            output = Puppet::Util.execute([patricidecommand], :failonfail => false)
+        end
     end
     
     # Check whether execute() accepts strings in addition to arrays.
Index: lib/puppet/util.rb
===================================================================
--- lib/puppet/util.rb	(revision 2377)
+++ lib/puppet/util.rb	(revision 2385)
@@ -267,7 +267,8 @@
     end
 
     # Execute the desired command, and return the status and output.
-    def execute(command, failonfail = true, uid = nil, gid = nil)
+    # def execute(command, failonfail = true, uid = nil, gid = nil)
+    def execute(command, arguments = {:failonfail => true})
         if command.is_a?(Array)
             command = command.flatten.collect { |i| i.to_s }
             str = command.join(" ")
@@ -284,30 +285,35 @@
             Puppet.debug "Executing '%s'" % str
         end
         
-        if uid
-            uid = Puppet::Util::SUIDManager.convert_xid(:uid, uid)
+        if arguments[:uid]
+            arguments[:uid] = Puppet::Util::SUIDManager.convert_xid(:uid, arguments[:uid])
         end
-        if gid
-            gid = Puppet::Util::SUIDManager.convert_xid(:gid, gid)
+        if arguments[:gid]
+            arguments[:gid] = Puppet::Util::SUIDManager.convert_xid(:gid, arguments[:gid])
         end
         
         @@os ||= Facter.value(:operatingsystem)
         output = nil
-        IO.popen("-") do |f|
-            if f
-                output = f.read
+        child_pid, child_status = nil
+        # The idea here is to avoid IO#read whenever possible.
+        if arguments[:squelch]
+            child_pid = Kernel.fork
+            if child_pid
+                # Parent process executes this
+                child_status = Process.waitpid2(child_pid)[1]
             else
+                # Child process executes this
                 begin
                     $stdin.reopen("/dev/null")
-                    $stderr.close
-                    $stderr = $stdout.dup
-                    if gid
-                        Process.egid = gid
-                        Process.gid = gid unless @@os == "Darwin"
+                    $stdout.reopen("/dev/null")
+                    $stderr.reopen("/dev/null")
+                    if arguments[:gid]
+                        Process.egid = arguments[:gid]
+                        Process.gid = arguments[:gid] unless @@os == "Darwin"
                     end
-                    if uid
-                        Process.euid = uid
-                        Process.uid = uid unless @@os == "Darwin"
+                    if arguments[:uid]
+                        Process.euid = arguments[:uid]
+                        Process.uid = arguments[:uid] unless @@os == "Darwin"
                     end
                     if command.is_a?(Array)
                         Kernel.exec(*command)
@@ -317,13 +323,44 @@
                 rescue => detail
                     puts detail.to_s
                     exit!(1)
-                end
-            end
-        end
+                end # begin; rescue
+            end # if child_pid; else
+        else
+            IO.popen("-") do |f|
+                if f
+                    # Parent process executes this
+                    output = f.read
+                else
+                    # Parent process executes this
+                    begin
+                        $stdin.reopen("/dev/null")
+                        $stderr.close
+                        $stderr = $stdout.dup
+                        if arguments[:gid]
+                            Process.egid = arguments[:gid]
+                            Process.gid = arguments[:gid] unless @@os == "Darwin"
+                        end
+                        if arguments[:uid]
+                            Process.euid = arguments[:uid]
+                            Process.uid = arguments[:uid] unless @@os == "Darwin"
+                        end
+                        if command.is_a?(Array)
+                            Kernel.exec(*command)
+                        else
+                            Kernel.exec(command)
+                        end
+                    rescue => detail
+                        puts detail.to_s
+                        exit!(1)
+                    end # begin; rescue
+                end # if f; else
+            end # IO.popen do |f|
+            child_status = $?
+        end # if arguments[:squelch]; else
 
-        if failonfail
-            unless $? == 0
-                raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, $?.exitstatus, output]
+        if arguments[:failonfail]
+            unless child_status == 0
+                raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, child_status.inspect, output]
             end
         end
 
Index: lib/puppet/provider/service/base.rb
===================================================================
--- lib/puppet/provider/service/base.rb	(revision 2377)
+++ lib/puppet/provider/service/base.rb	(revision 2385)
@@ -114,12 +114,12 @@
     # A simple wrapper so execution failures are a bit more informative.
     def texecute(type, command, fof = true)
         begin
-            output = execute(command, fof)
+            # #565: Services generally produce no output, so squelch them.
+            execute(command, :failonfail => fof, :squelch => true)
         rescue Puppet::ExecutionFailure => detail
             @model.fail "Could not %s %s: %s" % [type, @model.ref, detail]
         end
-
-        return output
+        return nil
     end
 
     # Use either a specified command or the default for our provider.
