master 1773679: Ensure that sentinels are called during 'accept-process-output'.

[Philipp Stephani]

branch: master
commit 1773679af3241919a85d6174b1554070a63cca79
Author: Philipp Stephani <phst@google.com>
Commit: Philipp Stephani <phst@google.com>

    Ensure that sentinels are called during 'accept-process-output'.
    
    When we're trying to notify a process about a status change, we need
    to ignore the SIGCHLD pipe temporarily, otherwise the code would
    likely not run into the timeout case that's necessary for a status
    change to happen.
    
    * src/process.c (wait_reading_process_output): Ignore the SIGCHLD pipe
    when notifying a process about a status change.
    * test/src/process-tests.el (process-tests/sentinel-called)
    (process-tests/sentinel-with-multiple-processes): New unit tests.
---
 src/process.c             |  9 ++++++++
 test/src/process-tests.el | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/src/process.c b/src/process.c
index aca87f8..09f8790 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5323,6 +5323,15 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
             compute_input_wait_mask (&Atemp);
          compute_write_mask (&Ctemp);
 
+         /* If a process status has changed, the child signal pipe
+            will likely be readable.  We want to ignore it for now,
+            because otherwise we wouldn't run into a timeout
+            below.  */
+         int fd = child_signal_read_fd;
+         eassert (fd < FD_SETSIZE);
+         if (0 <= fd)
+           FD_CLR (fd, &Atemp);
+
          timeout = make_timespec (0, 0);
          if ((thread_select (pselect, max_desc + 1,
                              &Atemp,
diff --git a/test/src/process-tests.el b/test/src/process-tests.el
index dad3642..d2a98dc 100644
--- a/test/src/process-tests.el
+++ b/test/src/process-tests.el
@@ -734,5 +734,58 @@ Return nil if that can't be determined."
                 (match-string-no-properties 1))))))
   process-tests--EMFILE-message)
 
+(ert-deftest process-tests/sentinel-called ()
+  "Check that sentinels are called after processes finish"
+  (let ((echo (executable-find "echo")))
+    (skip-unless echo)
+    (dolist (conn-type '(pipe pty))
+      (ert-info ((format "Connection type: %s" conn-type))
+        (process-tests--with-processes processes
+          (let* ((calls ())
+                 (process (make-process
+                           :name "echo"
+                           :command (list echo "first")
+                           :noquery t
+                           :connection-type conn-type
+                           :coding 'utf-8-unix
+                           :sentinel (lambda (process message)
+                                       (push (list process message)
+                                             calls)))))
+            (push process processes)
+            (while (accept-process-output process))
+            (should (equal calls
+                           (list (list process "finished\n"))))))))))
+
+(ert-deftest process-tests/sentinel-with-multiple-processes ()
+  "Check that sentinels are called in time even when other processes
+have written output."
+  (let ((echo (executable-find "echo"))
+        (bash (executable-find "bash")))
+    (skip-unless echo)
+    (skip-unless bash)
+    (dolist (conn-type '(pipe pty))
+      (ert-info ((format "Connection type: %s" conn-type))
+        (process-tests--with-processes processes
+          (let* ((calls ())
+                 (process (make-process
+                           :name "echo"
+                           :command (list echo "first")
+                           :noquery t
+                           :connection-type conn-type
+                           :coding 'utf-8-unix
+                           :sentinel (lambda (process message)
+                                       (push (list process message)
+                                             calls)))))
+            (push process processes)
+            (push (make-process
+                   :name "bash"
+                   :command (list bash "-c" "sleep 10 && echo second")
+                   :noquery t
+                   :connection-type conn-type)
+                  processes)
+            (while (accept-process-output process))
+            (should (equal calls
+                           (list (list process "finished\n"))))))))))
+
 (provide 'process-tests)
 ;;; process-tests.el ends here