Re: [PATCH v4] Add systemd socket launching support

[Matthew Leach]

Hi Stefan,

I've been thinking about what you said regarding reducing the coupling
this set of patches has to systemd.  I'm thinking that having a C
(non-lisp) function that can be called to set the FD that has been
passed to Emacs could work.  That way, we provide an interface for other
socket-passing libraries to call.

What are your thoughts?

Matthew Leach <address@hidden> writes:

> Hi all,
>
> Here is a new version of the systemd integration patches.  Main changes
> include, documentation updates (document that the :use-systemd-socket
> option may be silently ignored and include systemd units in the Emacs
> manual), and rename config.h macro to HAVE_LIBSYSTEMD.
>
> Feedback & comments welcome!
>
> Thanks,
> -- 
> Matt
>
> From d5972913f8c6ed8b4de6054e14395a373f2ee96e Mon Sep 17 00:00:00 2001
> From: Matthew Leach <address@hidden>
> Date: Sat, 26 Mar 2016 16:41:17 +0000
> Subject: [PATCH 1/4] Check for libsystemd when building Emacs.
>
> * configure.ac: Add new default-on option systemd and check for
>   libsystemd at configure time.
> * src/Makefile.in: Add libsystemd library and C flags to the Emacs
>   compilation options.
> ---
>  configure.ac    | 13 +++++++++++++
>  src/Makefile.in |  6 +++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index f3846f4..f53c670 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -330,6 +330,7 @@ OPTION_DEFAULT_ON([tiff],[don't compile with TIFF image 
> support])
>  OPTION_DEFAULT_ON([gif],[don't compile with GIF image support])
>  OPTION_DEFAULT_ON([png],[don't compile with PNG image support])
>  OPTION_DEFAULT_ON([rsvg],[don't compile with SVG image support])
> +OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
>  OPTION_DEFAULT_OFF([cairo],[compile with Cairo drawing (experimental)])
>  OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
>  OPTION_DEFAULT_ON([imagemagick],[don't compile with ImageMagick image 
> support])
> @@ -2716,6 +2717,18 @@ fi
>  AC_SUBST(LIBGNUTLS_LIBS)
>  AC_SUBST(LIBGNUTLS_CFLAGS)
>  
> +HAVE_LIBSYSTEMD=no
> +if test "${with_libsystemd}" = "yes" ; then
> +  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 226],
> +    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
> +  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
> +    AC_DEFINE(HAVE_LIBSYSTEMD, 1, [Define if using libsystemd.])
> +  fi
> +fi
> +
> +AC_SUBST(LIBSYSTEMD_LIBS)
> +AC_SUBST(LIBSYSTEMD_CFLAGS)
> +
>  NOTIFY_OBJ=
>  NOTIFY_SUMMARY=no
>  
> diff --git a/src/Makefile.in b/src/Makefile.in
> index c290a60..fc9360a 100644
> --- a/src/Makefile.in
> +++ b/src/Makefile.in
> @@ -307,6 +307,9 @@ LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
>  LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
>  LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
>  
> +LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
> +LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
> +
>  INTERVALS_H = dispextern.h intervals.h composite.h
>  
>  GETLOADAVG_LIBS = @GETLOADAVG_LIBS@
> @@ -372,6 +375,7 @@ ALL_CFLAGS=-Demacs $(MYCPPFLAGS) -I. -I$(srcdir) \
>    $(WEBKIT_CFLAGS) \
>    $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
>    $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
> +  $(LIBSYSTEMD_CFLAGS) \
>    $(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
>    $(WARN_CFLAGS) $(WERROR_CFLAGS) $(CFLAGS)
>  ALL_OBJC_CFLAGS=$(ALL_CFLAGS) $(GNU_OBJC_CFLAGS)
> @@ -489,7 +493,7 @@ LIBES = $(LIBS) $(W32_LIBS) $(LIBS_GNUSTEP) $(LIBX_BASE) 
> $(LIBIMAGE) \
>     $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
>     $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
>     $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) \
> -   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES)
> +   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS)
>  
>  $(leimdir)/leim-list.el: bootstrap-emacs$(EXEEXT)
>       $(MAKE) -C ../leim leim-list.el EMACS="$(bootstrap_exe)"
> -- 
> 2.7.4
>
> From 5f5ba1ce322a0c4b5ff7f2c7eb939928aec799c7 Mon Sep 17 00:00:00 2001
> From: Matthew Leach <address@hidden>
> Date: Sat, 26 Mar 2016 18:50:14 +0000
> Subject: [PATCH 2/4] Read the number of sockets passed by systemd.
>
> * src/emacs.c (systemd_socket): New variable for storing the socket
>   descriptor passed in by systemd.
>   (main): Call sd_listen_fds to read the number of sockets passed.
> ---
>  src/emacs.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/src/emacs.c b/src/emacs.c
> index 95d1905..67906e8 100644
> --- a/src/emacs.c
> +++ b/src/emacs.c
> @@ -56,6 +56,11 @@ along with GNU Emacs.  If not, see 
> <http://www.gnu.org/licenses/>.  */
>  #include <binary-io.h>
>  #endif
>  
> +#ifdef HAVE_LIBSYSTEMD
> +#include <systemd/sd-daemon.h>
> +#include <sys/socket.h>
> +#endif /* HAVE_LIBSYSTEMD */
> +
>  #ifdef HAVE_WINDOW_SYSTEM
>  #include TERM_HEADER
>  #endif /* HAVE_WINDOW_SYSTEM */
> @@ -195,6 +200,12 @@ int daemon_pipe[2];
>  HANDLE w32_daemon_event;
>  #endif
>  
> +#ifdef HAVE_LIBSYSTEMD
> +/* The socket descriptor passed by systemd.  If nothing has been
> +   passed, this will be 0. */
> +int systemd_socket = 0;
> +#endif /* HAVE_LIBSYSTEMD */
> +
>  /* Save argv and argc.  */
>  char **initial_argv;
>  int initial_argc;
> @@ -997,6 +1008,24 @@ main (int argc, char **argv)
>         exit (1);
>       }
>  
> +#ifdef HAVE_LIBSYSTEMD
> +      /* Read the number of sockets passed through by systemd. */
> +      systemd_socket = sd_listen_fds(1);
> +
> +      if (systemd_socket > 1)
> +        {
> +          fprintf (stderr, "\nWarning: systemd has passed more than one 
> socket to the Emacs process.\n\
> +Try adding 'Accept=false' in the Emacs socket unit file.\n");
> +          systemd_socket = 0;
> +        }
> +      else if (systemd_socket == 1 &&
> +               sd_is_socket (SD_LISTEN_FDS_START,
> +                             AF_UNSPEC, SOCK_STREAM, 1) >= 0)
> +        systemd_socket = SD_LISTEN_FDS_START;
> +      else
> +        systemd_socket = 0;
> +#endif /* HAVE_LIBSYSTEMD */
> +
>  #ifndef DAEMON_MUST_EXEC
>  #ifdef USE_GTK
>        fprintf (stderr, "\nWarning: due to a long standing Gtk+ 
> bug\nhttp://bugzilla.gnome.org/show_bug.cgi?id=85715\n\
> -- 
> 2.7.4
>
> From 2ed8471b2c6157e5639570d865db98e9fe33fb36 Mon Sep 17 00:00:00 2001
> From: Matthew Leach <address@hidden>
> Date: Sat, 26 Mar 2016 19:37:10 +0000
> Subject: [PATCH 3/4] Allow network processes to be made with a pre-allocated
>  fd.
>
> * src/process.c (connect_network_socket): Allow a pre-allocated
> socket descriptor to be used if passed to Emacs, avoiding the call
> to socket() and bind().
> (Fmake_network_process): Allow users to pass in :use-systemd-socket on
> the parameter plist to use a socket descriptor that has been passed to
> Emacs from systemd.
> (wait_reading_process_output): Call socket() & bind() every time.
> (syms_of_process): New symbol ":use-passed-socket".
> * doc/lispref/processes.texi (Network Processes): Document new
>   `make-network-process' option ':use-systemd-socket'.
> ---
>  doc/lispref/processes.texi |  7 +++++++
>  src/process.c              | 39 ++++++++++++++++++++++++++++++++-------
>  2 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
> index 8d3df55..83c1aa2 100644
> --- a/doc/lispref/processes.texi
> +++ b/doc/lispref/processes.texi
> @@ -2367,6 +2367,13 @@ automatically for the given @var{host} and 
> @var{service}.
>  ignored.  @code{ipv4} and @code{ipv6} specify to use IPv4 and IPv6,
>  respectively.
>  
> address@hidden :use-systemd-socket @var{use-systemd-socket}
> +If @var{use-systemd-socket} is address@hidden use any sockets passed
> +by systemd instead of allocating one.  This is used by the Emacs
> +server code to allow on-demand socket activation.  If Emacs hasn't
> +been compiled with libsystemd support or systemd didn't pass a socket,
> +this option is silently ignored.
> +
>  @item :local @var{local-address}
>  For a server process, @var{local-address} is the address to listen on.
>  It overrides @var{family}, @var{host} and @var{service}, so you
> diff --git a/src/process.c b/src/process.c
> index 198e7de..85ffd25 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -3075,7 +3075,8 @@ finish_after_tls_connection (Lisp_Object proc)
>  #endif
>  
>  static void
> -connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses)
> +connect_network_socket (Lisp_Object proc, Lisp_Object ip_addresses,
> +                        Lisp_Object use_systemd_socket_p)
>  {
>    ptrdiff_t count = SPECPDL_INDEX ();
>    ptrdiff_t count1;
> @@ -3089,6 +3090,16 @@ connect_network_socket (Lisp_Object proc, Lisp_Object 
> ip_addresses)
>    struct Lisp_Process *p = XPROCESS (proc);
>    Lisp_Object contact = p->childp;
>    int optbits = 0;
> +  int systemd_socket_descriptor = 0;
> +
> +#ifdef HAVE_LIBSYSTEMD
> +  if (!NILP (use_systemd_socket_p))
> +    {
> +      extern int systemd_socket;
> +      systemd_socket_descriptor = systemd_socket;
> +    }
> +#endif /* HAVE_LIBSYSTEMD */
> +
>  
>    /* Do this in case we never enter the while-loop below.  */
>    count1 = SPECPDL_INDEX ();
> @@ -3109,7 +3120,11 @@ connect_network_socket (Lisp_Object proc, Lisp_Object 
> ip_addresses)
>        sa = xmalloc (addrlen);
>        conv_lisp_to_sockaddr (family, ip_address, sa, addrlen);
>  
> -      s = socket (family, p->socktype | SOCK_CLOEXEC, p->ai_protocol);
> +      if (systemd_socket_descriptor)
> +          s = systemd_socket_descriptor;
> +      else
> +          s = socket (family, p->socktype | SOCK_CLOEXEC, p->ai_protocol);
> +
>        if (s < 0)
>       {
>         xerrno = errno;
> @@ -3168,8 +3183,11 @@ connect_network_socket (Lisp_Object proc, Lisp_Object 
> ip_addresses)
>                 report_file_error ("Cannot set reuse option on server 
> socket", Qnil);
>             }
>  
> -       if (bind (s, sa, addrlen))
> -         report_file_error ("Cannot bind server socket", Qnil);
> +          /* If we are passed a socket descriptor from systemd, it is
> +             already bound. */
> +       if (!systemd_socket_descriptor)
> +         if (bind (s, sa, addrlen))
> +           report_file_error ("Cannot bind server socket", Qnil);
>  
>  #ifdef HAVE_GETSOCKNAME
>         if (p->port == 0)
> @@ -3534,6 +3552,11 @@ The following network options can be specified for 
> this connection:
>                        (this is allowed by default for a server process).
>  :bindtodevice NAME -- bind to interface NAME.  Using this may require
>                        special privileges on some systems.
> +:use-systemd-socket BOOL -- Use any pre-allocated sockets that have
> +                            been passed to Emacs by systemd.  If Emacs
> +                            hasn't been compiled with libsystemd support
> +                            or systemd didn't pass a socket, this option
> +                            is silently ignored.
>  
>  Consult the relevant system programmer's manual pages for more
>  information on using these options.
> @@ -3578,7 +3601,7 @@ usage: (make-network-process &rest ARGS)  */)
>    EMACS_INT port = 0;
>    Lisp_Object tem;
>    Lisp_Object name, buffer, host, service, address;
> -  Lisp_Object filter, sentinel;
> +  Lisp_Object filter, sentinel, use_systemd_socket_p;
>    Lisp_Object ip_addresses = Qnil;
>    int socktype;
>    int family = -1;
> @@ -3618,6 +3641,7 @@ usage: (make-network-process &rest ARGS)  */)
>    buffer = Fplist_get (contact, QCbuffer);
>    filter = Fplist_get (contact, QCfilter);
>    sentinel = Fplist_get (contact, QCsentinel);
> +  use_systemd_socket_p = Fplist_get (contact, QCuse_systemd_socket);
>  
>    CHECK_STRING (name);
>  
> @@ -3914,7 +3938,7 @@ usage: (make-network-process &rest ARGS)  */)
>      }
>  #endif
>  
> -  connect_network_socket (proc, ip_addresses);
> +  connect_network_socket (proc, ip_addresses, use_systemd_socket_p);
>    return proc;
>  }
>  
> @@ -4848,7 +4872,7 @@ wait_reading_process_output (intmax_t time_limit, int 
> nsecs, int read_kbd,
>                 {
>                   Lisp_Object ip_addresses = check_for_dns (aproc);
>                   if (!NILP (ip_addresses) && !EQ (ip_addresses, Qt))
> -                   connect_network_socket (aproc, ip_addresses);
> +                   connect_network_socket (aproc, ip_addresses, Qnil);
>                   else
>                     retry_for_async = true;
>                 }
> @@ -7837,6 +7861,7 @@ syms_of_process (void)
>    DEFSYM (QCserver, ":server");
>    DEFSYM (QCnowait, ":nowait");
>    DEFSYM (QCsentinel, ":sentinel");
> +  DEFSYM (QCuse_systemd_socket, ":use-systemd-socket");
>    DEFSYM (QCtls_parameters, ":tls-parameters");
>    DEFSYM (Qnsm_verify_connection, "nsm-verify-connection");
>    DEFSYM (QClog, ":log");
> -- 
> 2.7.4
>
> From a09250b5c53c8da1157135f1ace2ecd4748d6510 Mon Sep 17 00:00:00 2001
> From: Matthew Leach <address@hidden>
> Date: Sat, 26 Mar 2016 20:43:26 +0000
> Subject: [PATCH 4/4] When Emacs is passed a socket descriptor, make the server
>  listen on it.
>
> * lisp/server.el (server-start): Set :use-systemd-socket to `t' when
>   calling `make-network-process'.
> * etc/NEWS: Document new systemd functionality and build option to
>   disable it.
> * doc/emacs/misc.texi (Emacs Server): Document systemd socket passing
>   functionality and provide systemd unit examples.
> ---
>  doc/emacs/misc.texi | 37 +++++++++++++++++++++++++++++++++++++
>  etc/NEWS            |  7 +++++++
>  lisp/server.el      |  1 +
>  3 files changed, 45 insertions(+)
>
> diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
> index b5a2150..2622592 100644
> --- a/doc/emacs/misc.texi
> +++ b/doc/emacs/misc.texi
> @@ -1580,6 +1580,43 @@ option.  @xref{Initial Options}.  When Emacs is 
> started this way, it
>  calls @code{server-start} after initialization, and returns control to
>  the calling terminal instead of opening an initial frame; it then
>  waits in the background, listening for edit requests.
> +
> address@hidden socket activation, systemd, Emacs
> address@hidden
> +If Emacs has been built with @command{systemd} support, the Emacs
> +server can be started by socket activation.  The @command{systemd}
> +service creates a socket and listens for connections on it; when
> address@hidden connects to it for the first time,
> address@hidden can launch the Emacs server and hands over the
> +socket to it for servicing @command{emacsclient} connections.  A setup
> +to use this functionality could be:
> +
> address@hidden/.config/systemd/user/emacs.service}:
> address@hidden
> +[Unit]
> +Description=Emacs
> +
> +[Service]
> +Type=forking
> +ExecStart=/path/to/emacs --daemon
> +ExecStop=/path/to/emacsclient --eval "(kill-emacs)"
> +Restart=always
> +
> +[Install]
> +WantedBy=default.target
> address@hidden example
> +
> address@hidden/.config/systemd/user/emacs.socket}:
> address@hidden
> +[Socket]
> +ListenStream=/path/to/.emacs.socket
> +
> +[Install]
> +WantedBy=sockets.target
> address@hidden example
> +
> +The @code{ListenStream} path will be the path that Emacs listens for
> +connections from @command{emacsclient}; this is a file of your choice.
>  @end itemize
>  
>  @cindex @env{TEXEDIT} environment variable
> diff --git a/etc/NEWS b/etc/NEWS
> index 66777e9..9afed5b 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -26,6 +26,13 @@ otherwise leave it unmarked.
>  * Installation Changes in Emacs 25.2
>  
>  +++
> +** Emacs now has systemd support.  This allows socket based
> +activation, where systemd can invoke the Emacs process upon a
> +socket connection and hand over the socket to Emacs.  Emacs will
> +use this socket for communicating with the emacsclient program.
> +This can be disabled with the new configure option
> +'--disable-systemd'.
> +
>  ** New configure option '--disable-build-details' attempts to build an
>  Emacs that is more likely to be reproducible; that is, if you build
>  and install Emacs twice, the second Emacs is a copy of the first.
> diff --git a/lisp/server.el b/lisp/server.el
> index 5243820..2c9113c 100644
> --- a/lisp/server.el
> +++ b/lisp/server.el
> @@ -655,6 +655,7 @@ server or call `\\[server-force-delete]' to forcibly 
> disconnect it."))
>                      :noquery t
>                      :sentinel #'server-sentinel
>                      :filter #'server-process-filter
> +                    :use-systemd-socket t
>                      ;; We must receive file names without being decoded.
>                      ;; Those are decoded by server-process-filter according
>                      ;; to file-name-coding-system.  Also don't get

-- 
Matt