From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#71896: shell-resync-dirs hang Date: Sat, 06 Jul 2024 12:58:00 +0300 Message-ID: <86tth24zbr.fsf@gnu.org> References: <03321175-1a92-4b82-a0cc-d6977b6a733a@Spark> <83b035dd-1ba4-4016-8ba4-cf9bde800175@Spark> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26783"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 71896@debbugs.gnu.org To: Troy Hinckley Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jul 06 11:59:26 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 1sQ2CS-0006jk-3W for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 06 Jul 2024 11:59:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sQ2C6-00075r-3v; Sat, 06 Jul 2024 05:59:02 -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 1sQ2C4-00072U-0Q for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2024 05:59:00 -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 1sQ2C3-0003kT-Dd for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2024 05:58:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sQ2C6-0000hh-C7 for bug-gnu-emacs@gnu.org; Sat, 06 Jul 2024 05:59:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 Jul 2024 09:59:02 +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.17202598972649 (code B ref 71896); Sat, 06 Jul 2024 09:59:02 +0000 Original-Received: (at 71896) by debbugs.gnu.org; 6 Jul 2024 09:58:17 +0000 Original-Received: from localhost ([127.0.0.1]:45606 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sQ2BM-0000ge-CL for submit@debbugs.gnu.org; Sat, 06 Jul 2024 05:58:16 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:34378) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sQ2BK-0000gS-53 for 71896@debbugs.gnu.org; Sat, 06 Jul 2024 05:58:15 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sQ2BA-0003DE-OE; Sat, 06 Jul 2024 05:58:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=yDCvcz3q4gBMCMwdcAzsNcSqYPpDcXBBkl6oqXJZ4f4=; b=igKZrV3xoTzgL3TYmHpI 5M/eLuXxptM4o3K4ZKqGC1vPz8aOcc86K0GDwkzDZ7WGfY88ECf9rh8o/SBgMtwz5/yAAakQ5QbUK 3stpFI0MGAydz4rapH7X4LIQQiFKOlAmj8cN49yuq67ajUC/TG/03I8L2+yse8om450CIwQOg7gX8 s5HpT1Rimi3GjxQGqHmgXIpqaeSevAnJ9T4uRgDAMfS+LfL53KkIC3dmRy3Yd6x1jG+bn/qv2Ck+E btuRExNYvjARgH9FPfEHfOTn3YxtJd88PJHWrv3+ZMXeh+4aYzO/t0uKiHMlcxTQWiKtfdFCj4Nyk 2EyPAhT4vz7FjQ==; In-Reply-To: <83b035dd-1ba4-4016-8ba4-cf9bde800175@Spark> (message from Troy Hinckley on Mon, 1 Jul 2024 21:22:35 -0500) 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:288483 Archived-At: > Date: Mon, 1 Jul 2024 21:22:35 -0500 > From: Troy Hinckley > > 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))))