Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Definition of SCHED-ATTRS incorrectly assumes that schedule operations that complete with set_stopped will complete on that scheduler #283

Open
lewissbaker opened this issue Aug 1, 2024 · 2 comments
Labels
bug Something isn't working design P0

Comments

@lewissbaker
Copy link
Collaborator

lewissbaker commented Aug 1, 2024

The SCHED-ATTRS(sch) helper is used in the continues_on() and schedule_from() algorithms and is used to compute the sender-attributes of the sender returned from these algorithms such that they have a get_completion_scheduler<set_value_t> and get_completion_scheduler<set_stopped_t> queries that both return sch.

However, I don't think it's valid to say that a set_stopped result will always complete on the specified scheduler for these algorithms.

In the case that the schedule operation that is trying to transfer execution to the scheduler's context completes with set_stopped, it is not guaranteed that the completion is currently on the scheduler's context (it might be completing synchronously inside a stop-request from an arbitrary thread), and yet the continues_on() operation as a whole will complete with set_stopped, violating the declared get_completion_scheduler<set_stopped_t> query result.

There are a couple of strategies that we can potentially take to eliminate this inconsistency:

  • We can remove the get_completion_scheduler<set_stopped_t> query from SCHED-ATTRS(sch) and only say things about the completion context of a set_value result.
  • We can require the schedule operation to complete with set_value - possibly wrapping the schedule operation with unstoppable() and terminate_on_error_and_stopped(), in which case the set_stopped result will only occur if the predecessor completed with set_stopped. This would also allow saying something about the set_error completion scheduler as well.
  • We can say that the operation as a whole completes with an error if the schedule operation does not complete with set_value (since it failed to meet its post-condition of continuing on that context).
    Possibly combining this with an unstoppable wrapper around the schedule operation.
@lewissbaker lewissbaker added bug Something isn't working P0 design labels Aug 1, 2024
@jiixyj
Copy link

jiixyj commented Oct 11, 2024

I wonder why the senders returned from exec::schedule should be allowed to complete with set_stopped anyway. The current semantics of the scheduler concept (as implied by SCHED-ATTRS) require a set_stopped completion to happen on an execution agent represented by that scheduler. But at that point it might as well complete with set_value, delegating the act of checking/responding to a cancellation request to the following work.

P3284 even proposes to wrap the exec::schedule calls inside schedule_from (and therefore by extension inside continues_on) in unstoppable, making a exec::get_completion_scheduler<exec::set_stopped_t> superfluous AFAICS (edit: ...at least in SCHED-ATTRS). edit3: ...on further thought, the unstoppable would only apply to the schedule sender itself, not to the set_stopped completion from the previous sender that it might propagate on the execution agent.

edit2: In my mind, the only use case for a set_stopped completion of a schedule() sender would be if it wasn't "cheap" to start some work on an execution resource. In that case, start() of the resulting operation state would check for a cancellation request and complete with set_stopped before doing any work. So exactly the inverse meaning of the exec::get_completion_scheduler<exec::set_stopped_t> as specified in SCHED-ATTRS currently. edit3: Striking the last sentence -- the exec::get_completion_scheduler<exec::set_stopped_t in SCHED-ATTRS should only be there for "relaying" set_stopped completions from the previous sender!

@jiixyj
Copy link

jiixyj commented Oct 11, 2024

Sorry for "thinking aloud" in the previous comment. Maybe it becomes clearer when looking at the receiver-type receiver from the schedule_from adapter (https://eel.is/c++draft/exec#schedule.from-9). This is the receiver that is connected to the schedule sender.

struct receiver-type {
    using receiver_concept = receiver_t;
    state-type* state;          // exposition only

    void set_value() && noexcept {
      visit(
        [this]<class Tuple>(Tuple& result) noexcept -> void {
          if constexpr (!same_as<monostate, Tuple>) {
            auto& [tag, ...args] = result;
            tag(std::move(state->rcvr), std::move(args)...);
          }
        },
        state->async-result);
    }

    template<class Error>
    void set_error(Error&& err) && noexcept {
      execution::set_error(std::move(state->rcvr), std::forward<Error>(err));
    }

    void set_stopped() && noexcept {
      execution::set_stopped(std::move(state->rcvr));
    }

    decltype(auto) get_env() const noexcept {
      return FWD-ENV(execution::get_env(state->rcvr));
    }
  };

Looking at this, only if the call to set_stopped of receiver-type does not happen, then it is guaranteed that any set_stopped completions of the whole thing can only happen in the set_value function of receiver-type, thereby making the current formulation of SCHED-ATTRS correct. This can be ensured by one of two things:

  • wrapping the call to schedule(sched) inside schedule_from in unstoppable (your option two, also chosen by P3284)
  • or requiring that every schedule sender's set_stopped completion completes on an associated execution agent (the status quo). But this is a bit nonsensical as noted earlier, as it might as well complete with set_value then.

Going with unstoppable(schedule(sched)) sounds like the way to go, especially since things like task, async_scope's join and on all require "going back" to the scheduler they started from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design P0
Projects
None yet
Development

No branches or pull requests

2 participants