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

Johannes Hofmann <[email protected]> writes:

> 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.
>
> Cheers,
> Johannes

Thank you: your solution is of course not only correct but far simpler
and better.

(Though I thought that in general sigaction() is to be preferred to
signal()?)