search

cplusplus / sender-receiver

Issues list for P2300
Apache License 2.0
16 stars 3 forks source link

[exec.schedule.from] Potential access to destroyed state in `impls-for::complete` #233

Open lewissbaker opened 1 month ago

lewissbaker commented 1 month ago

The current specification of schedule_from algorithm has the following definition for impls-for<schedule_form_t>::complete:

[]<class Tag, class... Args>(auto, auto& state, auto& rcvr, Tag, Args&&... args) noexcept -> void {
  using result_t = decayed-tuple<Tag, Args...>;
  constexpr bool nothrow = is_nothrow_constructible_v<result_t, Tag, Args...>;

  TRY-EVAL(std::move(rcvr), [&]() noexcept(nothrow) {
    state.async-result.template emplace<result_t>(Tag(), std::forward<Args>(args)...);
  }());

  if (state.async-result.valueless_by_exception())
    return;
  if (state.async-result.index() == 0)
    return;

  start(state.op-state);
};

The call to TRY-EVAL here will invoke set_error() on the receiver if the construction of result_t throws an exception. As the call to set_error() can potentially end up destroying the operation-state, the subsequent access of state.async-result.valueless_by_exception() is potentially accessing a dangling reference to state.

Instead, we need to have this function return; after set_error() is called. This may mean we need to directly define the expansion of TRY-EVAL instead of using the macro.

e.g. something like:

[]<class Tag, class... Args>(auto, auto& state, auto& rcvr, Tag, Args&&... args) noexcept -> void {
  using result_t = decayed-tuple<Tag, Args...>;
  constexpr bool nothrow = is_nothrow_constructible_v<result_t, Tag, Args...>;

  try {
    state.async-result.template emplace<result_t>(Tag(), std::forward<Args>(args)...);
  } catch (...) {
    if constexpr (!nothrow) {
      execution::set_error(std::move(rcvr), std::current_exception());
      return;
    }
  }

  assert(state.async-result.index() != 0);
  assert(!state.async-result.valueless_by_exception());

  start(state.op-state);
};