bug#51377: Automatically exit server when it has no remaining clients

[Jim Porter <jporterbugs@gmail.com>]

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

On 10/24/2021 11:43 AM, Gregory Heytings wrote:
>>
>> If `save-buffers-kill-emacs' were called after that, the Emacs daemon 
>> would be killed, losing the edits to bar.txt.
> 
> Indeed, you are correct.  Updated patch attached, which also takes care 
> of running processes.

Yeah, that looks like it should avoid any data loss. However, it's not 
the behavior I'd personally expect. As I understand it, this will just 
keep the Emacs daemon alive if there are any unsaved files. I'd find it 
easier to use if Emacs warned the user about unsaved files before 
killing the last client. Then, Emacs can safely kill the daemon once 
I've confirmed that that's what I want.

I've attached a lightly-tested patch series that implements things in 
the way that I'd be happiest with. There are a few points I should 
explain though.

First, I chose to add a new --lazy-daemon option (I'm not sure about the 
name; feel free to suggest a better one). This makes it possible to 
distinguish whether the daemon was started on demand via `emacsclient'. 
I chose a command-line option rather than a command accepted by the 
server protocol because all the other server protocol commands can be 
used any time a new `emacsclient' is started; this option only makes 
sense when starting the daemon (since it describes how the daemon was 
started). I also chose to add this as a "third" daemon type rather than 
an entirely separate flag since I don't think there's any reason to 
support a lazily-created foreground daemon.

In the second patch, I added a minor performance optimization to 
`server-kill-emacs-query-function'. Now it stops once it finds the first 
live client. This patch isn't strictly required though, and I could 
rework things to exclude it.

Next, I updated `server-save-buffers-kill-terminal' to check if the 
server is a lazy daemon and if the current client is the last one: if 
so, it calls `save-buffers-kill-emacs', which then calls an improved 
`server-kill-emacs-query-function' that knows to avoid prompting 
unnecessarily when this is the last client.

Notably, the change to `server-save-buffers-kill-terminal' alters the 
behavior of killing a frame created by `emacsclient -n'. Looking at the 
code before, I don't see how it could possibly have been the right 
behavior: if there were multiple frames open, it only closed *one* 
frame, even if all of those frames were created after a single call of 
`emacsclient -n' (e.g. by calling `make-frame-command'). If it was the 
*last* frame, it killed Emacs, even if the user explicitly called `emacs 
--daemon' on system boot and wants the daemon to live forever. I've 
changed the behavior so that when you call this from a `nowait' frame, 
it kills Emacs if a) it's a lazy daemon and b) there are no other 
clients. That's a lot closer to the behavior when calling this from a 
"normal" client frame.

Finally, these patches would all require documentation improvements to 
merge. However, it didn't seem like a good use of my time to do that 
until people agree that this is the right strategy overall.

