From: Eli Zaretskii <eliz@gnu.org>
To: Troy Hinckley <troyhinckley@dabrev.com>
Cc: 71896@debbugs.gnu.org
Subject: bug#71896: shell-resync-dirs hang
Date: Sat, 06 Jul 2024 12:58:00 +0300 [thread overview]
Message-ID: <86tth24zbr.fsf@gnu.org> (raw)
In-Reply-To: <83b035dd-1ba4-4016-8ba4-cf9bde800175@Spark> (message from Troy Hinckley on Mon, 1 Jul 2024 21:22:35 -0500)
> Date: Mon, 1 Jul 2024 21:22:35 -0500
> From: Troy Hinckley <troyhinckley@dabrev.com>
>
> This is related to #59804 and #54384.
>
> I will occasionally (about 30% of the time) see hangs when running shell-resync-dirs in Emacs 29. I tracked this
> down to two issues:
>
> First is in shell-eval-command. For Emacs 29 this function was added to fix #54384. It has this section in the
> code:
>
> ```
> ;; Wait until we get a prompt (which will be a line without
> ;; a newline). This is far from fool-proof -- if something
> ;; outputs incomplete data and then sleeps, we'll think
> ;; we've received the prompt.
> (while (not (let* ((lines (string-lines result))
> (last (car (last lines))))
> (and (length> lines 0)
> (not (equal last ""))
> (or (not prev)
> (not (equal last prev)))
> (setq prev last))))
> (accept-process-output proc 0 100))
> ```
>
> Note that is says that is looking for “a line without a newline” to determine if we have reached the prompt.
> However this code does not actually do that. If the result ends in a newline it will still terminate the loop and not
> wait for more input. We can see that by the fact that the following code evaluates to nil.
>
> ```
> (let ((result "dirs\n") prev)
> (not (let* ((lines (string-lines result))
> (last (car (last lines))))
> (and (length> lines 0)
> (not (equal last ""))
> (or (not prev)
> (not (equal last prev)))
> (setq prev last)))))
> ```
>
> I am not sure what this code is supposed to do, but the issue arrises if the process output sends anything to
> this function it will terminate and not wait for more input. In my case the issue is that the shell is echoing the
> command followed by the result (comint-process-echoes). About 30% of the time these two lines get sent as
> part of two different outputs. Meaning the second line (the directory for shell-resync-dirs) does not get captured
> and instead gets printed to the terminal.
Does the patch below solve the problem in shell-eval-command?
> This leads us to the hang. The issue is this code in shell-resync-dirs:
>
> ```
> (while dlsl
> (let ((newelt "")
> tem1 tem2)
> (while newelt
> ;; We need tem1 because we don't want to prepend
> ;; `comint-file-name-prefix' repeatedly into newelt via tem2.
> (setq tem1 (pop dlsl)
> tem2 (concat comint-file-name-prefix tem1 newelt))
> (cond ((file-directory-p tem2)
> (push tem2 ds)
> (when (string= " " (car dlsl))
> (pop dlsl))
> (setq newelt nil))
> (t
> (setq newelt (concat tem1 newelt)))))))
> ```
>
> This loop can only terminate if tem2 is a valid directory. Otherwise it will take the default case in the cond and
> loop forever. And since the bug in shell-eval-command does not provide the directory when the process output
> is split, we get a hang.
>
> I believe both of these need to be fixed to properly fix the bug.
>
> For the shell-eval-command I don’t understand what that loop is trying to do now, so I am not sure how to fix it
> without breaking its functionality. I would just use (string-suffix-p “\n” result) to check if the output ends in a
> newline, but the code is obviously trying to do something more complex there.
>
> If we fix that issue then it will resolve the hang in shell-resync-dirs, but I think that is just glossing over the
> problem. That functions should never hang, no matter what output it get’s from the shell. My recommendation
> would be to add `(and dlsl newelt)` as the condition for the inner while loop with newelt. That way if dlsl is
> empty, it will terminate the loop since there is nothing more to process. This fixed the issue for me.
Thanks, I think I agree with your suggestion for shell-resync-dirs.
But please undo the fix you evidently made there to avoid the infloop,
and see if the patch below for shell-eval-command makes
shell-resync-dirs do its job by correctly resynchronizing
shell-dirtrack.
diff --git a/lisp/shell.el b/lisp/shell.el
index e1936ff..f86156e 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -1629,10 +1629,12 @@ shell-eval-command
;; a newline). This is far from fool-proof -- if something
;; outputs incomplete data and then sleeps, we'll think
;; we've received the prompt.
- (while (not (let* ((lines (string-lines result))
- (last (car (last lines))))
+ (while (not (let* ((lines (string-lines result nil t))
+ (last (car (last lines)))
+ (last-end (substring last -1)))
(and (length> lines 0)
- (not (equal last ""))
+ (not (member last '("" "\n")))
+ (not (equal last-end "\n"))
(or (not prev)
(not (equal last prev)))
(setq prev last))))
next prev parent reply other threads:[~2024-07-06 9:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <03321175-1a92-4b82-a0cc-d6977b6a733a@Spark>
2024-07-02 2:22 ` bug#71896: shell-resync-dirs hang Troy Hinckley
2024-07-06 9:58 ` Eli Zaretskii [this message]
2024-07-08 17:04 ` Troy Hinckley
2024-07-08 17:42 ` Eli Zaretskii
2024-07-08 17:53 ` Troy Hinckley
2024-07-12 7:00 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86tth24zbr.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=71896@debbugs.gnu.org \
--cc=troyhinckley@dabrev.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.