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.
This commit is contained in:
Lennart Poettering 2013-12-12 22:21:25 +01:00
parent 6e41a3e53d
commit 6203e07a83
14 changed files with 93 additions and 80 deletions

5
TODO
View File

@ -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

View File

@ -645,8 +645,6 @@ int main(int argc, char *argv[]) {
goto finish;
}
r = 0;
finish:
context_free(&context, bus);

View File

@ -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;
}

View File

@ -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) {

View File

@ -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);

View File

@ -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;

View File

@ -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;

View File

@ -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;
}

View File

@ -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);

View File

@ -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;
};

View File

@ -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);

View File

@ -1155,8 +1155,6 @@ int main(int argc, char *argv[]) {
goto finish;
}
r = 0;
finish:
context_free(&context, bus);

View File

@ -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);

View File

@ -863,8 +863,6 @@ int main(int argc, char *argv[]) {
goto finish;
}
r = 0;
finish:
context_free(&context, bus);