When run with --fg-daemon mode (as done by the provided systemd unit file "emacs.service"), emacs will call sd_notify() on startup to notify systemd about its status. On exit, emacs will call sd_notify() unconditionally, i.e., even when not run in daemon mode. This means that when the daemon is running, other emacs processes derived from the main process (e.g., in batch mode started by the async package) may send exit notifications when they're exiting, even though the main daemon process is not exiting. systemd will (correctly) ignore these notifications because they come from the wrong PID; this is the lucky default value of "NotifyAccess=main" in the systemd unit, i.e., only the main PID may send notifications. However, systemd will also emit a warning in this case. With my config, this results in a lot of warning messages in the systemd user journal: Jan 20 14:23:06 systemd[1309]: emacs.service: Got notification message from PID 271936, but reception only permitted for main PID 1365 Jan 20 14:24:06 systemd[1309]: emacs.service: Got notification message from PID 271956, but reception only permitted for main PID 1365 Jan 20 14:25:07 systemd[1309]: emacs.service: Got notification message from PID 271983, but reception only permitted for main PID 1365 Jan 20 14:26:06 systemd[1309]: emacs.service: Got notification message from PID 272071, but reception only permitted for main PID 1365 Jan 20 14:27:06 systemd[1309]: emacs.service: Got notification message from PID 272092, but reception only permitted for main PID 1365 Jan 20 14:28:06 systemd[1309]: emacs.service: Got notification message from PID 272127, but reception only permitted for main PID 1365 Jan 20 14:29:06 systemd[1309]: emacs.service: Got notification message from PID 272304, but reception only permitted for main PID 1365 Jan 20 14:30:06 systemd[1309]: emacs.service: Got notification message from PID 272442, but reception only permitted for main PID 1365 I admit that this config may be exotic but the core of the issue is not the number of messages, it's simply that there shouldn't be any notification (nor warning). So I think the right thing to do is to call sd_notify in kill-emacs only when we run in foreground daemon mode, i.e., if daemon_type == 1. This is done when calling sd_notify in daemon-initialized. I assume the check was omitted in kill-emacs because daemon-initialized sets daemon_type = -1, so this information is no longer accessible when kill-emacs runs (and it was believed sd-notify is a no-op anyway in those cases -- but that's only partly true due to warning message). In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, cairo version 1.17.3) of 2020-08-28 built on juergen Windowing system distributor 'The X.Org Foundation', version 11.0.12010000 System Description: Arch Linux Configured using: 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-wide-int --with-modules --with-cairo --with-harfbuzz 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now' Configured features: XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP
> From: Tim Ruffing <public@timruffing.de>
> Date: Thu, 21 Jan 2021 17:26:31 +0100
>
> When run with --fg-daemon mode (as done by the provided systemd unit
> file "emacs.service"), emacs will call sd_notify() on startup to notify
> systemd about its status. On exit, emacs will call sd_notify()
> unconditionally, i.e., even when not run in daemon mode.
>
> This means that when the daemon is running, other emacs processes
> derived from the main process (e.g., in batch mode started by the async
> package) may send exit notifications when they're exiting, even though
> the main daemon process is not exiting. systemd will (correctly) ignore
> these notifications because they come from the wrong PID; this is the
> lucky default value of "NotifyAccess=main" in the systemd unit, i.e.,
> only the main PID may send notifications. However, systemd will also
> emit a warning in this case. With my config, this results in a lot of
> warning messages in the systemd user journal:
I'm not an expert on systemd, not even close, but please review the
reasoning for what we do in bug#31498, and let's take it from there.
Quote:
Both of these calls are successful no-ops if emacs was not started
by systemd.
CC'ing Lucas, who submitted that changeset, in case hoe would like to
comment.
Hm, I wasn’t aware systemd logged those warnings. Probably a good idea to guard the sd_notify(0, "STOPPING=1") with a guard on (a stashed copy of?) daemon_type, then, yes.
> Cc: 46022@debbugs.gnu.org > From: Lucas Werkmeister <mail@lucaswerkmeister.de> > Date: Thu, 21 Jan 2021 21:12:03 +0100 > > Hm, I wasn’t aware systemd logged those warnings. Probably a good idea > to guard the sd_notify(0, "STOPPING=1") with a guard on (a stashed copy > of?) daemon_type, then, yes. OK, thanks. Does the patch below seems reasonable and give good results? If yes, I'd like it to go into the upcoming Emacs 27.2. diff --git a/src/emacs.c b/src/emacs.c index f2e858f..67220eb 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -187,7 +187,8 @@ #define MAIN_PROGRAM /* Name for the server started by the daemon.*/ static char *daemon_name; -/* 0 not a daemon, 1 new-style (foreground), 2 old-style (background). */ +/* 0 not a daemon, 1 new-style (foreground), 2 old-style (background). + A negative value means the daemon initialization was already done. */ int daemon_type; #ifndef WINDOWSNT @@ -2371,7 +2372,10 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 1, "P", int exit_code; #ifdef HAVE_LIBSYSTEMD - sd_notify(0, "STOPPING=1"); + /* Notify systemd we are shutting down, but only if we have notified + it about startup. */ + if (daemon_type == -1) + sd_notify(0, "STOPPING=1"); #endif /* HAVE_LIBSYSTEMD */ /* Fsignal calls emacs_abort () if it sees that waiting_for_input is @@ -2876,7 +2880,7 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0, } /* Set it to an invalid value so we know we've already run this function. */ - daemon_type = -1; + daemon_type = -daemon_type; #else /* WINDOWSNT */ /* Signal the waiting emacsclient process. */
On 22.01.21 09:19, Eli Zaretskii wrote:
>> Cc: 46022@debbugs.gnu.org
>> From: Lucas Werkmeister <mail@lucaswerkmeister.de>
>> Date: Thu, 21 Jan 2021 21:12:03 +0100
>>
>> Hm, I wasn’t aware systemd logged those warnings. Probably a good idea
>> to guard the sd_notify(0, "STOPPING=1") with a guard on (a stashed copy
>> of?) daemon_type, then, yes.
>
> OK, thanks. Does the patch below seems reasonable and give good
> results? If yes, I'd like it to go into the upcoming Emacs 27.2.
>
> diff --git a/src/emacs.c b/src/emacs.c
> index f2e858f..67220eb 100644
> --- a/src/emacs.c
> +++ b/src/emacs.c
> @@ -187,7 +187,8 @@ #define MAIN_PROGRAM
> /* Name for the server started by the daemon.*/
> static char *daemon_name;
>
> -/* 0 not a daemon, 1 new-style (foreground), 2 old-style (background). */
> +/* 0 not a daemon, 1 new-style (foreground), 2 old-style (background).
> + A negative value means the daemon initialization was already done. */
> int daemon_type;
>
> #ifndef WINDOWSNT
> @@ -2371,7 +2372,10 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 1, "P",
> int exit_code;
>
> #ifdef HAVE_LIBSYSTEMD
> - sd_notify(0, "STOPPING=1");
> + /* Notify systemd we are shutting down, but only if we have notified
> + it about startup. */
> + if (daemon_type == -1)
> + sd_notify(0, "STOPPING=1");
> #endif /* HAVE_LIBSYSTEMD */
>
> /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
> @@ -2876,7 +2880,7 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
> }
>
> /* Set it to an invalid value so we know we've already run this function. */
> - daemon_type = -1;
> + daemon_type = -daemon_type;
>
> #else /* WINDOWSNT */
> /* Signal the waiting emacsclient process. */
>
That looks good to me. If daemon_type is 0, it will now stay at 0
instead of being marked as invalid, but that should be okay, since
daemon-initialized has no special code for daemon_type == 0.
> Cc: public@timruffing.de, 46022@debbugs.gnu.org
> From: Lucas Werkmeister <mail@lucaswerkmeister.de>
> Date: Fri, 22 Jan 2021 10:47:49 +0100
>
> > - daemon_type = -1;
> > + daemon_type = -daemon_type;
> >
> > #else /* WINDOWSNT */
> > /* Signal the waiting emacsclient process. */
> >
>
> That looks good to me. If daemon_type is 0, it will now stay at 0
> instead of being marked as invalid, but that should be okay, since
> daemon-initialized has no special code for daemon_type == 0.
Right.
Tim, does this work for you? Can you perhaps test this on your
system?
I tested the patch and I can conform that it works and makes the
warnings disappear.
Thanks for the quick response!
Tim
On Fri, 2021-01-22 at 13:45 +0200, Eli Zaretskii wrote:
> > Cc: public@timruffing.de, 46022@debbugs.gnu.org
> > From: Lucas Werkmeister <mail@lucaswerkmeister.de>
> > Date: Fri, 22 Jan 2021 10:47:49 +0100
> >
> > > - daemon_type = -1;
> > > + daemon_type = -daemon_type;
> > >
> > > #else /* WINDOWSNT */
> > > /* Signal the waiting emacsclient process. */
> > >
> >
> > That looks good to me. If daemon_type is 0, it will now stay at 0
> > instead of being marked as invalid, but that should be okay, since
> > daemon-initialized has no special code for daemon_type == 0.
>
> Right.
>
> Tim, does this work for you? Can you perhaps test this on your
> system?
> From: Tim Ruffing <public@timruffing.de>
> Cc: 46022@debbugs.gnu.org
> Date: Fri, 22 Jan 2021 13:07:35 +0100
>
> I tested the patch and I can conform that it works and makes the
> warnings disappear.
Thanks, I've now installed the change on the emacs-27 branch, and I'm
closing this bug report.