3 files changed, 53 insertions, 12 deletions

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) {
+        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);
+
+                s->dispatching = true;
                 r = s->prepare(s, s->userdata);
+                s->dispatching = false;
+
                 if (r < 0)
-                        return r;
+                        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;
 }