master 4cebd2d: Don't unblock SIGCHLD too early.

[Philipp Stephani]

branch: master
commit 4cebd2ded01f733957919cea9d10a67101cb7a67
Author: Philipp Stephani <phst@google.com>
Commit: Philipp Stephani <phst@google.com>

    Don't unblock SIGCHLD too early.
    
    We first need to register the received process ID so that
    'handle_child_signal' checks it.  Otherwise we might never call
    'waitpid' for these processes, risking deadlock.
    
    * src/callproc.c (call_process):
    * src/process.c (create_process): Don't unblock SIGCHLD before
    registering the process ID to wait for.
    
    * src/callproc.c (emacs_spawn): Accept a signal set from the caller.
---
 src/callproc.c | 31 ++++++++++++++++++++-----------
 src/lisp.h     |  4 ++--
 src/process.c  | 10 +++++++++-
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/callproc.c b/src/callproc.c
index 8d2a561..1da315b 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -314,6 +314,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int 
filefd,
 #ifdef MSDOS   /* Demacs 1.1.1 91/10/16 HIRANO Satoshi */
   char *tempfile = NULL;
 #else
+  sigset_t oldset;
   pid_t pid = -1;
 #endif
   int child_errno;
@@ -601,9 +602,12 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int 
filefd,
 
 #ifndef MSDOS
 
+  block_input ();
+  block_child_signal (&oldset);
+
   child_errno
     = emacs_spawn (&pid, filefd, fd_output, fd_error, new_argv, env,
-                   SSDATA (current_dir), NULL);
+                   SSDATA (current_dir), NULL, &oldset);
   eassert ((child_errno == 0) == (0 < pid));
 
   if (pid > 0)
@@ -624,6 +628,9 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int 
filefd,
        }
     }
 
+  unblock_child_signal (&oldset);
+  unblock_input ();
+
   if (pid < 0)
     report_file_errno (CHILD_SETUP_ERROR_DESC, Qnil, child_errno);
 
@@ -1227,17 +1234,21 @@ child_setup (int in, int out, int err, char **new_argv, 
char **env,
    process image file ARGV[0].  Use ENVP for the environment block for
    the new process.  Use CWD as working directory for the new process.
    If PTY is not NULL, it must be a pseudoterminal device.  If PTY is
-   NULL, don't perform any terminal setup.  */
+   NULL, don't perform any terminal setup.  OLDSET must be a pointer
+   to a signal set initialized by `block_child_signal'.  Before
+   calling this function, call `block_input' and `block_child_signal';
+   afterwards, call `unblock_input' and `unblock_child_signal'.  Be
+   sure to call `unblock_child_signal' only after registering NEWPID
+   in a list where `handle_child_signal' can find it!  */
 
 int
 emacs_spawn (pid_t *newpid, int std_in, int std_out, int std_err,
-             char **argv, char **envp, const char *cwd, const char *pty)
+             char **argv, char **envp, const char *cwd,
+             const char *pty, const sigset_t *oldset)
 {
-  sigset_t oldset;
   int pid;
 
-  block_input ();
-  block_child_signal (&oldset);
+  eassert (input_blocked_p ());
 
 #ifndef WINDOWSNT
   /* vfork, and prevent local vars from being clobbered by the vfork.  */
@@ -1249,6 +1260,7 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int 
std_err,
   int volatile stdout_volatile = std_out;
   int volatile stderr_volatile = std_err;
   char **volatile envp_volatile = envp;
+  const sigset_t *volatile oldset_volatile = oldset;
 
 #ifdef DARWIN_OS
   /* Darwin doesn't let us run setsid after a vfork, so use fork when
@@ -1270,6 +1282,7 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int 
std_err,
   std_out = stdout_volatile;
   std_err = stderr_volatile;
   envp = envp_volatile;
+  oldset = oldset_volatile;
 
   if (pid == 0)
 #endif /* not WINDOWSNT */
@@ -1364,7 +1377,7 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int 
std_err,
 #endif
 
       /* Stop blocking SIGCHLD in the child.  */
-      unblock_child_signal (&oldset);
+      unblock_child_signal (oldset);
 
       if (pty_flag)
        child_setup_tty (std_out);
@@ -1382,10 +1395,6 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int 
std_err,
 
   int vfork_error = pid < 0 ? errno : 0;
 
-  /* Stop blocking in the parent.  */
-  unblock_child_signal (&oldset);
-  unblock_input ();
-
   if (pid < 0)
     {
       eassert (0 < vfork_error);
diff --git a/src/lisp.h b/src/lisp.h
index ca0eb51..d139df9 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4495,8 +4495,8 @@ extern void setup_process_coding_systems (Lisp_Object);
 # define CHILD_SETUP_ERROR_DESC "Doing vfork"
 #endif
 
-extern int emacs_spawn (pid_t *, int, int, int, char **, char **, const char *,
-                       const char *);
+extern int emacs_spawn (pid_t *, int, int, int, char **, char **,
+                        const char *, const char *, const sigset_t *);
 extern char **make_environment_block (Lisp_Object);
 extern void init_callproc_1 (void);
 extern void init_callproc (void);
diff --git a/src/process.c b/src/process.c
index 06d750d..67e930e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2059,6 +2059,7 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
   bool pty_flag = 0;
   char pty_name[PTY_NAME_SIZE];
   Lisp_Object lisp_pty_name = Qnil;
+  sigset_t oldset;
 
   inchannel = outchannel = -1;
 
@@ -2139,13 +2140,16 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
   setup_process_coding_systems (process);
   char **env = make_environment_block (current_dir);
 
+  block_input ();
+  block_child_signal (&oldset);
+
   pty_flag = p->pty_flag;
   eassert (pty_flag == ! NILP (lisp_pty_name));
 
   vfork_errno
     = emacs_spawn (&pid, forkin, forkout, forkerr, new_argv, env,
                    SSDATA (current_dir),
-                   pty_flag ? SSDATA (lisp_pty_name) : NULL);
+                   pty_flag ? SSDATA (lisp_pty_name) : NULL, &oldset);
 
   eassert ((vfork_errno == 0) == (0 < pid));
 
@@ -2153,6 +2157,10 @@ create_process (Lisp_Object process, char **new_argv, 
Lisp_Object current_dir)
   if (pid >= 0)
     p->alive = 1;
 
+  /* Stop blocking in the parent.  */
+  unblock_child_signal (&oldset);
+  unblock_input ();
+
   /* Environment block no longer needed.  */
   unbind_to (count, Qnil);