diff --git a/src/libsystemd/sd-bus/bus-future.c b/src/libsystemd/sd-bus/bus-future.c index 9e72ba2766..cc8c5f03cc 100644 --- a/src/libsystemd/sd-bus/bus-future.c +++ b/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 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) + if (r < 0) 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); } diff --git a/src/libsystemd/sd-bus/test-bus-fiber.c b/src/libsystemd/sd-bus/test-bus-fiber.c index 179f8943da..09dbb7ecf9 100644 --- a/src/libsystemd/sd-bus/test-bus-fiber.c +++ b/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); } diff --git a/src/libsystemd/sd-bus/test-bus-watch-bind.c b/src/libsystemd/sd-bus/test-bus-watch-bind.c index 6561633b8a..bbcd70d1dd 100644 --- a/src/libsystemd/sd-bus/test-bus-watch-bind.c +++ b/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)); diff --git a/src/libsystemd/sd-event/event-future.c b/src/libsystemd/sd-event/event-future.c index 24ba19d678..e2e9a9b9c5 100644 --- a/src/libsystemd/sd-event/event-future.c +++ b/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); diff --git a/src/libsystemd/sd-future/fiber-io.c b/src/libsystemd/sd-future/fiber-io.c index b5c1f31245..e405fda3eb 100644 --- a/src/libsystemd/sd-future/fiber-io.c +++ b/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 diff --git a/src/libsystemd/sd-future/fiber.c b/src/libsystemd/sd-future/fiber.c index 19075c68fa..7c97524c72 100644 --- a/src/libsystemd/sd-future/fiber.c +++ b/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 sd_future_result(target); + return 0; } sd_future* sd_future_cancel_wait_unref(sd_future *f) { diff --git a/src/libsystemd/sd-future/test-fiber.c b/src/libsystemd/sd-future/test-fiber.c index d9234892e6..80b080bd26 100644 --- a/src/libsystemd/sd-future/test-fiber.c +++ b/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_EQ(sd_fiber_await(awaited), result); /* Already resolved. */ + ASSERT_OK_ZERO(sd_fiber_await(awaited)); /* Pending. */ + 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); diff --git a/src/libsystemd/sd-future/test-future-group.c b/src/libsystemd/sd-future/test-future-group.c index b33f83348a..a924b8a1e9 100644 --- a/src/libsystemd/sd-future/test-future-group.c +++ b/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 - * makes sd_fiber_await return -EINVAL — we don't ASSERT_OK that. The child is now - * RESOLVED and we can add it to the group. */ - (void) sd_fiber_await(s->child); + /* Drive the pre-built child to completion. The defer resolves with -EINVAL, but the wait + * itself succeeds. The child is now RESOLVED and we can add it to the group. */ + ASSERT_OK_ZERO(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 - * group returns that result, just like sd_future_result(). */ + /* Parent cancellation does not replace the group's result, and a later await of the + * 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) { diff --git a/src/shared/qmp-client.c b/src/shared/qmp-client.c index 42e7eb32bd..fa911cbfd6 100644 --- a/src/shared/qmp-client.c +++ b/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 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) + if (r < 0) 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); } diff --git a/src/systemd/sd-future.h b/src/systemd/sd-future.h index 93b76e4f50..a7e8b936de 100644 --- a/src/systemd/sd-future.h +++ b/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 - * the wait fails. An interrupted wait does not imply that the target has resolved; once it has, - * sd_future_result() gives its outcome independently of the wait's return value. An already-resolved - * target returns its result without consuming a pending interruption. The target must belong to the - * calling fiber's event loop. */ +/* Suspend until the target has resolved. Returns 0 once it has, and a negative error only if the wait + * itself did not complete: the calling fiber was interrupted (-ECANCELED, -ETIME), woken for an + * unrelated reason (-EBUSY), or the wait could not be set up. An interrupted wait does not imply that + * the target has resolved. The target's outcome is available through sd_future_result() afterwards; + * 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); diff --git a/src/test/test-qmp-client.c b/src/test/test-qmp-client.c index 9f6429ea2c..a9203a7e2a 100644 --- a/src/test/test-qmp-client.c +++ b/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;