diff --git a/TODO b/TODO index 4274b37ee9..0ea410953c 100644 --- a/TODO +++ b/TODO @@ -134,8 +134,7 @@ Features: - fix sd-event hookup when we connect to multiple servers one after the other * sd-event - - allow multiple signal handlers per signal - - when dispatching an event source then _unref() on it should remove it from the epoll + - allow multiple signal handlers per signal? * in the final killing spree, detect processes from the root directory, and complain loudly if they have argv[0][0] == '@' set. diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c index b3964325a0..6af52ecb3c 100644 --- a/src/libsystemd-bus/sd-event.c +++ b/src/libsystemd-bus/sd-event.c @@ -58,6 +58,7 @@ struct sd_event_source { EventSourceType type:4; int enabled:3; bool pending:1; + bool dispatching:1; int priority; unsigned pending_index; @@ -993,8 +994,21 @@ _public_ sd_event_source* sd_event_source_unref(sd_event_source *s) { assert(s->n_ref >= 1); s->n_ref--; - if (s->n_ref <= 0) - source_free(s); + if (s->n_ref <= 0) { + /* Here's a special hack: when we are called from a + * dispatch handler we won't free the event source + * immediately, but we will detach the fd from the + * epoll. This way it is safe for the caller to unref + * the event source and immediately close the fd, but + * we still retain a valid event source object after + * the callback. */ + + if (s->dispatching) { + if (s->type == SOURCE_IO) + source_io_unregister(s); + } else + source_free(s); + } return NULL; } @@ -1689,7 +1703,7 @@ static int source_dispatch(sd_event_source *s) { return r; } - sd_event_source_ref(s); + s->dispatching = true; switch (s->type) { @@ -1734,12 +1748,16 @@ static int source_dispatch(sd_event_source *s) { break; } - if (r < 0) { - log_debug("Event source %p returned error, disabling: %s", s, strerror(-r)); - sd_event_source_set_enabled(s, SD_EVENT_OFF); - } + s->dispatching = false; + + if (r < 0) + log_debug("Event source %p returned error, disabling: %s", s, strerror(-r)); + + if (s->n_ref == 0) + source_free(s); + else if (r < 0) + sd_event_source_set_enabled(s, SD_EVENT_OFF); - sd_event_source_unref(s); return 1; } @@ -1761,10 +1779,18 @@ static int event_prepare(sd_event *e) { return r; assert(s->prepare); - r = s->prepare(s, s->userdata); - if (r < 0) - return r; + s->dispatching = true; + r = s->prepare(s, s->userdata); + s->dispatching = false; + + if (r < 0) + log_debug("Prepare callback of event source %p returned error, disabling: %s", s, strerror(-r)); + + if (s->n_ref == 0) + source_free(s); + else if (r < 0) + sd_event_source_set_enabled(s, SD_EVENT_OFF); } return 0; diff --git a/src/libsystemd-bus/test-event.c b/src/libsystemd-bus/test-event.c index 5317008a87..28ef6a3692 100644 --- a/src/libsystemd-bus/test-event.c +++ b/src/libsystemd-bus/test-event.c @@ -28,9 +28,15 @@ static int prepare_handler(sd_event_source *s, void *userdata) { return 1; } -static bool got_a, got_b, got_c; +static bool got_a, got_b, got_c, got_unref; static unsigned got_d; +static int unref_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { + sd_event_source_unref(s); + got_unref = true; + return 0; +} + static int io_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { log_info("got IO on %c", PTR_TO_INT(userdata)); @@ -155,18 +161,26 @@ static int exit_handler(sd_event_source *s, void *userdata) { int main(int argc, char *argv[]) { sd_event *e = NULL; - sd_event_source *w = NULL, *x = NULL, *y = NULL, *z = NULL, *q = NULL; + sd_event_source *w = NULL, *x = NULL, *y = NULL, *z = NULL, *q = NULL, *t = NULL; static const char ch = 'x'; - int a[2] = { -1, -1 }, b[2] = { -1, -1}, d[2] = { -1, -1}; + int a[2] = { -1, -1 }, b[2] = { -1, -1}, d[2] = { -1, -1}, k[2] = { -1, -1 }; assert_se(pipe(a) >= 0); assert_se(pipe(b) >= 0); assert_se(pipe(d) >= 0); + assert_se(pipe(k) >= 0); assert_se(sd_event_default(&e) >= 0); assert_se(sd_event_set_watchdog(e, true) >= 0); + /* Test whether we cleanly can destroy an io event source from its own handler */ + got_unref = false; + assert_se(sd_event_add_io(e, k[0], EPOLLIN, unref_handler, NULL, &t) >= 0); + assert_se(write(k[1], &ch, 1) == 1); + assert_se(sd_event_run(e, (uint64_t) -1) >= 1); + assert_se(got_unref); + got_a = false, got_b = false, got_c = false, got_d = 0; /* Add a oneshot handler, trigger it, re-enable it, and trigger @@ -227,6 +241,8 @@ int main(int argc, char *argv[]) { close_pipe(a); close_pipe(b); + close_pipe(d); + close_pipe(k); return 0; }