> just a note: it's quite essential that SIGPIPE is set to SIG_DFL in the
> child process.  luckily, execve(2) will reset to SIG_DFL unless it's set
> to SIG_IGN.  a child with SIGPIPE set unexpectedly to SIG_IGN can easily
> get stuck in endless loops.  the grandchild SHOULD die when it tries to
> write to the closed stdout.

Agreed in principe; in practice this branch (which is not being
included as it fails in real world use) was intended to expose/explore
the behavior of certain ill behaved processes in an effort to
determine what they were doing and how best to work with them.

> I guess this will work, but it seems a bit hackish compared to just
> catching SIGCHLD.  if done right, no polling (timeout) is needed.

More than a bit hackish, it evolved into a large, brittle kludge.
Just catching SIGCHLD (which could be beautifully done) is, as
explained on the tickets, woefully inadequate.

>>          ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] 
>> = 'C'
>>          Kernel.exec(*command)
>
> not part of your patch, but I couldn't resist commenting on this cargo
> cult code :-).  setting ENV['LC_ALL'] is sufficient.  'LANGUAGE' is not
> POSIX, and not consulted when LC_MESSAGES (effectively) is 'C'.

We got into this problem by cleaning up code that was "obviously"
unneeded and found to our dismay that there were environments were it
was not as superfluous as it appeared.  You may be perfectly correct
but I'm not going to compound the error by making the same mistake
again, pulling out tested, working code at the eleventh hour just
because we all agree it probably isn't needed.

-- Markus
-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" 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-dev?hl=en.


Reply via email to