Issue #3013 has been updated by Ricky Zhou.

Here is an attempt which should pass the SIGPIPE test by ignoring SIGPIPE in 
the child process.  This causes attempts to write to stdout/stderr to fail with 
EPIPE, but the program should be terminated as a result.  I think that is 
acceptable behavior, as something very similar happens when these badly behaved 
daemons are started on a terminal that is then closed (any attempts to write to 
the terminal will fail with EIO).

However, I'm still not totally happy with the solution - what if the daemon 
happens to have its own specialized SIGPIPE handler or resets the signal 
handler itself?  Basically, if we're accounting for absolutely any sort of 
software, including stuff which doesn't daemonize properly, still writes to 
stdout/stderr, and messes with signals, I can't think of any way to make them 
all work.

If you're rushing to get 0.25.3 out and working with RHEL4, I'm fine with 
reverting to the tempfile method and then looking deeper into other approaches 
later.

Anyway, here's the diff against d4e92861a5dca9ed7829b094eb13eaaedecebf46 on 
your ticket branch.  This method uses select and waitpid with WNOHANG to stop 
trying to read once the child exits.

<pre>
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 71d6a1f..4a2e260 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -266,6 +266,7 @@ module Util
         child_pid = Kernel.fork
         $VERBOSE = oldverb
         if child_pid
+            # Parent process executes this
             output_write.close
 
             # Read output in if required
@@ -274,7 +275,17 @@ module Util
                 begin
                     method = output_read.respond_to?(:readpartial) ? 
:readpartial : :sysread
                     begin
-                        output << output_read.send(method,4096)
+                        if select([output_read], nil, nil, 0.1)
+                            # Make sure that there is data to be read to avoid
+                            # indefinitely blocking reads.
+                            output << output_read.send(method, 4096)
+                        elsif Process.waitpid(child_pid, Process::WNOHANG)
+                            # Once the child has exited, immediately stop
+                            # attempting to read, even if EOF has not been
+                            # reached.
+                            child_status = $?.exitstatus
+                            break
+                        end
                     rescue Errno::EINTR
                         # Just popping out to handle an unrelated signal, but 
we're not done
                     end while true
@@ -285,9 +296,10 @@ module Util
                 end
             end
 
-            # Parent process executes this
-            Process.waitpid(child_pid)
-            child_status = $?.exitstatus
+            if ! child_status
+                Process.waitpid(child_pid)
+                child_status = $?.exitstatus
+            end
         else
             # Child process executes this
             Process.setsid
@@ -299,6 +311,13 @@ module Util
                     $stdout.reopen('/dev/null', 'w')
                     $stderr.reopen('/dev/null', 'w')
                 else
+                    # Ignore SIGPIPE.  This allows daemons which do not
+                    # properly close stdout/stderr to receive EPIPE on
+                    # attempting to write to the pipe instead of being
+                    # terminated with a SIGPIPE.  See ticket #1564,
+                    # comment 7 and ticket #3013 for details.
+                    trap('SIGPIPE', 'SIG_IGN')
+
                     $stdout.reopen(output_write)
                     if arguments[:combine]
                         $stderr.reopen(output_write)
</pre>
----------------------------------------
Bug #3013: util.rb:execute broken on Ruby <1.8.3
http://projects.reductivelabs.com/issues/3013

Author: Ricky Zhou
Status: Ready for Testing
Priority: Urgent
Assigned to: Markus Roberts
Category: exec
Target version: 0.25.3
Affected version: 0.25.2
Keywords: 
Branch: http://github.com/MarkusQ/puppet/tree/ticket/0.25.x/3013


Apparently the patch in ticket #2731 introduced one more issue by using 
readpartial, which isn't available until ruby 1.8.3 (RHEL4 at least is 
affected).  I'm not sure how this is normally handled in ruby, but if the 
readpartial function is not available, the code should fall back to sysread 
(along with some code for handling EINTR).

Anybody with better ruby knowledge know how this should be done?


-- 
You have received this notification because you have either subscribed to it, 
or are involved in it.
To change your notification preferences, please click here: 
http://reductivelabs.com/redmine/my/account
--
You received this message because you are subscribed to the Google Groups "Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.

Reply via email to