bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Thomas Fitzsimmons]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Hi,

The attached patch fixes an issue reported on the mailing list [1].
After quitting a remote "emacsclient -c" frame using C-x 5 0, the SSH
session will hang on exit.  It is waiting for the X11 display connection
to be closed, but Emacs never closes it.

I have been using this patch for a few months without issue, with the
Lucid toolkit, running "emacsclient -c" over a remote X11 connection.

I just retested it on master (423c86cbde7b1ed1d42c7e21fef6e8be872857b0)
with "./configure --with-x-toolkit=lucid" and it works for me.

I would like others who use remote X11 emacsclient to try the patch, to
make sure it does not introduce crashes, error messages or warnings.  If
it works for others, I can push the patch to master.

Thomas

1. https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00950.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Close-X-connection-upon-deletion-of-last-emacsclient.patch --]
[-- Type: text/x-diff, Size: 1593 bytes --]

From 27496ab740722d2246ecec1ffbb7f7390d081873 Mon Sep 17 00:00:00 2001
From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Date: Wed, 7 Aug 2024 19:46:04 -0400
Subject: [PATCH] Close X connection upon deletion of last emacsclient frame

This fixes an issue reported on the mailing list:
https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00950.html

* lisp/server.el (server-handle-delete-frame): If the frame is an
X frame and DISPLAY is set, close the X connection to the display.
---
 lisp/server.el | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/server.el b/lisp/server.el
index abfd3d4d753..6f39ae651c7 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -510,7 +510,13 @@ server-handle-delete-frame
                             (eq proc (frame-parameter f 'client))))
                      (frame-list))))
       (server-log (format "server-handle-delete-frame, frame %s" frame) proc)
