Package: emacs;
Reported by: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 2 May 2024 19:38:01 UTC
Severity: normal
Found in version 29.2.50
To reply to this bug, email your comments to 70724 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#70724
; Package emacs
.
(Thu, 02 May 2024 19:38:01 GMT) Full text and rfc822 format available.Spencer Baugh <sbaugh <at> janestreet.com>
:bug-gnu-emacs <at> gnu.org
.
(Thu, 02 May 2024 19:38:01 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: bug-gnu-emacs <at> gnu.org Cc: dmity <at> gutov.dev, app-emacs-dev <at> janestreet.com Subject: 29.2.50; eglot-reconnect errors when the project is deleted Date: Thu, 02 May 2024 15:37:03 -0400
In some project /home/foo/proj, with pretty much any LSP server: 1. In /home/foo/proj, M-x eglot, starting some LSP server 2. Delete the directory /home/foo/proj 3. The LSP server will crash/exit 4. The process sentinel for the server will run, running eglot--on-shutdown which by default will call eglot-reconnect 5. eglot-reconnect extracts the saved project instance out of the server, which has a root directory which no longer exists, and calls eglot--connect with it 6. eglot--connect calls project-name on a nonexistent project instance, which may fail with an error depending on the project implementation (I have a custom project implementation, but I think this can happen with project-vc too) 7. This causes the process sentinel to error. I think the right fix is probably for eglot--on-shutdown (or maybe eglot-reconnect) to call (project-current nil (project-root pr)) to find the new project instance. If that returns nil, the project has disappeared, and eglot should just not try to reconnect. This also would make eglot behave better if the project layout changes (e.g. if there are nested projects). Alternatively, maybe eglot--on-shutdown shouldn't automatically reconnect in the first place? Maybe reconnection should happen automatically only when some specific buffer tries to interact with the LSP - then it can run project-current in the context of that specific buffer, and see there's no project, and fail. Plus, if the user kills all the buffers in the project (possibly with project-kill-buffers) before deleting it, this approach would entirely avoid the unnecessary eglot reconnection attempt. In GNU Emacs 29.2.50 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.12, Xaw scroll bars) of 2024-04-25 built on igm-qws-u22796a Repository revision: d07451c1f8053fa355d091351a614f232995ab8c Repository branch: emacs-29 Windowing system distributor 'The X.Org Foundation', version 11.0.12011000 System Description: Rocky Linux 8.9 (Green Obsidian) Configured using: 'configure -C --with-x-toolkit=lucid --with-gif=ifavailable' Configured features: CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XINPUT2 XPM LUCID ZLIB Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: ELisp/l Minor modes in effect: repeat-mode: t delete-selection-mode: t global-so-long-mode: t pixel-scroll-precision-mode: t jane-fe-minor-mode: t jane-fe-jenga-minor-mode: t editorconfig-mode: t dired-omit-mode: t which-function-mode: t global-git-commit-mode: t magit-auto-revert-mode: t shell-dirtrack-mode: t server-mode: t savehist-mode: t save-place-mode: t tooltip-mode: t global-eldoc-mode: t eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t tab-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t context-menu-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t line-number-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Features: (magit-patch make-mode doctor texinfo texinfo-loaddefs completion emacs-news-mode latexenc mode-local minibuf-eldef ...) Memory information: ((conses 16 4170589 532507) (symbols 48 67528 55) (strings 32 645610 32608) (string-bytes 1 31944500) (vectors 16 203742) (vector-slots 8 4053669 619438) (floats 8 1045 1428) (intervals 56 410065 1774) (buffers 976 767))
bug-gnu-emacs <at> gnu.org
:bug#70724
; Package emacs
.
(Sat, 04 May 2024 01:11:02 GMT) Full text and rfc822 format available.Message #8 received at 70724 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com>, 70724 <at> debbugs.gnu.org Cc: app-emacs-dev <at> janestreet.com Subject: Re: bug#70724: 29.2.50; eglot-reconnect errors when the project is deleted Date: Sat, 4 May 2024 04:09:48 +0300
Hi Spencer, On 02/05/2024 22:37, Spencer Baugh wrote: > > In some project /home/foo/proj, with pretty much any LSP server: > > 1. In /home/foo/proj, M-x eglot, starting some LSP server > > 2. Delete the directory /home/foo/proj > > 3. The LSP server will crash/exit > > 4. The process sentinel for the server will run, running > eglot--on-shutdown which by default will call eglot-reconnect > > 5. eglot-reconnect extracts the saved project instance out of the > server, which has a root directory which no longer exists, and calls > eglot--connect with it > > 6. eglot--connect calls project-name on a nonexistent project instance, > which may fail with an error depending on the project implementation > (I have a custom project implementation, but I think this can happen > with project-vc too) > > 7. This causes the process sentinel to error. > > I think the right fix is probably for eglot--on-shutdown (or maybe > eglot-reconnect) to call (project-current nil (project-root pr)) to find > the new project instance. If that returns nil, the project has > disappeared, and eglot should just not try to reconnect. This also > would make eglot behave better if the project layout changes (e.g. if > there are nested projects). I think I like this solution (as long as the nil value returned by project-current on this step is appropriately handled). Something like: diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index 6896baf30ce..7b2461c3ce6 100644 --- a/lisp/progmodes/eglot.el +++ b/lisp/progmodes/eglot.el @@ -1426,11 +1426,15 @@ eglot-reconnect (interactive (list (eglot--current-server-or-lose) t)) (when (jsonrpc-running-p server) (ignore-errors (eglot-shutdown server interactive nil 'preserve-buffers))) - (eglot--connect (eglot--major-modes server) - (eglot--project server) - (eieio-object-class-name server) - (eglot--saved-initargs server) - (eglot--language-ids server)) + (let* ((root (project-root (eglot--project server))) + (project (project-current nil root))) + (if (not project) + (eglot--error "Project in `%s' is gone!" root) + (eglot--connect (eglot--major-modes server) + project + (eieio-object-class-name server) + (eglot--saved-initargs server) + (eglot--language-ids server)))) (eglot--message "Reconnected!")) (defvar eglot--managed-mode) ; forward decl Though it also raises a question about the caching strategy for VC-Aware project backend. At the moment is associates a project with a directory more or less indefinitely, and this is a case to watch out for. > Alternatively, maybe eglot--on-shutdown shouldn't automatically > reconnect in the first place? Maybe reconnection should happen > automatically only when some specific buffer tries to interact with the > LSP - then it can run project-current in the context of that specific > buffer, and see there's no project, and fail. Plus, if the user kills > all the buffers in the project (possibly with project-kill-buffers) > before deleting it, this approach would entirely avoid the unnecessary > eglot reconnection attempt. This also sounds good, though it'd probably require more changes overall. Additionally, perhaps I'd change the association from (server -> project) to (server -> project-root), relying on the project backends' internal caches to fetch the project value whenever it's needed. That might be the most reliable approach. Perhaps the slowest in theory, but hopefully not noticeably so.
bug-gnu-emacs <at> gnu.org
:bug#70724
; Package emacs
.
(Sat, 18 May 2024 08:33:01 GMT) Full text and rfc822 format available.Message #11 received at 70724 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: sbaugh <at> janestreet.com, Dmitry Gutov <dmitry <at> gutov.dev> Cc: app-emacs-dev <at> janestreet.com, 70724 <at> debbugs.gnu.org Subject: Re: bug#70724: 29.2.50; eglot-reconnect errors when the project is deleted Date: Sat, 18 May 2024 11:31:53 +0300
Ping! Can we make some progress here? > Cc: app-emacs-dev <at> janestreet.com > Date: Sat, 4 May 2024 04:09:48 +0300 > From: Dmitry Gutov <dmitry <at> gutov.dev> > > Hi Spencer, > > On 02/05/2024 22:37, Spencer Baugh wrote: > > > > In some project /home/foo/proj, with pretty much any LSP server: > > > > 1. In /home/foo/proj, M-x eglot, starting some LSP server > > > > 2. Delete the directory /home/foo/proj > > > > 3. The LSP server will crash/exit > > > > 4. The process sentinel for the server will run, running > > eglot--on-shutdown which by default will call eglot-reconnect > > > > 5. eglot-reconnect extracts the saved project instance out of the > > server, which has a root directory which no longer exists, and calls > > eglot--connect with it > > > > 6. eglot--connect calls project-name on a nonexistent project instance, > > which may fail with an error depending on the project implementation > > (I have a custom project implementation, but I think this can happen > > with project-vc too) > > > > 7. This causes the process sentinel to error. > > > > I think the right fix is probably for eglot--on-shutdown (or maybe > > eglot-reconnect) to call (project-current nil (project-root pr)) to find > > the new project instance. If that returns nil, the project has > > disappeared, and eglot should just not try to reconnect. This also > > would make eglot behave better if the project layout changes (e.g. if > > there are nested projects). > > I think I like this solution (as long as the nil value returned by > project-current on this step is appropriately handled). > > Something like: > > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el > index 6896baf30ce..7b2461c3ce6 100644 > --- a/lisp/progmodes/eglot.el > +++ b/lisp/progmodes/eglot.el > @@ -1426,11 +1426,15 @@ eglot-reconnect > (interactive (list (eglot--current-server-or-lose) t)) > (when (jsonrpc-running-p server) > (ignore-errors (eglot-shutdown server interactive nil > 'preserve-buffers))) > - (eglot--connect (eglot--major-modes server) > - (eglot--project server) > - (eieio-object-class-name server) > - (eglot--saved-initargs server) > - (eglot--language-ids server)) > + (let* ((root (project-root (eglot--project server))) > + (project (project-current nil root))) > + (if (not project) > + (eglot--error "Project in `%s' is gone!" root) > + (eglot--connect (eglot--major-modes server) > + project > + (eieio-object-class-name server) > + (eglot--saved-initargs server) > + (eglot--language-ids server)))) > (eglot--message "Reconnected!")) > > (defvar eglot--managed-mode) ; forward decl > > > Though it also raises a question about the caching strategy for VC-Aware > project backend. At the moment is associates a project with a directory > more or less indefinitely, and this is a case to watch out for. > > > Alternatively, maybe eglot--on-shutdown shouldn't automatically > > reconnect in the first place? Maybe reconnection should happen > > automatically only when some specific buffer tries to interact with the > > LSP - then it can run project-current in the context of that specific > > buffer, and see there's no project, and fail. Plus, if the user kills > > all the buffers in the project (possibly with project-kill-buffers) > > before deleting it, this approach would entirely avoid the unnecessary > > eglot reconnection attempt. > > This also sounds good, though it'd probably require more changes > overall. Additionally, perhaps I'd change the association from (server > -> project) to (server -> project-root), relying on the project > backends' internal caches to fetch the project value whenever it's > needed. That might be the most reliable approach. Perhaps the slowest in > theory, but hopefully not noticeably so. > > > >
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.