From cb5a46b84504006196e4415266c1a6414c07c9b3 Mon Sep 17 00:00:00 2001 From: Kenny Levinsen Date: Wed, 8 Apr 2020 20:19:30 +0200 Subject: core: Add optional FDPOLL=0 argument to fdstore A service can specify FDSTORE=1 FDPOLL=0 to request that PID1 does not poll the fd to remove them on error. If set, fds will only be removed on FDSTOREREMOVE=1 or when the service is done. Fixes: #12086 --- man/sd_notify.xml | 14 ++++++++++++-- src/core/service.c | 26 ++++++++++++++------------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 3046ca88ee..0157ce864a 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -219,8 +219,8 @@ in a memfd_create2 memory file descriptor. Note that the service manager will accept messages for a service only if its FileDescriptorStoreMax= setting is non-zero (defaults to zero, see - systemd.service5). If file - descriptors sent are pollable (see + systemd.service5). If + FDPOLL=0 is not set and the file descriptors sent are pollable (see epoll_ctl2), then any EPOLLHUP or EPOLLERR event seen on them will result in their automatic removal from the store. Multiple arrays of file descriptors may be sent in separate messages, in @@ -251,6 +251,16 @@ submitted name does not follow these restrictions, it is ignored. + + FDPOLL=0 + + When used in combination with FDSTORE=1, disables polling of the stored + file descriptors regardless of whether or not they are pollable. As this option disables automatic cleanup + of the stored file descriptors on EPOLLERR and EPOLLHUP, care must be taken to ensure proper manual cleanup. + Use of this option is not generally recommended except for when automatic cleanup has unwanted behavior such + as prematurely discarding file descriptors from the store. + + It is recommended to prefix variable names that are not diff --git a/src/core/service.c b/src/core/service.c index 7d5928e455..cc8b44f1c8 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -423,7 +423,7 @@ static int on_fd_store_io(sd_event_source *e, int fd, uint32_t revents, void *us return 0; } -static int service_add_fd_store(Service *s, int fd, const char *name) { +static int service_add_fd_store(Service *s, int fd, const char *name, bool do_poll) { ServiceFDStore *fs; int r; @@ -459,13 +459,15 @@ static int service_add_fd_store(Service *s, int fd, const char *name) { return -ENOMEM; } - r = sd_event_add_io(UNIT(s)->manager->event, &fs->event_source, fd, 0, on_fd_store_io, fs); - if (r < 0 && r != -EPERM) { /* EPERM indicates fds that aren't pollable, which is OK */ - free(fs->fdname); - free(fs); - return r; - } else if (r >= 0) - (void) sd_event_source_set_description(fs->event_source, "service-fd-store"); + if (do_poll) { + r = sd_event_add_io(UNIT(s)->manager->event, &fs->event_source, fd, 0, on_fd_store_io, fs); + if (r < 0 && r != -EPERM) { /* EPERM indicates fds that aren't pollable, which is OK */ + free(fs->fdname); + free(fs); + return r; + } else if (r >= 0) + (void) sd_event_source_set_description(fs->event_source, "service-fd-store"); + } LIST_PREPEND(fd_store, s->fd_store, fs); s->n_fd_store++; @@ -473,7 +475,7 @@ static int service_add_fd_store(Service *s, int fd, const char *name) { return 1; /* fd newly stored */ } -static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name) { +static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name, bool do_poll) { int r; assert(s); @@ -485,7 +487,7 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name) { if (fd < 0) break; - r = service_add_fd_store(s, fd, name); + r = service_add_fd_store(s, fd, name, do_poll); if (r == -EXFULL) return log_unit_warning_errno(UNIT(s), r, "Cannot store more fds than FileDescriptorStoreMax=%u, closing remaining.", @@ -2961,7 +2963,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, fdn += strspn(fdn, WHITESPACE); (void) cunescape(fdn, 0, &t); - r = service_add_fd_store(s, fd, t); + r = service_add_fd_store(s, fd, t, true); if (r < 0) log_unit_error_errno(u, r, "Failed to add fd to store: %m"); else @@ -4068,7 +4070,7 @@ static void service_notify_message( name = NULL; } - (void) service_add_fd_store_set(s, fds, name); + (void) service_add_fd_store_set(s, fds, name, !strv_contains(tags, "FDPOLL=0")); } /* Notify clients about changed status or main pid */ -- cgit v1.2.1 From 30520492604bad0bb1e346b538119dcb37525d37 Mon Sep 17 00:00:00 2001 From: Kenny Levinsen Date: Thu, 9 Apr 2020 15:30:02 +0200 Subject: core: (De-)Serialize poll flag for fds in fdstore This replaces manual string splitting and unescaping with extract_first_word. --- src/core/service.c | 43 +++++++++++++++++++++++++------------------ src/core/service.h | 1 + 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index cc8b44f1c8..292df8d5e1 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -453,6 +453,7 @@ static int service_add_fd_store(Service *s, int fd, const char *name, bool do_po fs->fd = fd; fs->service = s; + fs->do_poll = do_poll; fs->fdname = strdup(name ?: "stored"); if (!fs->fdname) { free(fs); @@ -2717,7 +2718,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { if (!c) return log_oom(); - (void) serialize_item_format(f, "fd-store-fd", "%i %s", copy, c); + (void) serialize_item_format(f, "fd-store-fd", "%i \"%s\" %i", copy, c, fs->do_poll); } if (s->main_exec_status.pid > 0) { @@ -2946,30 +2947,36 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value, s->socket_fd = fdset_remove(fds, fd); } } else if (streq(key, "fd-store-fd")) { - const char *fdv; - size_t pf; + _cleanup_free_ char *fdv = NULL, *fdn = NULL, *fdp = NULL; int fd; + int do_poll; - pf = strcspn(value, WHITESPACE); - fdv = strndupa(value, pf); - - if (safe_atoi(fdv, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) + r = extract_first_word(&value, &fdv, NULL, 0); + if (r <= 0 || safe_atoi(fdv, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd)) { log_unit_debug(u, "Failed to parse fd-store-fd value: %s", value); - else { - _cleanup_free_ char *t = NULL; - const char *fdn; + return 0; + } - fdn = value + pf; - fdn += strspn(fdn, WHITESPACE); - (void) cunescape(fdn, 0, &t); + r = extract_first_word(&value, &fdn, NULL, EXTRACT_CUNESCAPE | EXTRACT_UNQUOTE); + if (r <= 0) { + log_unit_debug_errno(u, r, "Failed to parse fd-store-fd value \"%s\": %m", value); + return 0; + } - r = service_add_fd_store(s, fd, t, true); - if (r < 0) - log_unit_error_errno(u, r, "Failed to add fd to store: %m"); - else - fdset_remove(fds, fd); + r = extract_first_word(&value, &fdp, NULL, 0); + if (r == 0) { + /* If the value is not present, we assume the default */ + do_poll = 1; + } else if (r < 0 || safe_atoi(fdp, &do_poll) < 0) { + log_unit_debug_errno(u, r, "Failed to parse fd-store-fd value \"%s\": %m", value); + return 0; } + r = service_add_fd_store(s, fd, fdn, do_poll); + if (r < 0) + log_unit_error_errno(u, r, "Failed to add fd to store: %m"); + else + fdset_remove(fds, fd); } else if (streq(key, "main-exec-status-pid")) { pid_t pid; diff --git a/src/core/service.h b/src/core/service.h index 11e861a3d4..b9c8036f22 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -80,6 +80,7 @@ struct ServiceFDStore { int fd; char *fdname; sd_event_source *event_source; + bool do_poll; LIST_FIELDS(ServiceFDStore, fd_store); }; -- cgit v1.2.1