-      (server-delete-client proc 'noframe)))) ; Let delete-frame delete the frame later.
+      (server-delete-client proc 'noframe) ; Let delete-frame delete the frame later.
+      ;; Close the X connection next time the main loop becomes idle.
+      ;; This prevents an SSH session from hanging after exiting a
+      ;; remote "emacsclient -c" frame via C-x 5 0.
+      (let ((display (frame-parameter frame 'display)))
+	(when (and display (eq (framep frame) 'x))
+	  (run-at-time nil nil (lambda () (x-close-connection display))))))))
 
 (defun server-handle-suspend-tty (terminal)
   "Notify the client process that its tty device is suspended."
-- 
2.39.2

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Eli Zaretskii]

> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
> Date: Wed, 07 Aug 2024 20:47:48 -0400
> 
> The attached patch fixes an issue reported on the mailing list [1].
> After quitting a remote "emacsclient -c" frame using C-x 5 0, the SSH
> session will hang on exit.  It is waiting for the X11 display connection
> to be closed, but Emacs never closes it.
> 
> I have been using this patch for a few months without issue, with the
> Lucid toolkit, running "emacsclient -c" over a remote X11 connection.
> 
> I just retested it on master (423c86cbde7b1ed1d42c7e21fef6e8be872857b0)
> with "./configure --with-x-toolkit=lucid" and it works for me.
> 
> I would like others who use remote X11 emacsclient to try the patch, to
> make sure it does not introduce crashes, error messages or warnings.  If
> it works for others, I can push the patch to master.

Thanks, but "ssh -X" is not the only way of starting a remote client
session, is it?  How do we know closing the X connection is TRT in all
the cases, and cannot do any harm in some use cases other than yours?

I'm also surprised that such a fundamental problem is raised only now,
when remote connections existed for decades.  Are you saying this is a
regression due to some recent change we installed?  If not, how come
this went undetected for so many years?

Po Lu, any comments or suggestions?

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
>> Date: Wed, 07 Aug 2024 20:47:48 -0400
>> 
>> The attached patch fixes an issue reported on the mailing list [1].
>> After quitting a remote "emacsclient -c" frame using C-x 5 0, the SSH
>> session will hang on exit.  It is waiting for the X11 display connection
>> to be closed, but Emacs never closes it.
>> 
>> I have been using this patch for a few months without issue, with the
>> Lucid toolkit, running "emacsclient -c" over a remote X11 connection.
>> 
>> I just retested it on master (423c86cbde7b1ed1d42c7e21fef6e8be872857b0)
>> with "./configure --with-x-toolkit=lucid" and it works for me.
>> 
>> I would like others who use remote X11 emacsclient to try the patch, to
>> make sure it does not introduce crashes, error messages or warnings.  If
>> it works for others, I can push the patch to master.
>
> Thanks, but "ssh -X" is not the only way of starting a remote client
> session, is it?  How do we know closing the X connection is TRT in all
> the cases, and cannot do any harm in some use cases other than yours?
>
> I'm also surprised that such a fundamental problem is raised only now,
> when remote connections existed for decades.  Are you saying this is a
> regression due to some recent change we installed?  If not, how come
> this went undetected for so many years?
>
> Po Lu, any comments or suggestions?

The issue that ought to be fixed is emacsclient's waiting for the
display connection to be closed before exiting, as the automatic closure
of connections without frames is expressly disabled on X toolkit builds,
since closing a display connection is liable to induce crashes in the X
toolkit.  Unless I misunderstand Thomas and it is ssh that is refusing
to exit.

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Thomas Fitzsimmons]

Po Lu <luangruo@yahoo.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
>>> Date: Wed, 07 Aug 2024 20:47:48 -0400
>>> 
>>> The attached patch fixes an issue reported on the mailing list [1].
>>> After quitting a remote "emacsclient -c" frame using C-x 5 0, the SSH
>>> session will hang on exit.  It is waiting for the X11 display connection
>>> to be closed, but Emacs never closes it.
>>> 
>>> I have been using this patch for a few months without issue, with the
>>> Lucid toolkit, running "emacsclient -c" over a remote X11 connection.
>>> 
>>> I just retested it on master (423c86cbde7b1ed1d42c7e21fef6e8be872857b0)
>>> with "./configure --with-x-toolkit=lucid" and it works for me.
>>> 
>>> I would like others who use remote X11 emacsclient to try the patch, to
>>> make sure it does not introduce crashes, error messages or warnings.  If
>>> it works for others, I can push the patch to master.
>>
>> Thanks, but "ssh -X" is not the only way of starting a remote client
>> session, is it?  How do we know closing the X connection is TRT in all
>> the cases, and cannot do any harm in some use cases other than yours?
>>
>> I'm also surprised that such a fundamental problem is raised only now,
>> when remote connections existed for decades.  Are you saying this is a
>> regression due to some recent change we installed?  If not, how come
>> this went undetected for so many years?
>>
>> Po Lu, any comments or suggestions?
>
> The issue that ought to be fixed is emacsclient's waiting for the
> display connection to be closed before exiting, as the automatic closure
> of connections without frames is expressly disabled on X toolkit builds,
> since closing a display connection is liable to induce crashes in the X
> toolkit.  Unless I misunderstand Thomas and it is ssh that is refusing
> to exit.

Thank you for taking a look at this.

It is indeed SSH that is refusing to exit.

i.e., after exiting the emacsclient frame, I am returned to the remote
SSH shell.  Pressing control-d in that shell does not return me back to
my local shell (the shell from which I SSH'd).  I have to press
control-c and then I am returned back to my local shell.

Nothing emacs-related is hanging; the emacs daemon continues running,
and I can re-SSH and re-run emacsclient.  So the control-c to exit the
SSH session does not seem to destabilize the emacs daemon.  (However, I
suspect the control-c is severing the X display connection which seems
less safe and potentially more subject to race conditions than Emacs
itself closing the X connection.)

There are so many use cases for emacsclient that any patch seems risky,
which is why I filed this bug report with something that works for me.

If there is a comprehensive set of emacsclient connection/disconnection
tests, and/or any verbosity/debugging options that might help, then I
can run them by hand on my setup.  I did try:

1. local X11 "emacsclient -c -s test"
2. local X11 "emacsclient -nw -s test"
3. remote (over ssh -X) "emacsclient -c -s test"
4. remote (over ssh -X) "emacsclient -nw -s test"

against "emacs -Q --fg-daemon=test".  In each case, with this patch, the
key sequence C-x 5 0 exits the frame without issue, and (for tests 3 and
4) C-d exits the SSH session (returns me to my local shell) without my
having to press C-c.  Without my patch, test 3 does result in SSH
refusing to exit.

And, even with my patch, for test case 3, using C-x C-c to delete the
frame still results in SSH refusing to exit.  (It would be nice to fix
this case too, but I always use C-x 5 0, so I wanted to start by trying
to fix it.)

Thomas

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:

> Thank you for taking a look at this.
>
> It is indeed SSH that is refusing to exit.
>
> i.e., after exiting the emacsclient frame, I am returned to the remote
> SSH shell.  Pressing control-d in that shell does not return me back to
> my local shell (the shell from which I SSH'd).  I have to press
> control-c and then I am returned back to my local shell.
>
> Nothing emacs-related is hanging; the emacs daemon continues running,
> and I can re-SSH and re-run emacsclient.  So the control-c to exit the
> SSH session does not seem to destabilize the emacs daemon.  (However, I
> suspect the control-c is severing the X display connection which seems
> less safe and potentially more subject to race conditions than Emacs
> itself closing the X connection.)
>
> There are so many use cases for emacsclient that any patch seems risky,
> which is why I filed this bug report with something that works for me.
>
> If there is a comprehensive set of emacsclient connection/disconnection
> tests, and/or any verbosity/debugging options that might help, then I
> can run them by hand on my setup.  I did try:
>
> 1. local X11 "emacsclient -c -s test"
> 2. local X11 "emacsclient -nw -s test"
> 3. remote (over ssh -X) "emacsclient -c -s test"
> 4. remote (over ssh -X) "emacsclient -nw -s test"
>
> against "emacs -Q --fg-daemon=test".  In each case, with this patch, the
> key sequence C-x 5 0 exits the frame without issue, and (for tests 3 and
> 4) C-d exits the SSH session (returns me to my local shell) without my
> having to press C-c.  Without my patch, test 3 does result in SSH
> refusing to exit.
>
> And, even with my patch, for test case 3, using C-x C-c to delete the
> frame still results in SSH refusing to exit.  (It would be nice to fix
> this case too, but I always use C-x 5 0, so I wanted to start by trying
> to fix it.)
>
> Thomas

I suggest configuring Emacs with --with-x-toolkit=no (as is recommended
for users who connect to remote display servers to begin with).

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Thomas Fitzsimmons]

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

Po Lu <luangruo@yahoo.com> writes:

> Thomas Fitzsimmons <fitzsim@fitzsim.org> writes:
>
>> Thank you for taking a look at this.
>>
>> It is indeed SSH that is refusing to exit.
>>
>> i.e., after exiting the emacsclient frame, I am returned to the remote
>> SSH shell.  Pressing control-d in that shell does not return me back to
>> my local shell (the shell from which I SSH'd).  I have to press
>> control-c and then I am returned back to my local shell.
>>
>> Nothing emacs-related is hanging; the emacs daemon continues running,
>> and I can re-SSH and re-run emacsclient.  So the control-c to exit the
>> SSH session does not seem to destabilize the emacs daemon.  (However, I
>> suspect the control-c is severing the X display connection which seems
>> less safe and potentially more subject to race conditions than Emacs
>> itself closing the X connection.)
>>
>> There are so many use cases for emacsclient that any patch seems risky,
>> which is why I filed this bug report with something that works for me.
>>
>> If there is a comprehensive set of emacsclient connection/disconnection
>> tests, and/or any verbosity/debugging options that might help, then I
>> can run them by hand on my setup.  I did try:
>>
>> 1. local X11 "emacsclient -c -s test"
>> 2. local X11 "emacsclient -nw -s test"
>> 3. remote (over ssh -X) "emacsclient -c -s test"
>> 4. remote (over ssh -X) "emacsclient -nw -s test"
>>
>> against "emacs -Q --fg-daemon=test".  In each case, with this patch, the
>> key sequence C-x 5 0 exits the frame without issue, and (for tests 3 and
>> 4) C-d exits the SSH session (returns me to my local shell) without my
>> having to press C-c.  Without my patch, test 3 does result in SSH
>> refusing to exit.
>>
>> And, even with my patch, for test case 3, using C-x C-c to delete the
>> frame still results in SSH refusing to exit.  (It would be nice to fix
>> this case too, but I always use C-x 5 0, so I wanted to start by trying
>> to fix it.)
>>
>> Thomas
>
> I suggest configuring Emacs with --with-x-toolkit=no (as is recommended
> for users who connect to remote display servers to begin with).

That works, both C-x C-c and C-x 5 0 exit the emacsclient frame, and SSH
does not hang/refuse to exit in either case.  I will use
--with-x-toolkit=no from now on, thank you!

Somehow I missed that recommendation (maybe you can point me to it?).  I
have always avoided GTK for "emacsclient reasons" but I thought Lucid
would be OK.  Can I add something like the attached to etc/PROBLEMS?

Thanks,
Thomas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacsclient-ssh-hang-problems.patch --]
[-- Type: text/x-diff, Size: 1117 bytes --]

diff --git a/etc/PROBLEMS b/etc/PROBLEMS
index 7d7c9ab5f27..d0d529adb3c 100644
--- a/etc/PROBLEMS
+++ b/etc/PROBLEMS
@@ -178,6 +178,21 @@ The relevant bug report is here:
 A workaround is to set XLIB_SKIP_ARGB_VISUALS=1 in the environment
 before starting Emacs, or run Emacs as root.
 
+** emacsclient -c causes an ssh -X session to hang on exit.
+
+When Emacs is configured with an X toolkit, for example,
+--with-x-toolkit=lucid, and emacsclient is run over an "ssh -X"
+connection, deleting the emacsclient frame (via C-x 5 0 or C-x C-c) can
+leave the X display connection open.
+
+The symptom is that after you delete the "emacsclient -c" frame, and
+then attempt to exit ssh (with C-d, or "exit"), ssh will hang before
+returning you to the local shell.  You will have to press C-c (to sever
+the X connection) before ssh returns you to your local shell (that is,
+the shell in which you invoked "ssh -X").
+
+To avoid this issue configure Emacs with --with-x-toolkit=no.
+
 ** Emacs built with xwidgets aborts when displaying WebKit xwidgets
 
 This happens, for example, when 'M-x xwidget-webkit-browse-url'

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Eli Zaretskii]

> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 72517@debbugs.gnu.org
> Date: Thu, 08 Aug 2024 06:09:05 -0400
> 
> Can I add something like the attached to etc/PROBLEMS?

SGTM, just be sure to mention this bug in the log message when you
install this.

Thanks.

bug#72517: 31.0.50; [PATCH] Close X connection upon deletion of last emacsclient frame

[Thomas Fitzsimmons]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 72517@debbugs.gnu.org
>> Date: Thu, 08 Aug 2024 06:09:05 -0400
>> 
>> Can I add something like the attached to etc/PROBLEMS?
>
> SGTM, just be sure to mention this bug in the log message when you
> install this.

Done.

Thanks,
Thomas