From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Troy Hinckley Newsgroups: gmane.emacs.bugs Subject: bug#71896: shell-resync-dirs hang Date: Mon, 8 Jul 2024 12:04:07 -0500 Message-ID: References: <03321175-1a92-4b82-a0cc-d6977b6a733a@Spark> <83b035dd-1ba4-4016-8ba4-cf9bde800175@Spark> <86tth24zbr.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="668c1c0c_7545e146_24f" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="39406"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 71896@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jul 08 19:05:29 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sQrnt-0009s4-4A for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 08 Jul 2024 19:05:29 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sQrnU-00033K-Tc; Mon, 08 Jul 2024 13:05:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sQrnN-00032j-99 for bug-gnu-emacs@gnu.org; Mon, 08 Jul 2024 13:04:57 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sQrnM-0002jr-Si for bug-gnu-emacs@gnu.org; Mon, 08 Jul 2024 13:04:57 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sQrnR-0002BM-LN for bug-gnu-emacs@gnu.org; Mon, 08 Jul 2024 13:05:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Troy Hinckley Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 08 Jul 2024 17:05:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 71896 X-GNU-PR-Package: emacs Original-Received: via spool by 71896-submit@debbugs.gnu.org id=B71896.17204582818346 (code B ref 71896); Mon, 08 Jul 2024 17:05:01 +0000 Original-Received: (at 71896) by debbugs.gnu.org; 8 Jul 2024 17:04:41 +0000 Original-Received: from localhost ([127.0.0.1]:51392 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sQrn6-0002AX-Rm for submit@debbugs.gnu.org; Mon, 08 Jul 2024 13:04:41 -0400 Original-Received: from sender4-op-o15.zoho.com ([136.143.188.15]:17559) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sQrn2-0002AL-MF for 71896@debbugs.gnu.org; Mon, 08 Jul 2024 13:04:40 -0400 ARC-Seal: i=1; a=rsa-sha256; t=1720458257; cv=none; d=zohomail.com; s=zohoarc; b=Kjw6A35YWtfjg6QvOe475feXIaTJ8Rk1Lry9P9MuOpr5GkywDBTuDDa5EJEYFIJ/nky1JAm0N2Zk1d6K0/kyOAXj7OgfneUlXBfn6Lee2w6+2rGTieMu0bcC8y7D5CIg9Ifk0s/PTpXxtJJTHdOBAqP9g/zL7Fo8RiMuyzG9t7Q= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1720458257; h=Content-Type:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=i8p4EWXji2Nt/3bgTW9LjQ9ecD5AE9AJ9Uf8zs5T1lo=; b=MbFgrHTqBl5u9IcdbA6mrqU4kfg1aSd4JDY+euQnZR/ThA5t7KRC45zbnH6Wh5489ptdtC558tWbvYAvggu8d7XMAWoTr8yY0NlsN4QjYd/U9V1twNdf+JOg75ds5WVrfiyWYV1MST6RbNiJCfuhlTifzddtEimw1UaFAVnGdGE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=dabrev.com; spf=pass smtp.mailfrom=troyhinckley@dabrev.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1720458257; s=zoho; d=dabrev.com; i=troyhinckley@dabrev.com; h=Date:Date:From:From:To:To:Cc:Cc:Message-ID:In-Reply-To:References:Subject:Subject:MIME-Version:Content-Type:Message-Id:Reply-To; bh=i8p4EWXji2Nt/3bgTW9LjQ9ecD5AE9AJ9Uf8zs5T1lo=; b=DY6W+BsGM1tKtu0SulqNtp3tOZAKf14XMd2CBlzv6MmBZxnVLpbDTc6Mj1GQcAmf R4Fu1sHOxLbPpw/2XULYM73GMCka7w8+LXxtd7cmvQKYfjDONVAqqTCPbwzs6oOSJbf uvkwVIIPC1VKUsiJNgwv9wVC07DdTPcUdm29dEHM= Original-Received: by mx.zohomail.com with SMTPS id 1720458255824491.6835186396339; Mon, 8 Jul 2024 10:04:15 -0700 (PDT) In-Reply-To: <86tth24zbr.fsf@gnu.org> X-Readdle-Message-ID: ce5f9303-53dd-4f3e-a279-55a3ba4adacd@Spark X-ZohoMailClient: External X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:288601 Archived-At: --668c1c0c_7545e146_24f Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Thanks Eli, The proposed patch throws an error when =60last=E2=80=99 is =E2=80=9C=E2=80= =9D because (substring =E2=80=9C=E2=80=9D -1) produces (args-out-of-range= =E2=80=9C=E2=80=9D -1 nil). -Troy Hinckley On Jul 6, 2024 at 4:58=E2=80=AFAM -0500, Eli Zaretskii , = wrote: > > Date: Mon, 1 Jul 2024 21:22:35 -0500 > > =46rom: Troy Hinckley > > > > This is related to =2359804 and =2354384. > > > > I will occasionally (about 30% of the time) see hangs when running sh= ell-resync-dirs in Emacs 29. I tracked this > > down to two issues: > > > > =46irst is in shell-eval-command. =46or Emacs 29 this function was ad= ded to fix =2354384. It has this section in the > > code: > > > > =60=60=60 > > ;; 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 =22=22)) > > (or (not prev) > > (not (equal last prev))) > > (setq prev last)))) > > (accept-process-output proc 0 100)) > > =60=60=60 > > > > Note that is says that is looking for =E2=80=9Ca line without a newli= ne=E2=80=9D 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 c= ode evaluates to nil. > > > > =60=60=60 > > (let ((result =22dirs=5Cn=22) prev) > > (not (let* ((lines (string-lines result)) > > (last (car (last lines)))) > > (and (length> lines 0) > > (not (equal last =22=22)) > > (or (not prev) > > (not (equal last prev))) > > (setq prev last))))) > > =60=60=60 > > > > 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 ca= se 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=3F > > > This leads us to the hang. The issue is this code in shell-resync-dir= s: > > > > =60=60=60 > > (while dlsl > > (let ((newelt =22=22) > > tem1 tem2) > > (while newelt > > ;; We need tem1 because we don't want to prepend > > ;; =60comint-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=3D =22 =22 (car dlsl)) > > (pop dlsl)) > > (setq newelt nil)) > > (t > > (setq newelt (concat tem1 newelt))))))) > > =60=60=60 > > > > 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 provid= e 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. > > > > =46or the shell-eval-command I don=E2=80=99t understand what that loo= p 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= =E2=80=9C=5Cn=E2=80=9D result) to check if the output ends in a > > newline, but the code is obviously trying to do something more comple= x there. > > > > If we fix that issue then it will resolve the hang in shell-resync-di= rs, but I think that is just glossing over the > > problem. That functions should never hang, no matter what output it g= et=E2=80=99s from the shell. My recommendation > > would be to add =60(and dlsl newelt)=60 as the condition for the inne= r while loop with newelt. That way if dlsl is > > empty, it will terminate the loop since there is nothing more to proc= ess. 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 > =40=40 -1629,10 +1629,12 =40=40 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 =22=22)) > + (not (member last '(=22=22 =22=5Cn=22))) > + (not (equal last-end =22=5Cn=22)) > (or (not prev) > (not (equal last prev))) > (setq prev last)))) --668c1c0c_7545e146_24f Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline
Thanks Eli,
The proposed patch throws an error when =60last=E2=80=99 is =E2=80=9C=E2=80= =9D because (substring =E2=80=9C=E2=80=9D -1) produces (args-out-of-range= =E2=80=9C=E2=80=9D -1 nil).

