From: Lennart Poettering Date: Thu, 12 Dec 2013 21:21:25 +0000 (+0100) Subject: event: rework sd-event exit logic X-Git-Tag: v209~956 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=6203e07a83214a55bb1f88508fcda2005c601dea;hp=6e41a3e53d858f30e131c62350f51465558ca55c event: rework sd-event exit logic With this change a failing event source handler will not cause the entire event loop to fail. Instead, we just disable the specific event source, log a message at debug level and go on. This also introduces a new concept of "exit code" which can be stored in the event loop and is returned by sd_event_loop(). We also rename "quit" to "exit" everywhere else. Altogether this should make things more robus and keep errors local while still providing a way to return event loop errors in a clear way. --- diff --git a/TODO b/TODO index dad55c461..4274b37ee 100644 --- a/TODO +++ b/TODO @@ -131,13 +131,10 @@ Features: - longer term: * priority queues * priority inheritance + - fix sd-event hookup when we connect to multiple servers one after the other * sd-event - allow multiple signal handlers per signal - - when a handler returns an error, just turn off its event source, - but do not return anything up to the event loop caller. Instead - add parameter to sd_event_request_quit() to take retval. This way - errors rippling upwards are the option, not the default - when dispatching an event source then _unref() on it should remove it from the epoll * in the final killing spree, detect processes from the root directory, and diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 3f08d1c0e..eb2b35f2c 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -645,8 +645,6 @@ int main(int argc, char *argv[]) { goto finish; } - r = 0; - finish: context_free(&context, bus); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 0f67fb8d5..3f8b95dee 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1243,7 +1243,7 @@ static int dispatch_sigterm(sd_event_source *es, const struct signalfd_siginfo * log_info("Received SIG%s", signal_to_string(si->ssi_signo)); - sd_event_request_quit(s->event); + sd_event_exit(s->event, 0); return 0; } diff --git a/src/libsystemd-bus/bus-util.c b/src/libsystemd-bus/bus-util.c index 3afcb82bc..30ee67e85 100644 --- a/src/libsystemd-bus/bus-util.c +++ b/src/libsystemd-bus/bus-util.c @@ -35,18 +35,18 @@ #include "bus-util.h" #include "bus-internal.h" -static int quit_callback(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { +static int name_owner_change_callback(sd_bus *bus, sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { sd_event *e = userdata; assert(bus); assert(m); assert(e); - sd_event_request_quit(e); + sd_event_exit(e, 0); return 1; } -int bus_async_unregister_and_quit(sd_event *e, sd_bus *bus, const char *name) { +int bus_async_unregister_and_exit(sd_event *e, sd_bus *bus, const char *name) { _cleanup_free_ char *match = NULL; const char *unique; int r; @@ -55,6 +55,11 @@ int bus_async_unregister_and_quit(sd_event *e, sd_bus *bus, const char *name) { assert(bus); assert(name); + /* We unregister the name here and then wait for the + * NameOwnerChanged signal for this event to arrive before we + * quit. We do this in order to make sure that any queued + * requests are still processed before we really exit. */ + r = sd_bus_get_unique_name(bus, &unique); if (r < 0) return r; @@ -71,7 +76,7 @@ int bus_async_unregister_and_quit(sd_event *e, sd_bus *bus, const char *name) { if (r < 0) return -ENOMEM; - r = sd_bus_add_match(bus, match, quit_callback, e); + r = sd_bus_add_match(bus, match, name_owner_change_callback, e); if (r < 0) return r; @@ -84,7 +89,7 @@ int bus_async_unregister_and_quit(sd_event *e, sd_bus *bus, const char *name) { int bus_event_loop_with_idle(sd_event *e, sd_bus *bus, const char *name, usec_t timeout) { bool exiting = false; - int r; + int r, code; assert(e); assert(bus); @@ -103,7 +108,7 @@ int bus_event_loop_with_idle(sd_event *e, sd_bus *bus, const char *name, usec_t return r; if (r == 0 && !exiting) { - r = bus_async_unregister_and_quit(e, bus, name); + r = bus_async_unregister_and_exit(e, bus, name); if (r < 0) return r; @@ -111,7 +116,11 @@ int bus_event_loop_with_idle(sd_event *e, sd_bus *bus, const char *name, usec_t } } - return 0; + r = sd_event_get_exit_code(e, &code); + if (r < 0) + return r; + + return code; } int bus_name_has_owner(sd_bus *c, const char *name, sd_bus_error *error) { diff --git a/src/libsystemd-bus/bus-util.h b/src/libsystemd-bus/bus-util.h index 9d4923794..9a90c2a1e 100644 --- a/src/libsystemd-bus/bus-util.h +++ b/src/libsystemd-bus/bus-util.h @@ -52,7 +52,7 @@ int bus_map_all_properties(sd_bus *bus, const struct bus_properties_map *map, void *userdata); -int bus_async_unregister_and_quit(sd_event *e, sd_bus *bus, const char *name); +int bus_async_unregister_and_exit(sd_event *e, sd_bus *bus, const char *name); int bus_event_loop_with_idle(sd_event *e, sd_bus *bus, const char *name, usec_t timeout); diff --git a/src/libsystemd-bus/libsystemd-bus.sym b/src/libsystemd-bus/libsystemd-bus.sym index 4a849b382..f3dbb760c 100644 --- a/src/libsystemd-bus/libsystemd-bus.sym +++ b/src/libsystemd-bus/libsystemd-bus.sym @@ -227,15 +227,15 @@ global: sd_event_add_signal; sd_event_add_child; sd_event_add_defer; - sd_event_add_quit; + sd_event_add_exit; sd_event_run; sd_event_loop; + sd_event_exit; sd_event_get_state; sd_event_get_tid; - sd_event_get_quit; - sd_event_request_quit; + sd_event_get_exit_code; sd_event_get_now_realtime; sd_event_get_now_monotonic; sd_event_set_watchdog; diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c index 060dcc143..a4709a109 100644 --- a/src/libsystemd-bus/sd-bus.c +++ b/src/libsystemd-bus/sd-bus.c @@ -2646,7 +2646,7 @@ _public_ int sd_bus_attach_event(sd_bus *bus, sd_event *event, int priority) { if (r < 0) goto fail; - r = sd_event_add_quit(bus->event, quit_callback, bus, &bus->quit_event_source); + r = sd_event_add_exit(bus->event, quit_callback, bus, &bus->quit_event_source); if (r < 0) goto fail; diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c index 462dd413e..b3964325a 100644 --- a/src/libsystemd-bus/sd-event.c +++ b/src/libsystemd-bus/sd-event.c @@ -44,7 +44,7 @@ typedef enum EventSourceType { SOURCE_SIGNAL, SOURCE_CHILD, SOURCE_DEFER, - SOURCE_QUIT, + SOURCE_EXIT, SOURCE_WATCHDOG } EventSourceType; @@ -96,7 +96,7 @@ struct sd_event_source { struct { sd_event_handler_t callback; unsigned prioq_index; - } quit; + } exit; }; }; @@ -132,7 +132,7 @@ struct sd_event { Hashmap *child_sources; unsigned n_enabled_child_sources; - Prioq *quit; + Prioq *exit; pid_t original_pid; @@ -140,10 +140,12 @@ struct sd_event { dual_timestamp timestamp; int state; - bool quit_requested:1; + bool exit_requested:1; bool need_process_child:1; bool watchdog:1; + int exit_code; + pid_t tid; sd_event **default_event_ptr; @@ -284,11 +286,11 @@ static int latest_time_prioq_compare(const void *a, const void *b) { return 0; } -static int quit_prioq_compare(const void *a, const void *b) { +static int exit_prioq_compare(const void *a, const void *b) { const sd_event_source *x = a, *y = b; - assert(x->type == SOURCE_QUIT); - assert(y->type == SOURCE_QUIT); + assert(x->type == SOURCE_EXIT); + assert(y->type == SOURCE_EXIT); /* Enabled ones first */ if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF) @@ -338,7 +340,7 @@ static void event_free(sd_event *e) { prioq_free(e->monotonic_latest); prioq_free(e->realtime_earliest); prioq_free(e->realtime_latest); - prioq_free(e->quit); + prioq_free(e->exit); free(e->signal_sources); @@ -515,8 +517,8 @@ static void source_free(sd_event_source *s) { /* nothing */ break; - case SOURCE_QUIT: - prioq_remove(s->event->quit, s, &s->quit.prioq_index); + case SOURCE_EXIT: + prioq_remove(s->event->exit, s, &s->exit.prioq_index); break; } @@ -536,7 +538,7 @@ static int source_set_pending(sd_event_source *s, bool b) { int r; assert(s); - assert(s->type != SOURCE_QUIT); + assert(s->type != SOURCE_EXIT); if (s->pending == b) return 0; @@ -934,7 +936,7 @@ _public_ int sd_event_add_defer( return 0; } -_public_ int sd_event_add_quit( +_public_ int sd_event_add_exit( sd_event *e, sd_event_handler_t callback, void *userdata, @@ -949,22 +951,22 @@ _public_ int sd_event_add_quit( assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); - if (!e->quit) { - e->quit = prioq_new(quit_prioq_compare); - if (!e->quit) + if (!e->exit) { + e->exit = prioq_new(exit_prioq_compare); + if (!e->exit) return -ENOMEM; } - s = source_new(e, SOURCE_QUIT); + s = source_new(e, SOURCE_EXIT); if (!s) return -ENOMEM; - s->quit.callback = callback; + s->exit.callback = callback; s->userdata = userdata; - s->quit.prioq_index = PRIOQ_IDX_NULL; + s->exit.prioq_index = PRIOQ_IDX_NULL; s->enabled = SD_EVENT_ONESHOT; - r = prioq_put(s->event->quit, s, &s->quit.prioq_index); + r = prioq_put(s->event->exit, s, &s->exit.prioq_index); if (r < 0) { source_free(s); return r; @@ -1005,7 +1007,7 @@ _public_ sd_event *sd_event_source_get_event(sd_event_source *s) { _public_ int sd_event_source_get_pending(sd_event_source *s) { assert_return(s, -EINVAL); - assert_return(s->type != SOURCE_QUIT, -EDOM); + assert_return(s->type != SOURCE_EXIT, -EDOM); assert_return(s->event->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(s->event), -ECHILD); @@ -1096,8 +1098,8 @@ _public_ int sd_event_source_set_priority(sd_event_source *s, int priority) { if (s->prepare) prioq_reshuffle(s->event->prepare, s, &s->prepare_index); - if (s->type == SOURCE_QUIT) - prioq_reshuffle(s->event->quit, s, &s->quit.prioq_index); + if (s->type == SOURCE_EXIT) + prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index); return 0; } @@ -1168,9 +1170,9 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) { break; - case SOURCE_QUIT: + case SOURCE_EXIT: s->enabled = m; - prioq_reshuffle(s->event->quit, s, &s->quit.prioq_index); + prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index); break; case SOURCE_DEFER: @@ -1223,9 +1225,9 @@ _public_ int sd_event_source_set_enabled(sd_event_source *s, int m) { } break; - case SOURCE_QUIT: + case SOURCE_EXIT: s->enabled = m; - prioq_reshuffle(s->event->quit, s, &s->quit.prioq_index); + prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index); break; case SOURCE_DEFER: @@ -1321,7 +1323,7 @@ _public_ int sd_event_source_set_prepare(sd_event_source *s, sd_event_handler_t int r; assert_return(s, -EINVAL); - assert_return(s->type != SOURCE_QUIT, -EDOM); + assert_return(s->type != SOURCE_EXIT, -EDOM); assert_return(s->event->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(s->event), -ECHILD); @@ -1673,9 +1675,9 @@ static int source_dispatch(sd_event_source *s) { int r = 0; assert(s); - assert(s->pending || s->type == SOURCE_QUIT); + assert(s->pending || s->type == SOURCE_EXIT); - if (s->type != SOURCE_DEFER && s->type != SOURCE_QUIT) { + if (s->type != SOURCE_DEFER && s->type != SOURCE_EXIT) { r = source_set_pending(s, false); if (r < 0) return r; @@ -1727,14 +1729,18 @@ static int source_dispatch(sd_event_source *s) { r = s->defer.callback(s, s->userdata); break; - case SOURCE_QUIT: - r = s->quit.callback(s, s->userdata); + case SOURCE_EXIT: + r = s->exit.callback(s, s->userdata); break; } - sd_event_source_unref(s); + if (r < 0) { + log_debug("Event source %p returned error, disabling: %s", s, strerror(-r)); + sd_event_source_set_enabled(s, SD_EVENT_OFF); + } - return r; + sd_event_source_unref(s); + return 1; } static int event_prepare(sd_event *e) { @@ -1764,13 +1770,13 @@ static int event_prepare(sd_event *e) { return 0; } -static int dispatch_quit(sd_event *e) { +static int dispatch_exit(sd_event *e) { sd_event_source *p; int r; assert(e); - p = prioq_peek(e->quit); + p = prioq_peek(e->exit); if (!p || p->enabled == SD_EVENT_OFF) { e->state = SD_EVENT_FINISHED; return 0; @@ -1778,7 +1784,7 @@ static int dispatch_quit(sd_event *e) { sd_event_ref(e); e->iteration++; - e->state = SD_EVENT_QUITTING; + e->state = SD_EVENT_EXITING; r = source_dispatch(p); @@ -1850,8 +1856,8 @@ _public_ int sd_event_run(sd_event *e, uint64_t timeout) { assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(e->state == SD_EVENT_PASSIVE, -EBUSY); - if (e->quit_requested) - return dispatch_quit(e); + if (e->exit_requested) + return dispatch_exit(e); sd_event_ref(e); e->iteration++; @@ -1946,7 +1952,7 @@ _public_ int sd_event_loop(sd_event *e) { goto finish; } - r = 0; + r = e->exit_code; finish: sd_event_unref(e); @@ -1960,19 +1966,26 @@ _public_ int sd_event_get_state(sd_event *e) { return e->state; } -_public_ int sd_event_get_quit(sd_event *e) { +_public_ int sd_event_get_exit_code(sd_event *e, int *code) { assert_return(e, -EINVAL); + assert_return(code, -EINVAL); assert_return(!event_pid_changed(e), -ECHILD); - return e->quit_requested; + if (!e->exit_requested) + return -ENODATA; + + *code = e->exit_code; + return 0; } -_public_ int sd_event_request_quit(sd_event *e) { +_public_ int sd_event_exit(sd_event *e, int code) { assert_return(e, -EINVAL); assert_return(e->state != SD_EVENT_FINISHED, -ESTALE); assert_return(!event_pid_changed(e), -ECHILD); - e->quit_requested = true; + e->exit_requested = true; + e->exit_code = code; + return 0; } diff --git a/src/libsystemd-bus/test-event.c b/src/libsystemd-bus/test-event.c index 2b91eb0a7..5317008a8 100644 --- a/src/libsystemd-bus/test-event.c +++ b/src/libsystemd-bus/test-event.c @@ -63,7 +63,7 @@ static int child_handler(sd_event_source *s, const siginfo_t *si, void *userdata assert(userdata == INT_TO_PTR('f')); - assert_se(sd_event_request_quit(sd_event_source_get_event(s)) >= 0); + assert_se(sd_event_exit(sd_event_source_get_event(s), 0) >= 0); sd_event_source_unref(s); return 1; @@ -143,12 +143,12 @@ static int time_handler(sd_event_source *s, uint64_t usec, void *userdata) { return 2; } -static bool got_quit = false; +static bool got_exit = false; -static int quit_handler(sd_event_source *s, void *userdata) { +static int exit_handler(sd_event_source *s, void *userdata) { log_info("got quit handler on %c", PTR_TO_INT(userdata)); - got_quit = true; + got_exit = true; return 3; } @@ -183,7 +183,7 @@ int main(int argc, char *argv[]) { assert_se(sd_event_add_io(e, a[0], EPOLLIN, io_handler, INT_TO_PTR('a'), &x) >= 0); assert_se(sd_event_add_io(e, b[0], EPOLLIN, io_handler, INT_TO_PTR('b'), &y) >= 0); assert_se(sd_event_add_monotonic(e, 0, 0, time_handler, INT_TO_PTR('c'), &z) >= 0); - assert_se(sd_event_add_quit(e, quit_handler, INT_TO_PTR('g'), &q) >= 0); + assert_se(sd_event_add_exit(e, exit_handler, INT_TO_PTR('g'), &q) >= 0); assert_se(sd_event_source_set_priority(x, 99) >= 0); assert_se(sd_event_source_set_enabled(y, SD_EVENT_ONESHOT) >= 0); diff --git a/src/libsystemd-rtnl/rtnl-internal.h b/src/libsystemd-rtnl/rtnl-internal.h index dabf12d37..a1050a07f 100644 --- a/src/libsystemd-rtnl/rtnl-internal.h +++ b/src/libsystemd-rtnl/rtnl-internal.h @@ -74,7 +74,7 @@ struct sd_rtnl { sd_event_source *io_event_source; sd_event_source *time_event_source; - sd_event_source *quit_event_source; + sd_event_source *exit_event_source; sd_event *event; }; diff --git a/src/libsystemd-rtnl/sd-rtnl.c b/src/libsystemd-rtnl/sd-rtnl.c index 57d0b766f..98d0f89e5 100644 --- a/src/libsystemd-rtnl/sd-rtnl.c +++ b/src/libsystemd-rtnl/sd-rtnl.c @@ -743,7 +743,7 @@ static int prepare_callback(sd_event_source *s, void *userdata) { return 1; } -static int quit_callback(sd_event_source *event, void *userdata) { +static int exit_callback(sd_event_source *event, void *userdata) { sd_rtnl *rtnl = userdata; assert(event); @@ -790,7 +790,7 @@ int sd_rtnl_attach_event(sd_rtnl *rtnl, sd_event *event, int priority) { if (r < 0) goto fail; - r = sd_event_add_quit(rtnl->event, quit_callback, rtnl, &rtnl->quit_event_source); + r = sd_event_add_exit(rtnl->event, exit_callback, rtnl, &rtnl->exit_event_source); if (r < 0) goto fail; @@ -811,8 +811,8 @@ int sd_rtnl_detach_event(sd_rtnl *rtnl) { if (rtnl->time_event_source) rtnl->time_event_source = sd_event_source_unref(rtnl->time_event_source); - if (rtnl->quit_event_source) - rtnl->quit_event_source = sd_event_source_unref(rtnl->quit_event_source); + if (rtnl->exit_event_source) + rtnl->exit_event_source = sd_event_source_unref(rtnl->exit_event_source); if (rtnl->event) rtnl->event = sd_event_unref(rtnl->event); diff --git a/src/locale/localed.c b/src/locale/localed.c index 65318b606..2c9018017 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -1155,8 +1155,6 @@ int main(int argc, char *argv[]) { goto finish; } - r = 0; - finish: context_free(&context, bus); diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h index 3551077f3..c7c45067f 100644 --- a/src/systemd/sd-event.h +++ b/src/systemd/sd-event.h @@ -53,7 +53,7 @@ enum { enum { SD_EVENT_PASSIVE, SD_EVENT_RUNNING, - SD_EVENT_QUITTING, + SD_EVENT_EXITING, SD_EVENT_FINISHED }; @@ -82,15 +82,15 @@ int sd_event_add_realtime(sd_event *e, uint64_t usec, uint64_t accuracy, sd_even int sd_event_add_signal(sd_event *e, int sig, sd_event_signal_handler_t callback, void *userdata, sd_event_source **s); int sd_event_add_child(sd_event *e, pid_t pid, int options, sd_event_child_handler_t callback, void *userdata, sd_event_source **s); int sd_event_add_defer(sd_event *e, sd_event_handler_t callback, void *userdata, sd_event_source **s); -int sd_event_add_quit(sd_event *e, sd_event_handler_t callback, void *userdata, sd_event_source **s); +int sd_event_add_exit(sd_event *e, sd_event_handler_t callback, void *userdata, sd_event_source **s); int sd_event_run(sd_event *e, uint64_t timeout); int sd_event_loop(sd_event *e); +int sd_event_exit(sd_event *e, int code); int sd_event_get_state(sd_event *e); int sd_event_get_tid(sd_event *e, pid_t *tid); -int sd_event_get_quit(sd_event *e); -int sd_event_request_quit(sd_event *e); +int sd_event_get_exit_code(sd_event *e, int *code); int sd_event_get_now_realtime(sd_event *e, uint64_t *usec); int sd_event_get_now_monotonic(sd_event *e, uint64_t *usec); int sd_event_set_watchdog(sd_event *e, int b); diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 37173a252..8ac933c5e 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -863,8 +863,6 @@ int main(int argc, char *argv[]) { goto finish; } - r = 0; - finish: context_free(&context, bus);