[-- Attachment #2: 0001-Add-lazy-daemon-option-used-to-start-a-daemon-on-dem.patch --]
[-- Type: text/plain, Size: 6469 bytes --]

From ffa4fb0843a4db7a09519099e304e72cbffaf4b7 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 24 Oct 2021 10:42:28 -0700
Subject: [PATCH 1/3] Add --lazy-daemon option, used to start a daemon
 on-demand via emacsclient

src/emacs.c (main, Fdaemon_initialized): Handle '--lazy-daemon'.
(Fdaemon_type): New function.
(syms_of_emacs): Add Sdaemon_type.
lib-src/emacsclient.c (start_daemon_and_retry_set_socket):
Use '--lazy-daemon' when starting daemon.
---
 lib-src/emacsclient.c |  8 ++++----
 src/emacs.c           | 46 ++++++++++++++++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index cff3cec2a7..40ebfa0513 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1756,7 +1756,7 @@ start_daemon_and_retry_set_socket (void)
   else
     {
       char emacs[] = "emacs";
-      char daemon_option[] = "--daemon";
+      char daemon_option[] = "--lazy-daemon";
       char *d_argv[3];
       d_argv[0] = emacs;
       d_argv[1] = daemon_option;
@@ -1764,8 +1764,8 @@ start_daemon_and_retry_set_socket (void)
 # ifdef SOCKETS_IN_FILE_SYSTEM
       if (socket_name != NULL)
 	{
-	  /* Pass  --daemon=socket_name as argument.  */
-	  const char *deq = "--daemon=";
+	  /* Pass  --lazy-daemon=socket_name as argument.  */
+	  const char *deq = "--lazy-daemon=";
 	  char *daemon_arg = xmalloc (strlen (deq)
 				      + strlen (socket_name) + 1);
 	  strcpy (stpcpy (daemon_arg, deq), socket_name);
@@ -1790,7 +1790,7 @@ start_daemon_and_retry_set_socket (void)
      it is ready to accept client connections, by asserting an event
      whose name is known to the daemon (defined by nt/inc/ms-w32.h).  */
 
-  if (!CreateProcess (NULL, (LPSTR)"emacs --daemon", NULL, NULL, FALSE,
+  if (!CreateProcess (NULL, (LPSTR)"emacs --lazy-daemon", NULL, NULL, FALSE,
                       CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
     {
       char* msg = NULL;
diff --git a/src/emacs.c b/src/emacs.c
index a24543a586..e41b4abc49 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -245,6 +245,9 @@ #define MAIN_PROGRAM
 --chdir DIR                 change to directory DIR\n\
 --daemon, --bg-daemon[=NAME] start a (named) server in the background\n\
 --fg-daemon[=NAME]          start a (named) server in the foreground\n\
+--lazy-daemon[=NAME]        start a (named) server in the background\
+                              and record that it was started on-demand\n\
+                              by emacsclient\n\
 --debug-init                enable Emacs Lisp debugger for init file\n\
 --display, -d DISPLAY       use X server DISPLAY\n\
 ",
@@ -1635,12 +1638,17 @@ main (int argc, char **argv)
     {
       daemon_type = 2;          /* background */
     }
+  else if (argmatch (argv, argc, "-lazy-daemon", "--lazy-daemon", 12, NULL, &skip_args)
+      || argmatch (argv, argc, "-lazy-daemon", "--lazy-daemon", 12, &dname_arg, &skip_args))
+    {
+      daemon_type = 3;           /* background, lazy */
+    }
 
 
   if (daemon_type > 0)
     {
 #ifndef DOS_NT
-      if (daemon_type == 2)
+      if (daemon_type == 2 || daemon_type == 3)
         {
           /* Start as a background daemon: fork a new child process which
              will run the rest of the initialization code, then exit.
@@ -1668,7 +1676,7 @@ main (int argc, char **argv)
               fputs ("Cannot pipe!\n", stderr);
               exit (1);
             }
-        } /* daemon_type == 2 */
+        } /* daemon_type == 2 || daemon_type == 3 */
 
 #ifdef HAVE_LIBSYSTEMD
       /* Read the number of sockets passed through by systemd.  */
@@ -1692,7 +1700,7 @@ main (int argc, char **argv)
 	     stderr);
 #endif /* USE_GTK */
 
-      if (daemon_type == 2)
+      if (daemon_type == 2 || daemon_type == 3)
         {
           pid_t f;
 #ifndef DAEMON_MUST_EXEC
@@ -1747,8 +1755,9 @@ main (int argc, char **argv)
                 char fdStr[80];
                 int fdStrlen =
                   snprintf (fdStr, sizeof fdStr,
-                            "--bg-daemon=\n%d,%d\n%s", daemon_pipe[0],
-                            daemon_pipe[1], dname_arg ? dname_arg : "");
+                            "--%s-daemon=\n%d,%d\n%s", daemon_pipe[0],
+			    daemon_type == 2 ? "bg" : "lazy", daemon_pipe[1],
+			    dname_arg ? dname_arg : "");
 
                 if (! (0 <= fdStrlen && fdStrlen < sizeof fdStr))
                   {
@@ -1785,7 +1794,7 @@ main (int argc, char **argv)
           emacs_close (daemon_pipe[0]);
 
           setsid ();
-        } /* daemon_type == 2 */
+        } /* daemon_type == 2 || daemon_type == 3 */
 #elif defined(WINDOWSNT)
       /* Indicate that we want daemon mode.  */
       w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_DAEMON_EVENT);
@@ -2372,6 +2381,7 @@ main (int argc, char **argv)
   { "-daemon", "--daemon", 99, 0 },
   { "-bg-daemon", "--bg-daemon", 99, 0 },
   { "-fg-daemon", "--fg-daemon", 99, 0 },
+  { "-lazy-daemon", "--lazy-daemon", 99, 0 },
   { "-help", "--help", 90, 0 },
   { "-nl", "--no-loadup", 70, 0 },
   { "-nsl", "--no-site-lisp", 65, 0 },
@@ -3128,6 +3138,27 @@ DEFUN ("daemonp", Fdaemonp, Sdaemonp, 0, 0, 0,
     return Qnil;
 }
 
+DEFUN ("daemon-type", Fdaemon_type, Sdaemon_type, 0, 0, 0,
+       doc: /* If emacs was started as a daemon, return the type of daemon.
+The result is one of `foreground', `background', or `lazy'. */)
+  (void)
+{
+  switch (abs (daemon_type))
+    {
+    case 0:
+      return Qnil;
+    case 1:
+      return intern_c_string ("foreground");
+    case 2:
+      return intern_c_string ("background");
+    case 3:
+      return intern_c_string ("lazy");
+    default:
+      error ("Unrecognized daemon type");
+    }
+}
+
+
 DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
        doc: /* Mark the Emacs daemon as being initialized.
 This finishes the daemonization process by doing the other half of detaching
@@ -3153,7 +3184,7 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
 #endif /* HAVE_LIBSYSTEMD */
     }
 
-  if (daemon_type == 2)
+  if (daemon_type == 2 || daemon_type == 3)
     {
       int nfd;
 
@@ -3211,6 +3242,7 @@ syms_of_emacs (void)
   defsubr (&Sinvocation_name);
   defsubr (&Sinvocation_directory);
   defsubr (&Sdaemonp);
+  defsubr (&Sdaemon_type);
   defsubr (&Sdaemon_initialized);
 
   DEFVAR_LISP ("command-line-args", Vcommand_line_args,
-- 
2.25.1


[-- Attachment #3: 0002-Stop-searching-for-live-clients-once-we-find-one.patch --]
[-- Type: text/plain, Size: 1231 bytes --]

From df292aeaa73c01a7685ff34c9e0265df859d2d42 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 24 Oct 2021 11:21:23 -0700
Subject: [PATCH 2/3] Stop searching for live clients once we find one

lisp/server.el (server-kill-emacs-query-function): Use 'seq-some'
to search for live clients.
---
 lisp/server.el | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 5306a54776..5988560c83 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1581,12 +1581,10 @@ server-done
 
 (defun server-kill-emacs-query-function ()
   "Ask before exiting Emacs if it has live clients."
-  (or (not (let (live-client)
-             (dolist (proc server-clients)
-               (when (memq t (mapcar #'buffer-live-p
-                                     (process-get proc 'buffers)))
-                 (setq live-client t)))
-             live-client))
+  (or (not (seq-some (lambda (proc)
+                       (seq-some #'buffer-live-p
+                                 (process-get proc 'buffers)))
+                     server-clients))
       (yes-or-no-p "This Emacs session has clients; exit anyway? ")))
 
 (defun server-kill-buffer ()
-- 
2.25.1


[-- Attachment #4: 0003-When-killing-the-last-client-attached-to-a-lazy-daem.patch --]
[-- Type: text/plain, Size: 4190 bytes --]

From 26777b5fc13b15b33571617378001f9f0600f733 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 24 Oct 2021 12:11:37 -0700
Subject: [PATCH 3/3] When killing the last client attached to a lazy daemon,
 kill the daemon too

lisp/server.el (server-save-buffers-kill-terminal): Kill Emacs when
connected to a lazy daemon and there are no other clients.
(server-kill-emacs-query-function): Don't warn about killing Emacs
when not necessary.
---
 lisp/server.el | 61 +++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 5988560c83..0cbabba621 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1580,12 +1580,17 @@ server-done
     (server-buffer-done (current-buffer))))
 
 (defun server-kill-emacs-query-function ()
-  "Ask before exiting Emacs if it has live clients."
-  (or (not (seq-some (lambda (proc)
-                       (seq-some #'buffer-live-p
-                                 (process-get proc 'buffers)))
-                     server-clients))
-      (yes-or-no-p "This Emacs session has clients; exit anyway? ")))
+  "Ask before exiting Emacs if it has live clients.
+If Emacs was started as a lazy daemon and the only live client is the
+current frame's client, don't bother asking."
+  (let ((ignored-proc (and (eq (daemon-type) 'lazy)
+                           (frame-parameter nil 'client))))
+    (or (not (seq-some (lambda (proc)
+                         (unless (eq ignored-proc proc)
+                           (seq-some #'buffer-live-p
+                                     (process-get proc 'buffers))))
+                       server-clients))
+        (yes-or-no-p "This Emacs session has clients; exit anyway? "))))
 
 (defun server-kill-buffer ()
   "Remove the current buffer from its clients' buffer list.
@@ -1721,28 +1726,32 @@ server-save-buffers-kill-terminal
 With ARG non-nil, silently save all file-visiting buffers, then kill.
 
 If emacsclient was started with a list of filenames to edit, then
-only these files will be asked to be saved."
+only these files will be asked to be saved.
+
+If Emacs was started as a lazy daemon and this is the last client
+connected to it, this will call `save-buffers-kill-emacs'."
   (let ((proc (frame-parameter nil 'client)))
-    (cond ((eq proc 'nowait)
+    (unless (or (eq proc 'nowait) (processp proc))
+      (error "Invalid client frame"))
+    (if (and (eq (daemon-type) 'lazy)
+             (equal server-clients (unless (eq proc 'nowait) (list proc))))
+        ;; If we're the last client connected to a lazy daemon, kill Emacs.
+        (save-buffers-kill-emacs arg)
+      (if (eq proc 'nowait)
 	   ;; Nowait frames have no client buffer list.
-	   (if (cdr (frame-list))
-	       (progn (save-some-buffers arg)
-		      (delete-frame))
-	     ;; If we're the last frame standing, kill Emacs.
-	     (save-buffers-kill-emacs arg)))
-	  ((processp proc)
-	   (let ((buffers (process-get proc 'buffers)))
-	     (save-some-buffers
-	      arg (if buffers
-                      ;; Only files from emacsclient file list.
-		      (lambda () (memq (current-buffer) buffers))
-                    ;; No emacsclient file list: don't override
-                    ;; `save-some-buffers-default-predicate' (unless
-                    ;; ARG is non-nil), since we're not killing
-                    ;; Emacs (unlike `save-buffers-kill-emacs').
-		    (and arg t)))
-	     (server-delete-client proc)))
-	  (t (error "Invalid client frame")))))
+	  (progn (save-some-buffers arg)
+		 (delete-frame))
+        (let ((buffers (process-get proc 'buffers)))
+	  (save-some-buffers
+	   arg (if buffers
+                   ;; Only files from emacsclient file list.
+		   (lambda () (memq (current-buffer) buffers))
+                 ;; No emacsclient file list: don't override
+                 ;; `save-some-buffers-default-predicate' (unless ARG
+                 ;; is non-nil), since we're not killing Emacs (unlike
+                 ;; `save-buffers-kill-emacs').
+		 (and arg t)))
+	  (server-delete-client proc))))))
 
 (define-key ctl-x-map "#" 'server-edit)
 
-- 
2.25.1