Improve Eshell's behavior when waiting for processes

This has a few benefits.  First, it fixes a race condition when killing
old processes in 'eshell-command'.  Second, the "wait" built-in command
is now more useful.  Finally, killing processes when exiting Eshell (via
'eshell-round-robin-kill') should be much faster.

* lisp/eshell/esh-proc.el (esh-opt): Require.
(eshell-wait-for-process): Make obsolete in favor of...
(eshell-wait-for-processes): ... this.  Accept a timeout and support
PIDs.  Update callers.
(eshell/wait): New implementation accepting -t/--timeout.
(eshell-round-robin-kill): Use 'eshell-wait-for-processes'.

* lisp/eshell/eshell.el (eshell-command): Use 'eshell-round-robin-kill'.

* doc/misc/eshell.texi (List of Built-ins): Document the new "wait"
behavior.

* etc/NEWS: Announce this change.

5 files changed, 62 insertions, 25 deletions

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 69f94fab469..8547131194e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1201,8 +1201,11 @@ or a string, referring to an environment variable.
 
 @cmindex wait
 @cindex processes, waiting for
-@item wait [@var{process}]@dots{}
-Wait until each specified @var{process} has exited.
+@item wait [-t @var{timeout}] [@var{process}]@dots{}
+Wait until each specified @var{process} has exited.  Processes can
+either be process objects (@pxref{Processes, , , elisp, GNU Emacs Lisp
+Reference Manual}) or integer PIDs.  If you pass @code{-t} or
+@code{--timeout}, wait at most that many seconds before exiting.
 
 @cmindex which
 @item which @var{command}@dots{}
diff --git a/etc/NEWS b/etc/NEWS
index 8c49d4b24f6..f10f9ae4d65 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -54,6 +54,12 @@ this will prompt for confirmation before creating a new buffer when
 necessary.  To restore the previous behavior, set this option to
 'confirm-kill-process'.
 
++++
+*** Eshell's built-in "wait" command now accepts a timeout.
+By passing "-t" or "--timeout", you can specify a maximum time to wait
+for the processes to exit.  Additionally, you can now wait for external
+processes by passing their PIDs.
+
 ** SHR
 
 +++
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 0b3137127d2..b936f68a57a 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1299,7 +1299,7 @@ have been replaced by constants."
               (if-let (((memq (car form) eshell-deferrable-commands))
                        (procs (eshell-make-process-list result)))
                   (if synchronous-p
-                      (apply #'eshell/wait procs)
+                      (funcall #'eshell-wait-for-processes procs)
                     (eshell-manipulate form "inserting ignore form"
                       (setcar form 'ignore)
                       (setcdr form nil))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index f982e2101f5..0dcdf3bb76c 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -25,6 +25,7 @@
 
 (require 'esh-arg)
 (require 'esh-io)
+(require 'esh-opt)
 (require 'esh-util)
 
 (require 'pcomplete)
@@ -184,16 +185,46 @@ This is like `process-live-p', but additionally checks whether
       ;; cleared out the handles (see `eshell-sentinel').
       (process-get process :eshell-handles)))
 
-(defun eshell-wait-for-process (&rest procs)
-  "Wait until PROCS have successfully completed."
-  (dolist (proc procs)
-    (when (eshell-processp proc)
-      (while (eshell-process-active-p proc)
-        (when (input-pending-p)
-          (discard-input))
-        (sit-for eshell-process-wait-time)))))
+(defun eshell-wait-for-processes (&optional procs timeout)
+  "Wait until PROCS have completed execution.
+If TIMEOUT is non-nil, wait at most that many seconds.  Return non-nil
+if all the processes finished executing before the timeout expired."
+  (let ((expiration (when timeout (time-add (current-time) timeout))))
+    (catch 'timeout
+      (dolist (proc procs)
+        (while (if (processp proc)
+                   (eshell-process-active-p proc)
+                 (process-attributes proc))
+          (when (input-pending-p)
+            (discard-input))
+          (when (and expiration
+                     (not (time-less-p (current-time) expiration)))
+            (throw 'timeout nil))
+          (sit-for eshell-process-wait-time)))
+      t)))
 
-(defalias 'eshell/wait #'eshell-wait-for-process)
+(defun eshell-wait-for-process (&rest procs)
+  "Wait until PROCS have completed execution."
+  (declare (obsolete 'eshell-wait-for-processes "31.1"))
+  (eshell-wait-for-processes procs))
+
+(defun eshell/wait (&rest args)
+  "Wait until processes have completed execution."
+  (eshell-eval-using-options
+   "wait" args
+   '((?h "help" nil nil "show this usage screen")
+     (?t "timeout" t timeout "timeout in seconds")
+     :preserve-args
+     :show-usage
+     :usage "[OPTION] PROCESS...
+Wait until PROCESS(es) have completed execution.")
+   (when (stringp timeout)
+     (setq timeout (string-to-number timeout)))
+   (dolist (arg args)
+     (unless (or (processp arg) (natnump arg))
+       (error "wait: invalid argument type: %s" (type-of arg))))
+   (unless (eshell-wait-for-processes args timeout)
+     (error "wait: timed out after %s seconds" timeout))))
 
 (defun eshell/jobs ()
   "List processes, if there are any."
@@ -626,16 +657,14 @@ long to delay between signals."
 (defun eshell-round-robin-kill (&optional query)
   "Kill current process by trying various signals in sequence.
 See the variable `eshell-kill-processes-on-exit'."
-  (let ((sigs eshell-kill-process-signals))
-    (while sigs
+  (catch 'done
+    (dolist (sig eshell-kill-process-signals)
       (eshell-process-interact
-       (lambda (proc)
-         (signal-process (process-id proc) (car sigs))) t query)
-      (setq query nil)
-      (if (not eshell-process-list)
-          (setq sigs nil)
-        (sleep-for eshell-kill-process-wait-time)
-        (setq sigs (cdr sigs))))))
+       (lambda (proc) (signal-process proc sig)) t query)
+      (when (eshell-wait-for-processes (mapcar #'car eshell-process-list)
+                                       eshell-kill-process-wait-time)
+        (throw 'done nil))
+      (setq query nil))))
 
 (defun eshell-query-kill-processes ()
   "Kill processes belonging to the current Eshell buffer, possibly with query."
diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 568f6745067..b7be3dd1643 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -176,7 +176,7 @@
   (require 'cl-lib))
 (require 'esh-util)
 (require 'esh-module)                   ;For eshell-using-module
-(require 'esh-proc)                     ;For eshell-wait-for-process
+(require 'esh-proc)                     ;For eshell-wait-for-processes
 (require 'esh-io)                       ;For eshell-last-command-status
 (require 'esh-cmd)
 
@@ -357,8 +357,7 @@ buffer is already taken by another running shell command."
               (with-current-buffer bufname
                 ;; Stop all the processes in the old buffer (there may
                 ;; be several).
-                (eshell-process-interact #'interrupt-process t))
-              (accept-process-output)
+                (eshell-round-robin-kill))
               (kill-buffer bufname))
              ((eq eshell-command-async-buffer 'confirm-new-buffer)
               (shell-command--same-buffer-confirm "Use a new buffer")
@@ -377,7 +376,7 @@ buffer is already taken by another running shell command."
         ;; make the output as attractive as possible, with no
         ;; extraneous newlines
         (unless async
-          (apply #'eshell-wait-for-process (cadr eshell-foreground-command))
+          (funcall #'eshell-wait-for-processes (cadr eshell-foreground-command))
           (cl-assert (not eshell-foreground-command))
           (goto-char (point-max))
           (while (and (bolp) (not (bobp)))