From 76f8a18816388841d15efb73df7ac81135a41057 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sat, 28 Jun 2025 13:19:58 +0200 Subject: [PATCH] Fix #440: refactor systemd and s6 readiness notifcation This patch fixes issue #440 which details how systemd readiness notification has been broken since its inception in Finit. In addition to now providing a proper abstract socket, the refactor also drops the reliance on the Finit client API to fetch a socket. Now an s6 descriptor is the write end of a pipe() and the systemd socket is instead created before forking off the service. Signed-off-by: Joachim Wiberg --- src/api.c | 14 ------ src/client.c | 10 ----- src/finit.h | 1 - src/private.h | 1 - src/service.c | 122 +++++++++++++++++++++++++++++++++++++++++--------- src/svc.h | 2 +- 6 files changed, 102 insertions(+), 48 deletions(-) diff --git a/src/api.c b/src/api.c index 02987c17..a9fb0745 100644 --- a/src/api.c +++ b/src/api.c @@ -571,20 +571,6 @@ static void api_cb(uev_t *w, void *arg, int events) result = do_signal(rq.data, sizeof(rq.data), rq.runlevel); break; - case INIT_CMD_NOTIFY_SOCKET: - svc = svc_find_by_pid(rq.runlevel); - if (!svc) { - errx(1, "Unknown PID, cannot register notify socket"); - result = 1; - break; - } - - if (uev_io_init(ctx, &svc->notify_watcher, service_notify_cb, svc, sd, UEV_READ)) { - err(1, "Failed initializing %s readiness notifier", svc_ident(svc, NULL, 0)); - break; - } - return; /* Don't close sd, used for notify */ - default: dbg("Unsupported cmd: %d", rq.cmd); break; diff --git a/src/client.c b/src/client.c index a8c06ac4..426b406e 100644 --- a/src/client.c +++ b/src/client.c @@ -97,16 +97,6 @@ int client_request(struct init_request *rq, ssize_t len) return -1; } - /* - * Exception, only command that don't need to wait for a - * response from Finit. Registers the client socket as a - * readiness notification socket with Finit - */ - if (rq->cmd == INIT_CMD_NOTIFY_SOCKET) { - rq->cmd = INIT_CMD_ACK; - return 0; - } - pfd.fd = sd; pfd.events = POLLIN | POLLERR | POLLHUP; if ((rc = poll(&pfd, 1, REQUEST_TIMEOUT)) <= 0) { diff --git a/src/finit.h b/src/finit.h index 04ca5597..b65c8f81 100644 --- a/src/finit.h +++ b/src/finit.h @@ -99,7 +99,6 @@ #define INIT_CMD_SVC_FIND 131 #define INIT_CMD_SVC_FIND_BYC 132 #define INIT_CMD_SIGNAL 133 -#define INIT_CMD_NOTIFY_SOCKET 200 /* For readiness notification socket */ #define INIT_CMD_NACK 254 #define INIT_CMD_ACK 255 diff --git a/src/private.h b/src/private.h index 8f02f3b6..f1b6f37b 100644 --- a/src/private.h +++ b/src/private.h @@ -46,7 +46,6 @@ int api_exit (void); void conf_flush_events(void); void service_monitor (pid_t lost, int status); -void service_notify_cb(uev_t *w, void *arg, int events); const char *plugin_hook_str (hook_point_t no); int plugin_exists (hook_point_t no); diff --git a/src/service.c b/src/service.c index 30415f7b..a8a9d60c 100644 --- a/src/service.c +++ b/src/service.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #ifdef _LIBITE_LITE @@ -56,6 +57,9 @@ #include "utmp-api.h" #include "schedule.h" +#define NOTIFY_PATH "@run/finit/notify/%d" + + /* * run tasks block other tasks/services from starting, we track the * current run task here. For service_step() and service_start(). @@ -68,6 +72,8 @@ static struct wq work = { int service_interval = SERVICE_INTERVAL_DEFAULT; static void svc_set_state(svc_t *svc, svc_state_t new_state); +static void service_notify_cb(uev_t *w, void *arg, int events); + /** * service_timeout_cb - libuev callback wrapper for service timeouts @@ -582,6 +588,9 @@ static int service_start(svc_t *svc) int result = 0, do_progress = 1; char cmdline[CMD_SIZE] = ""; sigset_t nmask, omask; + int pipefd[2]; + int fd = -1; + int sd = -1; pid_t pid; size_t i; @@ -627,6 +636,29 @@ static int service_start(svc_t *svc) if (svc_is_sysv(svc)) logit(LOG_CONSOLE | LOG_NOTICE, "Calling '%s start' ...", cmdline); + switch (svc->notify) { + case SVC_NOTIFY_S6: + if (pipe(pipefd) == -1) { + err(1, "%s: failed opening pipe for s6 notify", svc_ident(svc, NULL, 0)); + svc_missing(svc); + return 1; + } + fd = pipefd[1]; + sd = pipefd[0]; + break; + + case SVC_NOTIFY_SYSTEMD: + sd = socket(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); + if (sd == -1) { + err(1, "%s: failed opening notify socket", svc_ident(svc, NULL, 0)); + svc_missing(svc); + return 1; + } + break; + default: + break; + } + if (!svc->desc[0]) do_progress = 0; @@ -653,38 +685,72 @@ static int service_start(svc_t *svc) pid = service_fork(svc); if (pid < 0) { + if (sd != -1) + close(sd); + if (fd != -1) + close(fd); result = -1; goto fail; } if (pid > 1) { + struct sockaddr_un sun; + size_t len; + dbg("Starting %s as PID %d", svc_ident(svc, NULL, 0), pid); svc->pid = pid; svc->start_time = jiffies(); + + switch (svc->notify) { + case SVC_NOTIFY_SYSTEMD: + memset(&sun, 0, sizeof(sun)); + sun.sun_family = AF_UNIX; + snprintf(sun.sun_path, sizeof(sun.sun_path), NOTIFY_PATH, pid); + len = strlen(sun.sun_path); + sun.sun_path[0] = 0; + result = bind(sd, (struct sockaddr *)&sun, offsetof(struct sockaddr_un, sun_path) + len); + if (result == -1) { + err(1, "%s: failed binding to notify socket", svc_ident(svc, NULL, 0)); + close(sd); + break; + } + /* fallthrough */ + case SVC_NOTIFY_S6: + close(fd); /* client-end of pipefd for s6 notify */ + result = uev_io_init(ctx, &svc->notify_watcher, service_notify_cb, svc, sd, UEV_READ); + if (result < 0) { + err(1, "%s: failed setting up notify callback", svc_ident(svc, NULL, 0)); + if (svc->notify == SVC_NOTIFY_S6) + close(fd); + close(sd); + break; + } + default: + break; + } } else if (pid == 0) { + char str[strlen(NOTIFY_PATH) + 32]; char *args[MAX_NUM_SVC_ARGS + 1]; int status; if (!svc_is_tty(svc)) redirect(svc); + switch (svc->notify) { + case SVC_NOTIFY_SYSTEMD: + snprintf(str, sizeof(str), NOTIFY_PATH, getpid()); + setenv("NOTIFY_SOCKET", str, 1); + /* fallthrough */ + case SVC_NOTIFY_S6: + close(sd); + break; + default: + break; + } + if (!svc_is_sysv(svc)) { wordexp_t we = { 0 }; int rc; - if (svc->notify == SVC_NOTIFY_SYSTEMD || svc->notify == SVC_NOTIFY_S6) { - if (client_command(INIT_CMD_NOTIFY_SOCKET)) { - err(1, "%s: failed creating notify socket", svc_ident(svc, NULL, 0)); - client_disconnect(); - /* does not change parent, restarting may work. */ - svc->notify = SVC_NOTIFY_NONE; - } else { - char val[20]; - - snprintf(val, sizeof(val), "%d", client_socket()); - setenv("NOTIFY_SOCKET", val, 1); - } - } - if ((rc = wordexp(svc->cmd, &we, 0))) { errx(1, "%s: failed wordexp(%s): %d", svc_ident(svc, NULL, 0), svc->cmd, rc); nomem: @@ -701,11 +767,11 @@ static int service_start(svc_t *svc) if (len == 0) break; - if (svc->notify == SVC_NOTIFY_SYSTEMD || svc->notify == SVC_NOTIFY_S6) { + if (svc->notify == SVC_NOTIFY_S6) { char *ptr = strstr(arg, "%n"); if (ptr) { - len = snprintf(str, sizeof(str), "%d", client_socket()); + len = snprintf(str, sizeof(str), "%d", fd); if (len > 0 && len <= 2) { ptr[0] = ' '; ptr[1] = ' '; @@ -2929,14 +2995,15 @@ void service_notify_reconf(void) * Called when a service sends readiness notification, or when * the service closes its end of the IPC connection. */ -void service_notify_cb(uev_t *w, void *arg, int events) +static void service_notify_cb(uev_t *w, void *arg, int events) { svc_t *svc = (svc_t *)arg; + int ready = 0; char buf[32]; ssize_t len; if (UEV_ERROR == events) { - dbg("Spurious problem with %s notify callback, restarting.", svc_ident(svc, NULL, 0)); + dbg("%s: spurious problem with notify callback, restarting.", svc_ident(svc, NULL, 0)); uev_io_start(w); return; } @@ -2946,11 +3013,24 @@ void service_notify_cb(uev_t *w, void *arg, int events) warn("Failed reading notification from %s", svc_ident(svc, NULL, 0)); return; } - buf[len] = 0; - /* systemd and s6, respectively. The latter then closes the socket */ - if (!strcmp(buf, "READY=1\n") || buf[len - 1] == '\n') { + /* Check for systemd READY=1 or s6 newline termination */ + if (svc->notify == SVC_NOTIFY_SYSTEMD) { + char *token = strtok(buf, "\n"); + + while (token) { + if (!strcmp(token, "READY=1")) { + ready = 1; + break; + } + token = strtok(NULL, "\n"); + } + } else if (svc->notify == SVC_NOTIFY_S6 && buf[len - 1] == '\n') { + ready = 1; + } + + if (ready) { /* * native (pidfile) services are marked as started by * the pidfile plugin. diff --git a/src/svc.h b/src/svc.h index 91b647ea..2b425f3a 100644 --- a/src/svc.h +++ b/src/svc.h @@ -85,7 +85,7 @@ typedef enum { SVC_ONCRASH_SCRIPT, } svc_oncrash_action_t; -/* 0: none, 1: systemd, 2: s6 */ +/* 0: none, 1: finit (native), 2: systemd, 3: s6 */ typedef enum { SVC_NOTIFY_NONE = 0, SVC_NOTIFY_PID,