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

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."

...

"  POSIX.1-1990  disallowed  setting  the  action  for SIGCHLD to
SIG_IGN.  POSIX.1-2001  allows this possibility, so that ignoring
SIGCHLD  can  be  used  to  prevent  the creation of zombies (see
wait(2)). Nevertheless, the historical BSD and System V behaviors
for ignoring SIGCHLD differ, so that the only completely portable
method of ensuring that terminated children do not become zombies
is to catch the SIGCHLD signal and perform a wait(2) or similar."


  So,  I'm  more inclined to using the same method as in dpid and
downloads.dpi ...

-- 
  Cheers
  Jorge.-