From b778cba4bfab53c567f6a81adb6413ab7d634c92 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Oct 2020 22:17:31 +0200 Subject: [PATCH 1/8] sd-event: optionally, if an event source fails, exit the event loop Currently, if an event source callback returns an error, we'll disable the event source and continue. This adds a per-event source flag that if turned on goes further: the event loop is also exited, propagating the error code. This is inspired by some patterns repeatedly seen in #15206. The idea is that event sources that server the "primary" function of a program are marked like this, so that if they fail the failure is instantly propagated and terminates the program. --- src/libsystemd/libsystemd.sym | 2 + src/libsystemd/sd-event/event-source.h | 1 + src/libsystemd/sd-event/sd-event.c | 51 ++++++++++++++++++++++---- src/systemd/sd-event.h | 2 + 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index c6dfcd6791..6e7f2eee53 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -726,6 +726,8 @@ LIBSYSTEMD_247 { global: sd_event_add_time_relative; sd_event_source_set_time_relative; + sd_event_source_get_exit_on_failure; + sd_event_source_set_exit_on_failure; sd_bus_error_has_names_sentinel; diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h index 08eb9b6a61..a8a30d825e 100644 --- a/src/libsystemd/sd-event/event-source.h +++ b/src/libsystemd/sd-event/event-source.h @@ -60,6 +60,7 @@ struct sd_event_source { bool pending:1; bool dispatching:1; bool floating:1; + bool exit_on_failure:1; int64_t priority; unsigned pending_index; diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 7dd43f2ddc..e891f62bb7 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -3183,16 +3183,21 @@ static int process_inotify(sd_event *e) { } static int source_dispatch(sd_event_source *s) { + _cleanup_(sd_event_unrefp) sd_event *saved_event = NULL; EventSourceType saved_type; int r = 0; assert(s); assert(s->pending || s->type == SOURCE_EXIT); - /* Save the event source type, here, so that we still know it after the event callback which might invalidate - * the event. */ + /* Save the event source type, here, so that we still know it after the event callback which might + * invalidate the event. */ saved_type = s->type; + /* Similar, store a reference to the event loop object, so that we can still access it after the + * callback might have invalidated/disconnected the event source. */ + saved_event = sd_event_ref(s->event); + if (!IN_SET(s->type, SOURCE_DEFER, SOURCE_EXIT)) { r = source_set_pending(s, false); if (r < 0) @@ -3299,9 +3304,15 @@ static int source_dispatch(sd_event_source *s) { s->dispatching = false; - if (r < 0) - log_debug_errno(r, "Event source %s (type %s) returned error, disabling: %m", - strna(s->description), event_source_type_to_string(saved_type)); + if (r < 0) { + log_debug_errno(r, "Event source %s (type %s) returned error, %s: %m", + strna(s->description), + event_source_type_to_string(saved_type), + s->exit_on_failure ? "exiting" : "disabling"); + + if (s->exit_on_failure) + (void) sd_event_exit(saved_event, r); + } if (s->n_ref == 0) source_free(s); @@ -3334,9 +3345,15 @@ static int event_prepare(sd_event *e) { r = s->prepare(s, s->userdata); s->dispatching = false; - if (r < 0) - log_debug_errno(r, "Prepare callback of event source %s (type %s) returned error, disabling: %m", - strna(s->description), event_source_type_to_string(s->type)); + if (r < 0) { + log_debug_errno(r, "Prepare callback of event source %s (type %s) returned error, %s: %m", + strna(s->description), + event_source_type_to_string(s->type), + s->exit_on_failure ? "exiting" : "disabling"); + + if (s->exit_on_failure) + (void) sd_event_exit(e, r); + } if (s->n_ref == 0) source_free(s); @@ -3974,3 +3991,21 @@ _public_ int sd_event_source_set_floating(sd_event_source *s, int b) { return 1; } + +_public_ int sd_event_source_get_exit_on_failure(sd_event_source *s) { + assert_return(s, -EINVAL); + assert_return(s->type != SOURCE_EXIT, -EDOM); + + return s->exit_on_failure; +} + +_public_ int sd_event_source_set_exit_on_failure(sd_event_source *s, int b) { + assert_return(s, -EINVAL); + assert_return(s->type != SOURCE_EXIT, -EDOM); + + if (s->exit_on_failure == !!b) + return 0; + + s->exit_on_failure = b; + return 1; +} diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h index dc96bfa681..3a53c3d27d 100644 --- a/src/systemd/sd-event.h +++ b/src/systemd/sd-event.h @@ -160,6 +160,8 @@ int sd_event_source_set_destroy_callback(sd_event_source *s, sd_event_destroy_t int sd_event_source_get_destroy_callback(sd_event_source *s, sd_event_destroy_t *ret); int sd_event_source_get_floating(sd_event_source *s); int sd_event_source_set_floating(sd_event_source *s, int b); +int sd_event_source_get_exit_on_failure(sd_event_source *s); +int sd_event_source_set_exit_on_failure(sd_event_source *s, int b); /* Define helpers so that __attribute__((cleanup(sd_event_unrefp))) and similar may be used. */ _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_event, sd_event_unref); From 647f2ee2593cff12a935779861b203eb632b67f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Oct 2020 10:14:44 +0200 Subject: [PATCH 2/8] man: add docs for sd_event_source_set_exit_on_failure() --- man/rules/meson.build | 4 + man/sd_event_source_set_exit_on_failure.xml | 108 ++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 man/sd_event_source_set_exit_on_failure.xml diff --git a/man/rules/meson.build b/man/rules/meson.build index dd236be1a1..00cd57420e 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -564,6 +564,10 @@ manpages = [ 'SD_EVENT_ONESHOT', 'sd_event_source_get_enabled'], ''], + ['sd_event_source_set_exit_on_failure', + '3', + ['sd_event_source_get_exit_on_failure'], + ''], ['sd_event_source_set_floating', '3', ['sd_event_source_get_floating'], ''], ['sd_event_source_set_prepare', '3', [], ''], ['sd_event_source_set_priority', diff --git a/man/sd_event_source_set_exit_on_failure.xml b/man/sd_event_source_set_exit_on_failure.xml new file mode 100644 index 0000000000..f9d87488c4 --- /dev/null +++ b/man/sd_event_source_set_exit_on_failure.xml @@ -0,0 +1,108 @@ + + + + + + + + sd_event_source_set_exit_on_failure + systemd + + + + sd_event_source_set_exit_on_failure + 3 + + + + sd_event_source_set_exit_on_failure + sd_event_source_get_exit_on_failure + + Set or retrieve the exit-on-failure feature of event sources + + + + + #include <systemd/sd-event.h> + + + int sd_event_source_set_exit_on_failure + sd_event_source *source + int b + + + + int sd_event_source_get_exit_on_failure + sd_event_source *source + + + + + + + Description + + sd_event_source_set_exit_on_failure() may be used to set/unset the + exit-on-failure flag of the event source object specified as source. The flag + defaults to off. If on and the callback function set for the event source returns a failure code (i.e. a + negative value) the event loop is exited too, using the callback return code as the exit code for + sd_event_exit3. If + off, the event source is disabled but the event loop continues to run. Setting this flag is useful for + "dominant" event sources that define the purpose and reason for the event loop, and whose failure hence + should propagate to the event loop itself — as opposed to "auxiliary" event sources whose failures should + remain local and affect the event source, but not propagate further. + + sd_event_source_get_exit_on_failure() may be used to query the flag currently + set for the event source object source. + + + + Return Value + + On success, sd_event_source_set_exit_on_failure() returns a non-negative + integer. sd_event_source_get_exit_on_failure() returns 0 if the flag is off, > 0 + if the flag is on. On failure, both return a negative errno-style error code. + + + Errors + + Returned errors may indicate the following problems: + + + + -EINVAL + + source is not a valid pointer to an + sd_event_source object. + + + + -EDOM + + The event source refers to an exit event source (as created with + sd_event_add_exit3), + for which this functionality is not supported. + + + + + + + + + + See Also + + + sd-event3, + sd_event_add_io3, + sd_event_add_time3, + sd_event_add_signal3, + sd_event_add_child3, + sd_event_add_inotify3, + sd_event_add_defer3 + + + + From 76c59537f3fef708109f90f6a728a8252d65e420 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Oct 2020 22:20:42 +0200 Subject: [PATCH 3/8] socket-proxy: close correct fd, log at right log level --- src/socket-proxy/socket-proxyd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 6042014069..9924220f03 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -522,8 +522,8 @@ static int accept_cb(sd_event_source *s, int fd, uint32_t revents, void *userdat r = add_connection_socket(context, nfd); if (r < 0) { - log_error_errno(r, "Failed to accept connection, ignoring: %m"); - safe_close(fd); + log_warning_errno(r, "Failed to accept connection, ignoring: %m"); + safe_close(nfd); } } From ccaa30c199499b2de22c2d0cc7bd53b4fa7acf1e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Oct 2020 22:21:05 +0200 Subject: [PATCH 4/8] socket-proxy: port to new sd_event_source_set_exit_on_failure() API --- src/socket-proxy/socket-proxyd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/socket-proxy/socket-proxyd.c b/src/socket-proxy/socket-proxyd.c index 9924220f03..64dd623c80 100644 --- a/src/socket-proxy/socket-proxyd.c +++ b/src/socket-proxy/socket-proxyd.c @@ -528,11 +528,8 @@ static int accept_cb(sd_event_source *s, int fd, uint32_t revents, void *userdat } r = sd_event_source_set_enabled(s, SD_EVENT_ONESHOT); - if (r < 0) { - log_error_errno(r, "Error while re-enabling listener with ONESHOT: %m"); - sd_event_exit(context->event, r); - return r; - } + if (r < 0) + return log_error_errno(r, "Error while re-enabling listener with ONESHOT: %m"); return 1; } @@ -561,11 +558,14 @@ static int add_listen_socket(Context *context, int fd) { r = set_ensure_put(&context->listen, NULL, source); if (r < 0) { - log_error_errno(r, "Failed to add source to set: %m"); sd_event_source_unref(source); - return r; + return log_error_errno(r, "Failed to add source to set: %m"); } + r = sd_event_source_set_exit_on_failure(source, true); + if (r < 0) + return log_error_errno(r, "Failed to enable exit-on-failure logic: %m"); + /* Set the watcher to oneshot in case other processes are also * watching to accept(). */ r = sd_event_source_set_enabled(source, SD_EVENT_ONESHOT); From 463f9ce3bc1a651759e76ba5d54a8181d1212b78 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Oct 2020 09:13:17 +0200 Subject: [PATCH 5/8] test: add test that validates that PTR_TO_INT(INT_TO_PTR()) covers whole int range --- src/test/test-util.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/test-util.c b/src/test/test-util.c index d4a6c8f5c3..cfbafcc5ca 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -414,6 +414,8 @@ static void test_foreach_pointer(void) { int a, b, c, *i; size_t k = 0; + log_info("/* %s */", __func__); + FOREACH_POINTER(i, &a, &b, &c) { switch (k) { @@ -489,6 +491,17 @@ static void test_foreach_pointer(void) { assert(k == 11); } +static void test_ptr_to_int(void) { + log_info("/* %s */", __func__); + + /* Primary reason to have this test is to validate that pointers are large enough to hold entire int range */ + assert_se(PTR_TO_INT(INT_TO_PTR(0)) == 0); + assert_se(PTR_TO_INT(INT_TO_PTR(1)) == 1); + assert_se(PTR_TO_INT(INT_TO_PTR(-1)) == -1); + assert_se(PTR_TO_INT(INT_TO_PTR(INT_MAX)) == INT_MAX); + assert_se(PTR_TO_INT(INT_TO_PTR(INT_MIN)) == INT_MIN); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_INFO); @@ -508,6 +521,7 @@ int main(int argc, char *argv[]) { test_system_tasks_max(); test_system_tasks_max_scale(); test_foreach_pointer(); + test_ptr_to_int(); return 0; } From bac0bfc1d0ee1e57055dffb78c77a1ee021ce536 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Oct 2020 09:13:37 +0200 Subject: [PATCH 6/8] udev-util: make use of sd-event's NULL callback support --- src/shared/udev-util.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 1b5995c130..b8c5bdc4b8 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -175,10 +175,6 @@ found: return sd_event_exit(sd_device_monitor_get_event(monitor), 0); } -static int device_timeout_handler(sd_event_source *s, uint64_t usec, void *userdata) { - return sd_event_exit(sd_event_source_get_event(s), -ETIMEDOUT); -} - static int device_wait_for_initialization_internal( sd_device *_device, const char *devlink, @@ -248,7 +244,7 @@ static int device_wait_for_initialization_internal( r = sd_event_add_time_relative( event, &timeout_source, CLOCK_MONOTONIC, timeout, 0, - device_timeout_handler, NULL); + NULL, INT_TO_PTR(-ETIMEDOUT)); if (r < 0) return log_error_errno(r, "Failed to add timeout event source: %m"); } From b9350e70aa050d97c47ff59994e84fa8c3de4738 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Oct 2020 09:51:36 +0200 Subject: [PATCH 7/8] sd-event: support callback=NULL in IO/child/inotify/defer event sources, too Also, document this functionality more prominently, including with a reference from sd_event_exit(). This is mostly to make things complete, as previously we supported NULL callbacks only in _add_time() and _add_signal(). However, I think this makes snese for IO event sources too (think: when some fd such as a pipe end sees SIGHUP or so, exit), as well as defer or post event sources (i.e. exit once we got nothing else to do). This also adds support for inotify event sources, simply to complete things (I can't see the immediate use, but maybe someone else comes up with it). The only event source type that doesn't allow callback=NULL now are exit callbacks, but for them they make little sense, as the event loop is exiting then anyway. --- man/sd_event_add_child.xml | 15 ++++++---- man/sd_event_add_defer.xml | 16 ++++++---- man/sd_event_add_inotify.xml | 13 ++++++-- man/sd_event_add_io.xml | 6 ++++ man/sd_event_add_signal.xml | 10 +++++-- man/sd_event_add_time.xml | 10 +++---- man/sd_event_exit.xml | 17 ++++++++++- src/libsystemd/sd-event/sd-event.c | 48 ++++++++++++++++++++++++++---- 8 files changed, 106 insertions(+), 29 deletions(-) diff --git a/man/sd_event_add_child.xml b/man/sd_event_add_child.xml index 1db536ff2a..74ebe6d264 100644 --- a/man/sd_event_add_child.xml +++ b/man/sd_event_add_child.xml @@ -150,11 +150,10 @@ pthread_sigmask3). - If the second parameter of - sd_event_add_child() is passed as NULL no - reference to the event source object is returned. In this case the - event source is considered "floating", and will be destroyed - implicitly when the event loop itself is destroyed. + If the second parameter of sd_event_add_child() is passed as + NULL no reference to the event source object is returned. In this case the event + source is considered "floating", and will be destroyed implicitly when the event loop itself is + destroyed. Note that the handler function is invoked at a time where the child process is not reaped yet (and @@ -164,6 +163,12 @@ event sources are installed will not be reaped by the event loop implementation. + If the handler parameter to sd_event_add_child() is + NULL, and the event source fires, this will be considered a request to exit the + event loop. In this case, the userdata parameter, cast to an integer, is passed as + the exit code parameter to + sd_event_exit3. + If both a child process state change event source and a SIGCHLD signal event source is installed in the same event loop, the configured event source priorities decide diff --git a/man/sd_event_add_defer.xml b/man/sd_event_add_defer.xml index 92be8353e6..d1d6d980ee 100644 --- a/man/sd_event_add_defer.xml +++ b/man/sd_event_add_defer.xml @@ -116,11 +116,17 @@ SD_EVENT_OFF with sd_event_source_set_enabled3. - If the second parameter of these functions is passed as - NULL no reference to the event source object is returned. In this - case the event source is considered "floating", and will be - destroyed implicitly when the event loop itself is - destroyed. + If the second parameter of these functions is passed as NULL no reference to + the event source object is returned. In this case the event source is considered "floating", and will be + destroyed implicitly when the event loop itself is destroyed. + + If the handler parameter to sd_event_add_defer() or + sd_event_add_post() is NULL, and the event source fires, this + will be considered a request to exit the event loop. In this case, the userdata + parameter, cast to an integer, is passed as the exit code parameter to + sd_event_exit3. Similar + functionality is not available for sd_event_add_exit(), as these types of event + sources are only dispatched when exiting anyway. diff --git a/man/sd_event_add_inotify.xml b/man/sd_event_add_inotify.xml index 826f4c4ada..27d43853e6 100644 --- a/man/sd_event_add_inotify.xml +++ b/man/sd_event_add_inotify.xml @@ -95,9 +95,16 @@ it with sd_event_source_set_enabled3. - If the second parameter of sd_event_add_inotify() is passed as NULL no reference to the - event source object is returned. In this case the event source is considered "floating", and will be destroyed - implicitly when the event loop itself is destroyed. + If the second parameter of sd_event_add_inotify() is passed as + NULL no reference to the event source object is returned. In this case the event + source is considered "floating", and will be destroyed implicitly when the event loop itself is + destroyed. + + If the handler parameter to sd_event_add_inotify() is + NULL, and the event source fires, this will be considered a request to exit the + event loop. In this case, the userdata parameter, cast to an integer, is passed as + the exit code parameter to + sd_event_exit3. sd_event_source_get_inotify_mask() retrieves the configured inotify watch mask of an event source created previously with sd_event_add_inotify(). It takes the event source object diff --git a/man/sd_event_add_io.xml b/man/sd_event_add_io.xml index 35d02a1700..51238f4755 100644 --- a/man/sd_event_add_io.xml +++ b/man/sd_event_add_io.xml @@ -161,6 +161,12 @@ "floating", and will be destroyed implicitly when the event loop itself is destroyed. + If the handler to sd_event_add_io() is + NULL, and the event source fires, this will be considered a request to exit the + event loop. In this case, the userdata parameter, cast to an integer, is passed as + the exit code parameter to + sd_event_exit3. + Note that this call does not take possession of the file descriptor passed in, ownership (and thus the duty to close it when it is no longer needed) remains with the caller. However, with the sd_event_source_set_io_fd_own() call (see below) the event source may optionally diff --git a/man/sd_event_add_signal.xml b/man/sd_event_add_signal.xml index 43794bd7ce..85de53120f 100644 --- a/man/sd_event_add_signal.xml +++ b/man/sd_event_add_signal.xml @@ -79,9 +79,7 @@ threads before this function is called (using sigprocmask2 or pthread_sigmask3). If - the handler is not specified (handler is NULL), a default - handler which causes the program to exit cleanly will be used. + project='man-pages'>pthread_sigmask3). By default, the event source is enabled permanently (SD_EVENT_ON), but this may be changed with @@ -107,6 +105,12 @@ "floating", and will be destroyed implicitly when the event loop itself is destroyed. + If the handler parameter to sd_event_add_signal() is + NULL, and the event source fires, this will be considered a request to exit the + event loop. In this case, the userdata parameter, cast to an integer, is passed as + the exit code parameter to + sd_event_exit3. + sd_event_source_get_signal() returns the configured signal number of an event source created previously with sd_event_add_signal(). It takes the diff --git a/man/sd_event_add_time.xml b/man/sd_event_add_time.xml index 1fc24c8ab0..24a316f9ed 100644 --- a/man/sd_event_add_time.xml +++ b/man/sd_event_add_time.xml @@ -161,12 +161,10 @@ "floating", and will be destroyed implicitly when the event loop itself is destroyed. - If the handler to - sd_event_add_time() is - NULL, and the event source fires, this will - be considered a request to exit the event loop. In this case, the - userdata parameter, cast to an integer, is - used for the exit code passed to + If the handler parameter to sd_event_add_time() is + NULL, and the event source fires, this will be considered a request to exit the + event loop. In this case, the userdata parameter, cast to an integer, is passed as + the exit code parameter to sd_event_exit3. Use CLOCK_BOOTTIME_ALARM and diff --git a/man/sd_event_exit.xml b/man/sd_event_exit.xml index a02897af0b..53aed70012 100644 --- a/man/sd_event_exit.xml +++ b/man/sd_event_exit.xml @@ -74,6 +74,16 @@ conflict with regular exit codes returned by sd_event_loop(), if these exit codes shall be distinguishable. + + Note that for most event source types passing the callback pointer as NULL in + the respective constructor call (i.e. in + sd_event_add_time3, + sd_event_add_signal3, + …) has the effect of sd_event_exit() being invoked once the event source triggers, + with the specified userdata pointer cast to an integer as the exit code parameter. This is useful to + automatically terminate an event loop after some condition, such as a time-out or reception of + SIGTERM or similar. See the documentation for the respective constructor call for + details. @@ -128,7 +138,12 @@ systemd1, sd-event3, sd_event_new3, - sd_event_add_exit3 + sd_event_add_exit3, + sd_event_add_time3, + sd_event_add_signal3, + sd_event_add_io3, + sd_event_add_defer3, + sd_event_add_inotify3 diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index e891f62bb7..d3df2e1309 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -972,6 +972,12 @@ static sd_event_source *source_new(sd_event *e, bool floating, EventSourceType t return s; } +static int io_exit_callback(sd_event_source *s, int fd, uint32_t revents, void *userdata) { + assert(s); + + return sd_event_exit(sd_event_source_get_event(s), PTR_TO_INT(userdata)); +} + _public_ int sd_event_add_io( sd_event *e, sd_event_source **ret, @@ -987,10 +993,12 @@ _public_ int sd_event_add_io( assert_return(e = event_resolve(e), -ENOPKG); assert_return(fd >= 0, -EBADF); assert_return(!(events & ~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET)), -EINVAL); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = io_exit_callback; + s = source_new(e, !ret, SOURCE_IO); if (!s) return -ENOMEM; @@ -1235,6 +1243,12 @@ _public_ int sd_event_add_signal( return 0; } +static int child_exit_callback(sd_event_source *s, const siginfo_t *si, void *userdata) { + assert(s); + + return sd_event_exit(sd_event_source_get_event(s), PTR_TO_INT(userdata)); +} + static bool shall_use_pidfd(void) { /* Mostly relevant for debugging, i.e. this is used in test-event.c to test the event loop once with and once without pidfd */ return getenv_bool_secure("SYSTEMD_PIDFD") != 0; @@ -1256,10 +1270,12 @@ _public_ int sd_event_add_child( assert_return(pid > 1, -EINVAL); assert_return(!(options & ~(WEXITED|WSTOPPED|WCONTINUED)), -EINVAL); assert_return(options != 0, -EINVAL); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = child_exit_callback; + if (e->n_enabled_child_sources == 0) { /* Caller must block SIGCHLD before using us to watch children, even if pidfd is available, * for compatibility with pre-pidfd and because we don't want the reap the child processes @@ -1357,10 +1373,12 @@ _public_ int sd_event_add_child_pidfd( assert_return(pidfd >= 0, -EBADF); assert_return(!(options & ~(WEXITED|WSTOPPED|WCONTINUED)), -EINVAL); assert_return(options != 0, -EINVAL); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = child_exit_callback; + if (e->n_enabled_child_sources == 0) { r = signal_is_blocked(SIGCHLD); if (r < 0) @@ -1426,6 +1444,12 @@ _public_ int sd_event_add_child_pidfd( return 0; } +static int generic_exit_callback(sd_event_source *s, void *userdata) { + assert(s); + + return sd_event_exit(sd_event_source_get_event(s), PTR_TO_INT(userdata)); +} + _public_ int sd_event_add_defer( sd_event *e, sd_event_source **ret, @@ -1437,10 +1461,12 @@ _public_ int sd_event_add_defer( assert_return(e, -EINVAL); assert_return(e = event_resolve(e), -ENOPKG); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = generic_exit_callback; + s = source_new(e, !ret, SOURCE_DEFER); if (!s) return -ENOMEM; @@ -1471,10 +1497,12 @@ _public_ int sd_event_add_post( assert_return(e, -EINVAL); assert_return(e = event_resolve(e), -ENOPKG); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = generic_exit_callback; + s = source_new(e, !ret, SOURCE_POST); if (!s) return -ENOMEM; @@ -1826,6 +1854,12 @@ static int inode_data_realize_watch(sd_event *e, struct inode_data *d) { return 1; } +static int inotify_exit_callback(sd_event_source *s, const struct inotify_event *event, void *userdata) { + assert(s); + + return sd_event_exit(sd_event_source_get_event(s), PTR_TO_INT(userdata)); +} + _public_ int sd_event_add_inotify( sd_event *e, sd_event_source **ret, @@ -1844,10 +1878,12 @@ _public_ int sd_event_add_inotify( assert_return(e, -EINVAL); assert_return(e = event_resolve(e), -ENOPKG); assert_return(path, -EINVAL); - assert_return(callback, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); + if (!callback) + callback = inotify_exit_callback; + /* Refuse IN_MASK_ADD since we coalesce watches on the same inode, and hence really don't want to merge * masks. Or in other words, this whole code exists only to manage IN_MASK_ADD type operations for you, hence * the user can't use them for us. */ From cbda8bd5fba097a6c90262828e8934b63e2efb22 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 2 Oct 2020 10:00:00 +0200 Subject: [PATCH 8/8] udev: make use of NULL callback in IO handlers --- src/udev/udev-ctrl.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index dafe3da251..9deb3864b5 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -355,10 +355,6 @@ int udev_ctrl_send(struct udev_ctrl *uctrl, enum udev_ctrl_msg_type type, int in return 0; } -static int udev_ctrl_wait_io_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { - return sd_event_exit(sd_event_source_get_event(s), 0); -} - int udev_ctrl_wait(struct udev_ctrl *uctrl, usec_t timeout) { _cleanup_(sd_event_source_unrefp) sd_event_source *source_io = NULL, *source_timeout = NULL; int r; @@ -385,7 +381,7 @@ int udev_ctrl_wait(struct udev_ctrl *uctrl, usec_t timeout) { return r; } - r = sd_event_add_io(uctrl->event, &source_io, uctrl->sock, EPOLLIN, udev_ctrl_wait_io_handler, NULL); + r = sd_event_add_io(uctrl->event, &source_io, uctrl->sock, EPOLLIN, NULL, INT_TO_PTR(0)); if (r < 0) return r;