From 8c35638c12eaa4815542c66cfeaf788e9c1e431d Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Sat, 6 Jul 2024 14:09:08 -0700 Subject: [PATCH] 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. (cherry picked from commit 8e46f44ea0eb761e24beda8c5cdbc8fcca87307a) --- doc/misc/eshell.texi | 7 +++-- etc/NEWS | 6 ++++ lisp/eshell/esh-cmd.el | 2 +- lisp/eshell/esh-proc.el | 65 +++++++++++++++++++++++++++++------------ lisp/eshell/eshell.el | 7 ++--- 5 files changed, 62 insertions(+), 25 deletions(-) diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index 8c6a5286619..27300e28ca5 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))) -- 2.39.2