Re: Proposed Patch for Zombies Left by Dpi_start_dpid() in dpi.c

Hi Jorge,

On Wed, Aug 28, 2013 at 08:48:35PM -0400, Jorge Arellano Cid wrote:
> Hi,
> 
> On Wed, Aug 28, 2013 at 07:51:34PM +0200, Johannes Hofmann wrote:
> > Hi JG,
> > 
> > On Tue, Aug 27, 2013 at 03:15:14PM -0400, JG wrote:
> > > It seems that the dpid children forked by Dpi_start_dpid() in dpi.c
> > > are not reaped when they terminate themselves after a period of
> > > inactivity while the parent dillo process is still running.  The patch
> > > below is simply to reap these zombies.  The SIGCHLD handler should
> > > probably do more, but I am not yet familiar enough with Dillo to know
> > > what.
> > > 
> > > I believe this patch adheres to the Dillo rules of coding style, but
> > > am new to the list and a novice at Dillo patching.  And hence also
> > > apologies to Jorge for this duplication of a previous communication by
> > > private email (although with one small change to get the patch to
> > > compile under OpenBSD as well as Linux).
> > 
> > I can reproduce the issue here by killing dpid manually.
> > For me the following also fixes it:
> > 
> > diff -r 659dc205c377 src/dillo.cc
> > --- a/src/dillo.cc    Wed Aug 21 11:42:38 2013 +0200
> > +++ b/src/dillo.cc    Wed Aug 28 19:48:10 2013 +0200
> > @@ -329,6 +329,7 @@
> >  
> >     // Some OSes exit dillo without this (not GNU/Linux).
> >     signal(SIGPIPE, SIG_IGN);
> > +   signal(SIGCHLD, SIG_IGN);
> >  
> >     /* Handle command line options */
> >     opt_argv = dNew0(char*, numOptions(Options) + 1);
> > 
> > Can you please check whether it works for you too? It would have the
> > advantage that we have all signal related stuff in one place.
> 
>   Oh,  I'm looking at it now too! 
> For me this is an old TODO: with subtle issues lurking.
> 
> 
>   Somehow, setting SIG_IGN to a child rings me wrong...  :-)
> 
> 
>   After some digging (Manual page sigaction(2)):
> 
> ...
> 
> "A  child  created  via  fork(2)  inherits a copy of its parent's
> signal  dispositions.  During  an  execve(2), the dispositions of
> handled  signals  are  reset  to the default; the dispositions of
> ignored signals are left unchanged."

I guess this is no problem, as dpid explicitely adds a signal
handler for SIGCHLD again, but I'm happy if you take care of the
whole issue :-)

Cheers,
Johannes