Applying the patch by wyvern from #662. This should hopefully kill the client hanging problems.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2604 980ebf18-57e1-0310-9a29-db15c13687c0

3 files changed, 55 insertions, 66 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ca5ca75a3..c8075536e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+    Applied the patch from #667 to hopefully kill the client hanging
+    problems (permanently, this time).
+
     Fixed functions so that they accept most other rvalues as valid values
     (#548).
 
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 7735bb2ec..51e4c6397 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -295,69 +295,52 @@ module Util
         @@os ||= Facter.value(:operatingsystem)
         output = nil
         child_pid, child_status = nil
+        # There are problems with read blocking with badly behaved children
+        # read.partialread doesn't seem to capture either stdout or stderr
+        # We hack around this using a temporary file
+
         # 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")
-                    $stdout.reopen("/dev/null")
-                    $stderr.reopen("/dev/null")
-                    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 child_pid; else
+        output_file="/dev/null"
+        if ! arguments[:squelch]
+            require "tempfile"
+            output_file = Tempfile.new("puppet")
+        end
+
+        child_pid = Kernel.fork
+        if child_pid
+            # Parent process executes this
+            child_status = Process.waitpid2(child_pid)[1]
         else
-            IO.popen("-") do |f|
-                if f
-                    # Parent process executes this
-                    output = f.read
+            # Child process executes this
+            begin
+                $stdin.reopen("/dev/null")
+                $stdout.reopen(output_file)
+                $stderr.reopen(output_file)
+                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
+                ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C'
+                if command.is_a?(Array)
+                    Kernel.exec(*command)
                 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
-								ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C'
-                        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
+                    Kernel.exec(command)
+                end
+            rescue => detail
+                puts detail.to_s
+                exit!(1)
+            end # begin; rescue
+        end # if child_pid
+ 	
+        # read output in if required
+        if ! arguments[:squelch]
+            output = output_file.open.read
+            output_file.delete
+        end
 
         if arguments[:failonfail]
             unless child_status == 0
diff --git a/test/util/utiltest.rb b/test/util/utiltest.rb
index 689bb7629..419d9820e 100755
--- a/test/util/utiltest.rb
+++ b/test/util/utiltest.rb
@@ -337,6 +337,13 @@ class TestPuppetUtil < Test::Unit::TestCase
       orig_lc_all = ENV["LC_ALL"]
       orig_lc_messages = ENV["LC_MESSAGES"]
       orig_language = ENV["LANGUAGE"]
+
+      cleanup do
+          ENV["LANG"] = orig_lang
+          ENV["LC_ALL"] = orig_lc_all
+          ENV["LC_MESSAGES"] = orig_lc_messages
+          ENV["LANGUAGE"] = orig_lc_messages
+      end
       
       # Mmm, we love gettext(3)
       ENV["LANG"] = "en_US"
@@ -345,17 +352,13 @@ class TestPuppetUtil < Test::Unit::TestCase
       ENV["LANGUAGE"] = "en_US"
 
       %w{LANG LC_ALL LC_MESSAGES LANGUAGE}.each do |env|
-        assert_equal 'C',
+        assert_equal('C',
                      Puppet::Util.execute(['ruby', '-e', "print ENV['#{env}']"]),
-                     "Environment var #{env} wasn't set to 'C'"
+                     "Environment var #{env} wasn't set to 'C'")
         
         assert_equal 'en_US', ENV[env], "Environment var #{env} not set back correctly"
       end
 
-      ENV["LANG"] = orig_lang
-      ENV["LC_ALL"] = orig_lc_all
-      ENV["LC_MESSAGES"] = orig_lc_messages
-      ENV["LANGUAGE"] = orig_lc_messages
     end
     
     # Check whether execute() accepts strings in addition to arrays.