From: "João Távora" <joaotavora@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Thomas Koch <thomas@koch.ro>,
Michael Albinus <michael.albinus@gmx.de>,
61350@debbugs.gnu.org
Subject: bug#61350: Eglot over Tramp freezes with large project
Date: Wed, 15 Mar 2023 20:14:06 +0000 [thread overview]
Message-ID: <874jql6acx.fsf@gmail.com> (raw)
In-Reply-To: <878rfx6bqe.fsf@gmail.com> ("João Távora"'s message of "Wed, 15 Mar 2023 19:44:25 +0000")
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
João Távora <joaotavora@gmail.com> writes:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> Does that "some" include _this_ problem? Let's take this one for
> example.
>
> make -C test filenotify-tests
> SELECTOR=file-notify-test04-autorevert
Sorry, here I meant to type 'file-notify-test04-autorevert-remote'
> We know this was introduced by the recent changes
> 54ef338ba3670415cf47fabc33a92d4904707c7e.
FWIW, I've run the following experiment:
1. Reverted 54ef338ba3670415cf47fabc33a92d4904707c7e
2. Applied a simpler, more conservative, patch, discussed with Michael
before. The patch keeps the protocol for tramp-a-p-o untouched. It
simply adds a "while" to the 'a-p-o' call issued for "related"
processes.
3. Ran the filenotify test suite. All but one tests pass.
'file-notify-test04-autorevert-remote' now passes. The failure I
get, 'file-notify-test06-dir-validity-remote', is _also_ in Emacs 29
4. Removed the Tramp-specific Eglot workaround in
lisp/progmodes/eglot.el which disabled Controlmaster.
5. Ran the Eglot-over-Tramp experiment with the JDTLS LSP server
multiple times with 0% failures.
6. The two Tramp-related failures in the eglot-tests.el test suite are
_also_ fixed. Ran this multiple times, 0% failures.
It seems evident that my simpler patch puts us in a much better place
than we are now, so I propose we push them. Patches are attached.
Of course, we can continue to explore other more elegant solutions.
João
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Revert-Improve-Tramp-processes-to-accept-output-over.patch --]
[-- Type: text/x-patch, Size: 9830 bytes --]
From f8ea932928fe37573d28c197c1c9d2da9a08089d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Wed, 15 Mar 2023 19:18:00 +0000
Subject: [PATCH 1/3] Revert "Improve Tramp processes to accept output over the
same socket"
This reverts commit 54ef338ba3670415cf47fabc33a92d4904707c7e.
---
lisp/net/tramp-adb.el | 2 +-
lisp/net/tramp-gvfs.el | 2 +-
lisp/net/tramp-sh.el | 14 +-------------
lisp/net/tramp-smb.el | 6 +++---
lisp/net/tramp-sudoedit.el | 2 +-
lisp/net/tramp.el | 33 +++++++++------------------------
6 files changed, 16 insertions(+), 43 deletions(-)
diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index d338201ab72..64f45e7958d 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -990,7 +990,7 @@ tramp-adb-handle-make-process
(progn
(goto-char (point-min))
(not (search-forward "\n" nil t)))
- (tramp-accept-process-output p))
+ (tramp-accept-process-output p 0))
(delete-region (point-min) (point)))
;; Provide error buffer. This shows only
;; initial error messages; messages
diff --git a/lisp/net/tramp-gvfs.el b/lisp/net/tramp-gvfs.el
index c1ad37de1d2..266724c587f 100644
--- a/lisp/net/tramp-gvfs.el
+++ b/lisp/net/tramp-gvfs.el
@@ -1469,7 +1469,7 @@ tramp-gvfs-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index 882b79b3ee7..48ebfff6cfe 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -2424,10 +2424,6 @@ tramp-do-copy-or-rename-file-out-of-band
copy-program copy-args)))
(tramp-message v 6 "%s" (string-join (process-command p) " "))
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
@@ -3757,10 +3753,6 @@ tramp-sh-handle-file-notify-add-watch
(string-join sequence " "))
(tramp-message v 6 "Run `%s', %S" (string-join sequence " ") p)
(process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and only if
- ;; the respective options are set. Perhaps, the setting could
- ;; be more fine-grained.
- (process-put p 'shared-socket t)
;; Needed for process filter.
(process-put p 'events events)
(process-put p 'watch-name localname)
@@ -3769,7 +3761,7 @@ tramp-sh-handle-file-notify-add-watch
(set-process-sentinel p #'tramp-file-notify-process-sentinel)
;; There might be an error if the monitor is not supported.
;; Give the filter a chance to read the output.
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(unless (process-live-p p)
(tramp-error
p 'file-notify-error "Monitoring not supported for `%s'" file-name))
@@ -5121,10 +5113,6 @@ tramp-maybe-open-connection
;; Set sentinel and query flag. Initialize variables.
(set-process-sentinel p #'tramp-process-sentinel)
(process-put p 'vector vec)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps,
- ;; the setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'adjust-window-size-function #'ignore)
(set-process-query-on-exit-flag p nil)
(setq tramp-current-connection (cons vec (current-time)))
diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
index bb4ab9e3057..1aa4520eeb6 100644
--- a/lisp/net/tramp-smb.el
+++ b/lisp/net/tramp-smb.el
@@ -757,7 +757,7 @@ tramp-smb-action-get-acl
"Read ACL data from connection buffer."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(with-current-buffer (tramp-get-connection-buffer vec)
;; There might be a hidden password prompt.
(widen)
@@ -1363,7 +1363,7 @@ tramp-smb-action-set-acl
"Set ACL data."
(unless (process-live-p proc)
;; Accept pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(tramp-message
vec 10 "\n%s" (tramp-get-buffer-string (tramp-get-connection-buffer vec)))
(throw 'tramp-action 'ok)))
@@ -2023,7 +2023,7 @@ tramp-smb-wait-for-output
;; Read pending output.
(while (not (re-search-forward tramp-smb-prompt nil t))
- (while (tramp-accept-process-output p))
+ (while (tramp-accept-process-output p 0))
(goto-char (point-min)))
(tramp-message vec 6 "\n%s" (buffer-string))
diff --git a/lisp/net/tramp-sudoedit.el b/lisp/net/tramp-sudoedit.el
index 3cacde2468c..abb9afc570b 100644
--- a/lisp/net/tramp-sudoedit.el
+++ b/lisp/net/tramp-sudoedit.el
@@ -692,7 +692,7 @@ tramp-sudoedit-action-sudo
"Check, whether a sudo process has finished. Remove unneeded output."
;; There might be pending output for the exit status.
(unless (process-live-p proc)
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
;; Delete narrowed region, it would be in the way reading a Lisp form.
(goto-char (point-min))
(widen)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index b6e985db6b1..47173b95bea 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5087,11 +5087,6 @@ tramp-handle-make-process
;; t. See Bug#51177.
(when filter
(set-process-filter p filter))
- (process-put p 'vector v)
- ;; This is neded for ssh or PuTTY based processes, and
- ;; only if the respective options are set. Perhaps, the
- ;; setting could be more fine-grained.
- (process-put p 'shared-socket t)
(process-put p 'remote-command orig-command)
(tramp-set-connection-property p "remote-command" orig-command)
@@ -5494,7 +5489,7 @@ tramp-handle-file-notify-rm-watch
;; There might be pending output. Avoid problems with reentrant
;; call of Tramp.
(ignore-errors
- (while (tramp-accept-process-output proc)))
+ (while (tramp-accept-process-output proc 0)))
(tramp-message proc 6 "Kill %S" proc)
(delete-process proc))
@@ -5646,13 +5641,13 @@ tramp-action-process-alive
"Check, whether a process has finished."
(unless (process-live-p proc)
;; There might be pending output.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(throw 'tramp-action 'process-died)))
(defun tramp-action-out-of-band (proc vec)
"Check, whether an out-of-band copy has finished."
;; There might be pending output for the exit status.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(cond ((and (not (process-live-p proc))
(zerop (process-exit-status proc)))
(tramp-message vec 3 "Process has finished.")
@@ -5683,7 +5678,7 @@ tramp-process-one-action
(while (not found)
;; Reread output once all actions have been performed.
;; Obviously, the output was not complete.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(setq todo actions)
(while todo
(setq item (pop todo)
@@ -5800,21 +5795,11 @@ with-tramp-locked-connection
,@body)
(tramp-flush-connection-property ,proc "locked"))))
-(defun tramp-accept-process-output (proc &optional _timeout)
+(defun tramp-accept-process-output (proc &optional timeout)
"Like `accept-process-output' for Tramp processes.
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
- (declare (advertised-calling-convention (proc) "29.2"))
- ;; There could be other processes which use the same socket for
- ;; communication. This could block the output for the current
- ;; process. Read such output first. (Bug#61350)
- (when-let (((process-get proc 'shared-socket))
- (v (process-get proc 'vector)))
- (dolist (p (delq proc (process-list)))
- (when (tramp-file-name-equal-p v (process-get p 'vector))
- (accept-process-output p 0 nil t))))
-
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
@@ -5824,10 +5809,10 @@ tramp-accept-process-output
;; JUST-THIS-ONE is set due to Bug#12145. `with-local-quit'
;; returns t in order to report success.
(if (with-local-quit
- (setq result (accept-process-output proc 0 nil t)) t)
+ (setq result (accept-process-output proc timeout nil t)) t)
(tramp-message
- proc 10 "%s %s %s\n%s"
- proc (process-status proc) result (buffer-string))
+ proc 10 "%s %s %s %s\n%s"
+ proc timeout (process-status proc) result (buffer-string))
;; Propagate quit.
(keyboard-quit)))
result)))
@@ -6840,7 +6825,7 @@ tramp-interrupt-process
(tramp-get-remote-null-device (process-get proc 'vector))))
;; Wait, until the process has disappeared. If it doesn't,
;; fall back to the default implementation.
- (while (tramp-accept-process-output proc))
+ (while (tramp-accept-process-output proc 0))
(not (process-live-p proc))))))
(add-hook 'interrupt-process-functions #'tramp-interrupt-process)
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0003-Simpler-fix-for-bug-61350.patch --]
[-- Type: text/x-patch, Size: 1118 bytes --]
From 16e93416780e416d27ed514e3d8a876405a4f4bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Wed, 15 Mar 2023 20:02:43 +0000
Subject: [PATCH 3/3] Simpler fix for bug#61350
* lisp/net/tramp.el (tramp-accept-process-output): Accept process
from related processes.
---
lisp/net/tramp.el | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 47173b95bea..885b29f9471 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -5800,6 +5800,11 @@ tramp-accept-process-output
This is needed in order to hide `last-coding-system-used', which is set
for process communication also.
If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
+ (when-let (((process-get proc 'shared-socket))
+ (v (process-get proc 'vector)))
+ (dolist (p (delq proc (process-list)))
+ (when (tramp-file-name-equal-p v (process-get p 'vector))
+ (while (accept-process-output p 0 nil t)))))
(with-current-buffer (process-buffer proc)
(let ((inhibit-read-only t)
last-coding-system-used
--
2.39.2
next prev parent reply other threads:[~2023-03-15 20:14 UTC|newest]
Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-07 16:33 bug#61350: Eglot over Tramp freezes with large project Thomas Koch
2023-02-17 9:54 ` Michael Albinus
2023-02-17 10:33 ` Thomas Koch
2023-02-18 11:10 ` Thomas Koch
2023-02-18 12:07 ` Michael Albinus
2023-02-23 11:55 ` Thomas Koch
2023-02-25 14:36 ` Michael Albinus
2023-02-23 12:17 ` João Távora
2023-02-23 14:18 ` João Távora
2023-02-23 14:47 ` Thomas Koch
2023-02-23 15:22 ` João Távora
2023-02-24 17:19 ` Michael Albinus
2023-02-24 17:45 ` João Távora
2023-02-25 14:27 ` Michael Albinus
2023-02-25 23:09 ` João Távora
2023-02-26 10:24 ` Thomas Koch
2023-02-26 15:58 ` Michael Albinus
2023-02-26 17:23 ` Michael Albinus
2023-02-26 21:13 ` João Távora
2023-02-26 21:45 ` João Távora
2023-02-27 7:53 ` Michael Albinus
2023-02-27 9:42 ` João Távora
2023-02-27 20:11 ` Michael Albinus
2023-02-27 7:47 ` Michael Albinus
2023-02-27 9:35 ` João Távora
2023-02-27 20:10 ` Michael Albinus
2023-02-28 0:10 ` João Távora
2023-02-28 10:38 ` Michael Albinus
2023-02-28 11:33 ` João Távora
2023-02-28 12:59 ` Michael Albinus
2023-02-28 14:41 ` João Távora
2023-02-28 14:18 ` Michael Albinus
2023-02-28 14:51 ` João Távora
2023-02-28 15:01 ` Michael Albinus
2023-02-28 17:55 ` Thomas Koch
2023-03-01 14:10 ` João Távora
2023-03-01 16:19 ` João Távora
2023-03-02 11:01 ` Michael Albinus
2023-03-02 11:22 ` João Távora
2023-03-02 11:50 ` Michael Albinus
2023-03-05 11:21 ` Michael Albinus
2023-03-05 11:45 ` Thomas Koch
2023-03-05 12:23 ` Michael Albinus
2023-03-07 12:49 ` Michael Albinus
2023-03-07 13:04 ` Thomas Koch
2023-03-07 13:33 ` João Távora
2023-03-07 13:52 ` Michael Albinus
2023-03-07 14:03 ` João Távora
2023-03-07 14:31 ` Michael Albinus
2023-03-11 9:00 ` Michael Albinus
2023-03-11 10:14 ` Thomas Koch
2023-03-11 11:47 ` João Távora
2023-03-11 12:27 ` Michael Albinus
2023-03-11 11:42 ` João Távora
2023-03-11 12:44 ` Michael Albinus
2023-03-11 14:01 ` João Távora
2023-03-11 14:25 ` Michael Albinus
2023-03-12 0:48 ` João Távora
2023-03-12 10:22 ` Michael Albinus
2023-03-14 11:01 ` Michael Albinus
2023-03-14 15:00 ` Michael Albinus
2023-03-14 15:19 ` João Távora
2023-03-14 15:42 ` Michael Albinus
2023-03-15 17:47 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 18:05 ` João Távora
2023-03-15 18:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 19:44 ` João Távora
2023-03-15 20:14 ` João Távora [this message]
2023-03-15 21:34 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 21:55 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-16 13:28 ` João Távora
2023-03-18 12:34 ` Michael Albinus
2023-03-15 21:43 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-15 21:49 ` João Távora
2023-03-16 6:24 ` Jim Porter
2023-03-16 13:25 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-16 13:28 ` João Távora
2023-03-16 15:58 ` João Távora
2023-03-16 20:36 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-03-16 22:04 ` João Távora
2023-03-07 13:47 ` Michael Albinus
2023-03-06 12:42 ` Michael Albinus
2023-03-06 13:45 ` João Távora
2023-03-06 13:42 ` João Távora
2023-03-02 10:40 ` Michael Albinus
2023-02-28 19:37 ` João Távora
2023-03-01 8:44 ` Michael Albinus
2023-03-01 11:15 ` João Távora
2023-03-01 10:46 ` Gregory Heytings
2023-03-01 11:08 ` João Távora
2023-03-01 11:23 ` Gregory Heytings
2023-03-01 11:37 ` João Távora
2023-03-01 14:51 ` Michael Albinus
2023-03-01 15:02 ` Gregory Heytings
2023-04-24 1:44 ` Aaron Madlon-Kay
2023-05-05 11:32 ` Michael Albinus
2023-05-05 13:14 ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-05-05 14:53 ` Michael Albinus
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
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874jql6acx.fsf@gmail.com \
--to=joaotavora@gmail.com \
--cc=61350@debbugs.gnu.org \
--cc=michael.albinus@gmx.de \
--cc=monnier@iro.umontreal.ca \
--cc=thomas@koch.ro \
/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 public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).