-Troy Hinckley
On Jul 6, 2024 at 4:58=E2=80=AFAM -= 0500, Eli Zaretskii <eliz=40gnu.org>, wrote:
Date: Mon, 1 Jul 2024 21:22:35 -0500
=46rom: Troy Hinckley <troyhinckley=40dabrev.com>

This is related to =2359804 and =2354384.

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:

=46irst is in shell-eval-command. =46or Emacs 29 this function was added = to fix =2354384. It has this section in the
code:

=60=60=60
;; 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 =22=22))
(or (not prev)
(not (equal last prev)))
(setq prev last))))
(accept-process-output proc 0 100))
=60=60=60

Note that is says that is looking for =E2=80=9Ca line without a newline=E2= =80=9D to determine if we have reached the prompt.
However this code does not actually do that. If the result ends in a newl= ine 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.

=60=60=60
(let ((result =22dirs=5Cn=22) prev)
(not (let* ((lines (string-lines result))
(last (car (last lines))))
(and (length> lines 0)
(not (equal last =22=22))
(or (not prev)
(not (equal last prev)))
(setq prev last)))))
=60=60=60

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 t= he 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=3F

This leads us to the hang. The issue is thi= s code in shell-resync-dirs:

=60=60=60
(while dlsl
(let ((newelt =22=22)
tem1 tem2)
(while newelt
;; We need tem1 because we don't want to prepend
;; =60comint-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=3D =22 =22 (car dlsl))
(pop dlsl))
(setq newelt nil))
(t
(setq newelt (concat tem1 newelt)))))))
=60=60=60

This loop can only terminate if tem2 is a valid directory. Otherwise it w= ill take the default case in the cond and
loop forever. And since the bug in shell-eval-command does not provide th= e 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.

=46or the shell-eval-command I don=E2=80=99t 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 =E2= =80=9C=5Cn=E2=80=9D result) to check if the output ends in a
newline, but the code is obviously trying to do something more complex th= ere.

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=E2= =80=99s from the shell. My recommendation
would be to add =60(and dlsl newelt)=60 as the condition for the inner wh= ile 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
=40=40 -1629,10 +1629,12 =40=40 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 =22=22))
+ (not (member last '(=22=22 =22=5Cn=22)))
+ (not (equal last-end =22=5Cn=22))
(or (not prev)
(not (equal last prev)))
(setq prev last))))
--668c1c0c_7545e146_24f--