sd-future: make sd_fiber_await() report failures rather then future results

sd_fiber_await() returned the target future's result on completion and an error
on interruption, so a negative value could mean either that the target future
resolved with an error or that the wait never finished. Most callers had to check
the target future's state afterwards to tell the two apart. Even worse, a target
resolving with -ECANCELED was indistinguishable from a cancelled wait.

Let's return 0 once the target future has resolved and a negative error only when
the wait itself did not complete. The target future's outcome stays available through
sd_future_result(), which callers have to additionally inspect now.

src/libsystemd/sd-bus/bus-future.c

@@ -123,14 +123,12 @@ int bus_call_suspend(
         if (r < 0)
                 return sd_bus_error_set_errno(reterr_error, r);
 
+        /* An interruption takes precedence even if the reply arrived in the same tick: the wait
+         * consumed the queued cancellation or timeout, and returning the reply instead would drop it. */
         r = sd_fiber_await(f);
-
+        if (r < 0)
-        /* If the future isn't resolved, the suspend was interrupted before a reply arrived (fiber
-         * cancelled, fiber-wide SD_FIBER_TIMEOUT scope expired, …). There's no reply to extract,
-         * so surface the resume error directly. When the future is resolved, future_get_bus_reply()
-         * recovers either the reply or the detailed sd_bus_error from the error reply. */
-        if (sd_future_state(f) != SD_FUTURE_RESOLVED)
                 return sd_bus_error_set_errno(reterr_error, r);
 
+        /* Recovers either the reply or the detailed sd_bus_error from the error reply. */
         return future_get_bus_reply(f, reterr_error, ret_reply);
 }

src/libsystemd/sd-bus/test-bus-fiber.c

@@ -299,6 +299,7 @@ static int interrupted_calls_fiber(void *userdata) {
                 }
 
                 ASSERT_OK_ZERO(sd_fiber_await(caller));
+                ASSERT_OK_ZERO(sd_future_result(caller));
                 ASSERT_NULL(s->c->pending_call);
                 ASSERT_NULL(s->c->waiter);
         }

src/libsystemd/sd-bus/test-bus-watch-bind.c

