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.

diff --git a/src/process.c b/src/process.c
index aca87f8ed353a777b65d5772957c1bfd5a82832d..09f87908a45124b6a8a43e2e0b5039c9531513f7 100644 (file)
--- 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 dad36426a09d2122ae555878f0f82cda8278f8c3..d2a98dc19f29e614de1dc9b0180352d87799654c 100644 (file)
--- 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