bug#12980: 24.3.50; Zombie process left after call-process

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 28 Nov 2012 18:30:08 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: hanche@math.ntnu.no, 12980@debbugs.gnu.org, 9488@debbugs.gnu.org
> 
> > The feature, whereby this function (call-process) could return a
> > description of errno, is hereby lost.  That's quite nasty in the MSDOS
> > case
> 
> That feature is supported only by MS-DOS, right?

Yes, I think so.

> On all other platforms an error is thrown.  Anyway, the new patch
> attempts to preserve the MS-DOS behavior.

Thanks.

> > Why is this eassert a good idea?  We don't need to enforce the Posix
> > spec at a price of crashing Emacs on users, do we?  What bad things
> > can happen if you see some other errno here?
> 
> It's not a question of enforcing Posix semantics.  It's a
> question of catching potentially serious internal
> programming errors in Emacs.  That part of the code has been
> completely rewritten, so the details of the comment is moot.
> However, the new code has an "eassert (errno == EINTR)" in
> the new get_child_status function.  I preceded that with a
> comment to try to help explain why the eassert is there.

Here's the comment:

> +      /* CHILD must be a child process that has not been reaped, and
> +	 STATUS and OPTIONS must be valid.  */
> +      eassert (errno == EINTR);

Are we sure that either CHILD will have exited at this point, or else
OPTIONS won't include WNOHANG?  Can this function be ever called if
neither of these conditions is true?

> > it would be nice to have somewhere a commentary with an
> > overview of how termination of child processes is handled.
> 
> I added some, before handle_child_signal.

Thanks.

> -  if (synch_process_termsig)
> +  if (WIFSIGNALED (status))
>      {
>        const char *signame;
>  
>        synchronize_system_messages_locale ();
> -      signame = strsignal (synch_process_termsig);
> +      signame = strsignal (WTERMSIG (status));
>  
>        if (signame == 0)
>  	signame = "unknown";
>  
> -      synch_process_death = signame;
> +      return code_convert_string_norecord (build_string (signame),
> +					   Vlocale_coding_system, 0);
>      }
>  
> -  if (synch_process_death)
> -    return code_convert_string_norecord (build_string (synch_process_death),
> -					 Vlocale_coding_system, 0);
> -  return make_number (synch_process_retcode);
> +  eassert (WIFEXITED (status));
> +  return make_number (WEXITSTATUS (status));

The use of WIF* macros here, in particular WIFSIGNALED and WTERMSIG,
will probably require more accurate definitions of these for Windows
(currently, they are quite naive and therefore wrong).  But that can
wait.

Otherwise, I see no problems related to Windows.  Thanks!