@@ -189,6 +189,8 @@ static int request_exit(void *userdata) {
         /* Wait for all client fibers to complete before requesting exit */
         ASSERT_OK(sd_fiber_await(args->client1));
         ASSERT_OK(sd_fiber_await(args->client2));
+        ASSERT_OK(sd_future_result(args->client1));
+        ASSERT_OK(sd_future_result(args->client2));
 
         ASSERT_OK(sd_bus_new(&bus));
 

src/libsystemd/sd-event/event-future.c

@@ -411,6 +411,10 @@ int event_run_suspend(sd_event *e, uint64_t timeout) {
         if (r < 0)
                 return r;
 
+        r = sd_future_result(group);
+        if (r < 0)
+                return r;
+
         r = sd_event_prepare(e);
         if (r == 0)
                 r = sd_event_wait(e, 0);

src/libsystemd/sd-future/fiber-io.c

@@ -54,6 +54,10 @@ static ssize_t fiber_io_operation(
         if (r < 0)
                 return r;
 
+        r = sd_future_result(io);
+        if (r < 0)
+                return r;
+
         return func(fd, args);
 }
 
@@ -227,9 +231,13 @@ int sd_fiber_connect(int sockfd, const struct sockaddr *addr, socklen_t addrlen)
         if (r < 0)
                 return r;
 
+        r = sd_fiber_await(io);
+        if (r < 0)
+                return r;
+
         /* future_new_io resolves with the revents mask on success; translate any positive value
          * (e.g. POLLOUT) back to the connect(2) success status. */
-        r = sd_fiber_await(io);
+        r = sd_future_result(io);
         return r > 0 ? 0 : r;
 }
 
@@ -445,6 +453,11 @@ int sd_fiber_ppoll(struct pollfd *fds, size_t n_fds, const struct timespec *time
         r = sd_fiber_await(group);
         if (r < 0 && r != -ETIME)
                 return r;
+        if (r >= 0) {
+                r = sd_future_result(group);
+                if (r < 0)
+                        return r;
+        }
 
         /* Always sweep fds with a non-blocking ppoll(): the timer and an fd readiness can resolve in
          * the same event-loop tick (or the fd can become ready between the timer firing and us being

src/libsystemd/sd-future/fiber.c

@@ -822,7 +822,11 @@ int sd_fiber_sleep(uint64_t usec) {
         if (r < 0)
                 return r;
 
-        return sd_fiber_await(timer);
+        r = sd_fiber_await(timer);
+        if (r < 0)
+                return r;
+
+        return sd_future_result(timer);
 }
 
 static int fiber_wait_callback(sd_future *target, void *userdata) {
@@ -861,7 +865,7 @@ int sd_fiber_await(sd_future *target) {
         assert_return(sd_future_get_event(target) == sd_future_get_event(f), -EINVAL);
 
         if (sd_future_state(target) == SD_FUTURE_RESOLVED)
-                return sd_future_result(target);
+                return 0;
 
         /* The fiber is executing inside event dispatch, so its loop cannot have finished yet.
          * Waiting during exit is supported by the slot's exit source. */
@@ -879,8 +883,7 @@ int sd_fiber_await(sd_future *target) {
         if (sd_future_state(target) != SD_FUTURE_RESOLVED)
                 return -EBUSY;
 
-        /* Read the result while the slot still pins the target: releasing it may drop the last ref. */
+        return 0;
-        return sd_future_result(target);
 }
 
 sd_future* sd_future_cancel_wait_unref(sd_future *f) {

src/libsystemd/sd-future/test-fiber.c

@@ -379,16 +379,17 @@ TEST(fiber_wait_for_completed) {
         ASSERT_OK_EQ(sd_future_result(target), 100);
 }
 
-/* Test: awaiting an already-resolved future returns the future's result directly */
+/* Test: awaiting an already-resolved future completes immediately */
 static int await_resolved_fiber(void *userdata) {
         sd_future *target = userdata;
 
         ASSERT_EQ((int) sd_future_state(target), (int) SD_FUTURE_RESOLVED);
-        ASSERT_OK_EQ(sd_fiber_await(target), 77);
+        ASSERT_OK_ZERO(sd_fiber_await(target));
+        ASSERT_OK_EQ(sd_future_result(target), 77);
         return 0;
 }
 
-TEST(fiber_await_resolved_returns_result) {
+TEST(fiber_await_resolved) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
         ASSERT_OK(sd_event_new(&e));
         ASSERT_OK(sd_event_set_exit_on_idle(e, true));
@@ -1667,13 +1668,14 @@ static int await_result_fiber(void *userdata) {
                 _cleanup_(sd_future_unrefp) sd_future *awaited = NULL;
 
                 ASSERT_OK(sd_future_new_defer(sd_fiber_get_event(), result, &awaited));
-                ASSERT_EQ(sd_fiber_await(awaited), result); /* Pending. */
+                ASSERT_OK_ZERO(sd_fiber_await(awaited)); /* Pending. */
-                ASSERT_EQ(sd_fiber_await(awaited), result); /* Already resolved. */
+                ASSERT_EQ(sd_future_result(awaited), result);
+                ASSERT_OK_ZERO(sd_fiber_await(awaited)); /* Already resolved. */
                 ASSERT_OK_ZERO(sd_fiber_yield());
 
                 /* An already-resolved future must leave an unrelated pending interruption alone. */
                 ASSERT_OK(sd_fiber_resume(sd_fiber_get_current(), -ECANCELED));
-                ASSERT_EQ(sd_fiber_await(awaited), result);
+                ASSERT_OK_ZERO(sd_fiber_await(awaited));
                 ASSERT_ERROR(sd_fiber_yield(), ECANCELED);
         }
 
@@ -1693,7 +1695,14 @@ TEST(fiber_await_result) {
 }
 
 static int await_borrowed_fiber(void *userdata) {
-        return sd_fiber_await(userdata);
+        _cleanup_(sd_future_unrefp) sd_future *target = sd_future_ref(ASSERT_PTR(userdata));
+        int r;
+
+        r = sd_fiber_await(target);
+        if (r < 0)
+                return r;
+
+        return sd_future_result(target);
 }
 
 TEST(fiber_await_last_reference) {
@@ -1709,11 +1718,11 @@ TEST(fiber_await_last_reference) {
                 ASSERT_OK(sd_fiber_new(e, "await-borrowed", await_borrowed_fiber, target,
                                        /* destroy= */ NULL, &waiter));
 
-                /* Suspend in await, so its non-floating callback slot holds a reference to target. */
+                /* Suspend in await, so the waiter's own reference and the wait's slot keep target alive. */
                 ASSERT_OK_POSITIVE(sd_event_run(e, 0));
                 ASSERT_EQ(sd_future_state(waiter), SD_FUTURE_PENDING);
 
-                /* Leave the slot as the sole owner. Await must read the result before freeing it. */
+                /* Drop our reference while the wait is in flight: the waiter still reads the result. */
                 ASSERT_OK(sd_future_resolve(target, result));
                 target = sd_future_unref(target);
 

src/libsystemd/sd-future/test-future-group.c

@@ -404,7 +404,7 @@ TEST(future_group_cancels_parent_on_child_error) {
         ASSERT_ERROR(s.group_result, EINVAL);
 }
 
-/* Awaiting a group must return the child's error instead of cancelling the waiting parent. */
+/* Awaiting a group must complete with the child's error instead of cancelling the waiting parent. */
 static int await_gets_error_driver(void *userdata) {
         _cleanup_(sd_future_unrefp) sd_future *group = NULL, *errorer = NULL;
         int *await_result = ASSERT_PTR(userdata);
@@ -413,7 +413,8 @@ static int await_gets_error_driver(void *userdata) {
         ASSERT_OK(sd_future_new_defer(sd_fiber_get_event(), -EINVAL, &errorer));
         ASSERT_OK(sd_future_group_add(group, errorer));
 
-        *await_result = sd_fiber_await(group);
+        ASSERT_OK_ZERO(sd_fiber_await(group));
+        *await_result = sd_future_result(group);
         ASSERT_OK_ZERO(sd_fiber_yield());
         return 0;
 }
@@ -462,10 +463,9 @@ typedef struct AddResolvedState {
 static int add_resolved_driver(void *userdata) {
         AddResolvedState *s = ASSERT_PTR(userdata);
 
-        /* Drive the pre-built child to completion. The defer resolves with -EINVAL, which
+        /* Drive the pre-built child to completion. The defer resolves with -EINVAL, but the wait
-         * makes sd_fiber_await return -EINVAL — we don't ASSERT_OK that. The child is now
+         * itself succeeds. The child is now RESOLVED and we can add it to the group. */
-         * RESOLVED and we can add it to the group. */
+        ASSERT_OK_ZERO(sd_fiber_await(s->child));
-        (void) sd_fiber_await(s->child);
         ASSERT_EQ(sd_future_state(s->child), SD_FUTURE_RESOLVED);
 
         s->add_result = sd_future_group_add(s->group, s->child);
@@ -473,7 +473,8 @@ static int add_resolved_driver(void *userdata) {
 
         /* Await drives the loop one more tick so the defer that wraps the RESOLVED child can
          * fire group_child_resolved and settle the group. */
-        s->await_result = sd_fiber_await(s->group);
+        ASSERT_OK_ZERO(sd_fiber_await(s->group));
+        s->await_result = sd_future_result(s->group);
         return 0;
 }
 
@@ -510,7 +511,8 @@ typedef struct AddManyState {
 static int add_many_driver(void *userdata) {
         AddManyState *s = ASSERT_PTR(userdata);
         ASSERT_OK(sd_future_group_add_many(s->group, s->a, s->b, s->c));
-        s->join_result = sd_fiber_await(s->group);
+        ASSERT_OK_ZERO(sd_fiber_await(s->group));
+        s->join_result = sd_future_result(s->group);
         return 0;
 }
 
@@ -744,12 +746,12 @@ static int parent_await_driver(void *userdata) {
         s->ready = true;
         ASSERT_EQ(sd_fiber_suspend(), s->result);
 
-        /* Parent cancellation does not replace the group's result. A later await of the resolved
+        /* Parent cancellation does not replace the group's result, and a later await of the
-         * group returns that result, just like sd_future_result(). */
+         * resolved group completes immediately. */
         if (s->result == -ECANCELED) {
                 ASSERT_EQ(sd_future_state(s->group), SD_FUTURE_RESOLVED);
                 ASSERT_ERROR(sd_future_result(s->group), EIO);
-                ASSERT_ERROR(sd_fiber_await(s->group), EIO);
+                ASSERT_OK_ZERO(sd_fiber_await(s->group));
         }
         return 0;
 }
@@ -830,7 +832,14 @@ TEST(future_group_external_cancel_leaves_parent_running) {
 }
 
 static int peer_await_driver(void *userdata) {
-        return sd_fiber_await(userdata);
+        sd_future *group = ASSERT_PTR(userdata);
+        int r;
+
+        r = sd_fiber_await(group);
+        if (r < 0)
+                return r;
+
+        return sd_future_result(group);
 }
 
 TEST(future_group_peer_await_does_not_suppress_parent_cancel) {

src/shared/qmp-client.c

@@ -977,16 +977,14 @@ static int qmp_client_call_suspend(
         if (r < 0)
                 return r;
 
+        /* An interruption takes precedence even if the reply arrived in the same tick: the wait
+         * consumed the queued cancellation or timeout, and returning the reply instead would drop it. */
         r = sd_fiber_await(call);
-
+        if (r < 0)
-        /* If the future isn't resolved, the suspend was interrupted before a reply arrived (fiber
-         * cancelled, fiber-wide SD_FIBER_TIMEOUT scope expired, …). There's no reply to extract,
-         * so surface the resume error directly. When the future is resolved, future_get_qmp_reply()
-         * already encodes success (1), QMP-level error (-EIO with the desc captured if asked for),
-         * and no-reply (negative future result) — pass it through. */
-        if (sd_future_state(call) != SD_FUTURE_RESOLVED)
                 return r;
 
+        /* future_get_qmp_reply() encodes success (1), QMP-level error (-EIO with the desc captured if
+         * asked for), and no-reply (negative future result) — pass it through. */
         return future_get_qmp_reply(call, ret_result, ret_error_desc);
 }
 

src/systemd/sd-future.h

@@ -141,11 +141,12 @@ sd_future* sd_fiber_get_awaiting(sd_future *f);
 
 int sd_fiber_yield(void);
 int sd_fiber_sleep(uint64_t usec);
-/* Return the target's result on completion, or a wait error if the calling fiber is interrupted or
+/* Suspend until the target has resolved. Returns 0 once it has, and a negative error only if the wait
- * the wait fails. An interrupted wait does not imply that the target has resolved; once it has,
+ * itself did not complete: the calling fiber was interrupted (-ECANCELED, -ETIME), woken for an
- * sd_future_result() gives its outcome independently of the wait's return value. An already-resolved
+ * unrelated reason (-EBUSY), or the wait could not be set up. An interrupted wait does not imply that
- * target returns its result without consuming a pending interruption. The target must belong to the
+ * the target has resolved. The target's outcome is available through sd_future_result() afterwards;
- * calling fiber's event loop. */
+ * the caller must hold its own reference to read it. An already-resolved target returns 0 without
+ * consuming a pending interruption. The target must belong to the calling fiber's event loop. */
 int sd_fiber_await(sd_future *target);
 int sd_fiber_suspend(void);
 int sd_fiber_resume(sd_future *f, int result);

src/test/test-qmp-client.c

@@ -419,7 +419,7 @@ static int qmp_client_call_fiber(void *userdata) {
         sd_json_variant *status = ASSERT_NOT_NULL(sd_json_variant_by_key(result, "status"));
         ASSERT_STREQ(sd_json_variant_string(status), "running");
 
-        /* QMP-level error: future resolves with -EIO, sd_fiber_await() returns -EIO, and
+        /* QMP-level error: the wait completes, the future resolves with -EIO, and
          * future_get_qmp_reply() also returns -EIO (mirroring future_get_bus_reply()) — with the
          * detailed description captured via error_desc on top, and result left NULL. */
         f = sd_future_unref(f);
@@ -427,7 +427,7 @@ static int qmp_client_call_fiber(void *userdata) {
         error_desc = mfree(error_desc);
 
         ASSERT_OK(qmp_client_call_future(client, "stop", NULL, &f));
-        ASSERT_ERROR(sd_fiber_await(f), EIO);
+        ASSERT_OK_ZERO(sd_fiber_await(f));
         ASSERT_ERROR(sd_future_result(f), EIO);
         ASSERT_ERROR(future_get_qmp_reply(f, &result, &error_desc), EIO);
 
@@ -541,6 +541,7 @@ static int qmp_client_call_interrupted_fiber(void *userdata) {
         ASSERT_NOT_NULL(result);
         ASSERT_NULL(error_desc);
         ASSERT_OK_ZERO(sd_fiber_await(mock));
+        ASSERT_OK_ZERO(sd_future_result(mock));
         ASSERT_OK_ZERO(sd_fiber_yield());
         ASSERT_OK(sd_event_exit(sd_fiber_get_event(), 0));
         return 0;