Skip to content

Sequence operation states that don't retain sender-owned data: write_env over a sequence (#2294) and exec::iterate #2305

Description

@solarispika

While looking at sequence senders, I found two places where subscribe returns an operation state that keeps references into storage owned by the sender, without retaining that storage itself. The operation then depends on the sender outliving it, and both cases fail under ASAN once the sender is destroyed between subscribe and start.

This is contrary to the sender/operation lifetime model in [exec.async.ops]/9 ("The lifetime of an asynchronous operation's associated operation state does not depend on the lifetimes of either the sender or the receiver from which it was created"). I know exec::subscribe and the sequence algorithms are experimental and not covered by that wording directly. Still, the regular-sender path follows it: the default operation-state construction stores the sender's data decayed, forwarded from the sender through get-state ([exec.snd.expos]/27 and /37; __defaults::__get_state and __state in __basic_sender.hpp). The sequence algorithm closest to iterate, static_thread_pool's schedule_all, also owns its decayed range (static_thread_pool.hpp:1582, :1710-1714).

Tested on main @ f4c123f7 with clang++ -std=c++20 -g -O0 -fsanitize=address (Apple clang 17, arm64).

1. write_env over a sequence sender (transparent-adaptor path from #2294)

#2294 made write_env transparent to sequence senders (the write_env half of #2053). On that path, the env that write_env injects isn't owned by the resulting operation state.

__sequence_adaptor_traits<__write_env_t>::__child_env_fn (include/exec/sequence_senders.hpp:264-272) builds the child env with __join(__data, env), where __data is an lvalue _Data const &. So the result is env<_Data const &, ...>, which references the data stored in the write_env sender. The transparent branch of subscribe (:1070-1088) stores that env in __adaptor_rcvr::__env_ (:925-928) inside the child's operation state, and nothing else on that path keeps the data alive. The regular write_env path is different: its operation state owns a decayed copy, and __write_env.hpp:53 joins __state.__data_. When the sender is a temporary, its storage can be gone before start(), and a later query that reaches the injected data then reads through a dangling reference. A push-driven source reading get_scheduler when it dispatches an item is one example.

The tests in test_write_env_sequence.cpp don't hit this, because the test sequence never queries the injected env after subscribe returns.

Repro

#include <exec/sequence_senders.hpp>
#include <stdexec/execution.hpp>
#include <cstdio>
#include <optional>
#include <string>

struct tag_q : stdexec::__query<tag_q> {};
struct my_prop {
  std::string s;
  auto query(tag_q) const noexcept -> std::string const & { return s; }
};

struct seq {
  using sender_concept = exec::sequence_sender_tag;
  using item_types = exec::item_types<decltype(stdexec::just(int{}))>;
  using completion_signatures =
    stdexec::completion_signatures<stdexec::set_value_t(), stdexec::set_stopped_t()>;

  template <class R>
  struct op {
    using operation_state_concept = stdexec::operation_state_t;
    R r_;
    void start() noexcept {
      std::printf("%s\n", tag_q{}(stdexec::get_env(r_)).c_str()); // reads the injected env
      stdexec::set_value(static_cast<R &&>(r_));
    }
  };

  template <stdexec::receiver R>
  auto subscribe(R r) const -> op<R> { return {std::move(r)}; }
};

struct rcvr {
  using receiver_concept = stdexec::receiver_tag;
  template <class I> auto set_next(I &&i) noexcept { return static_cast<I &&>(i); }
  void set_value() noexcept {}
  void set_stopped() noexcept {}
  template <class E> void set_error(E &&) noexcept {}
};

int main() {
  using op_t = decltype(exec::subscribe(stdexec::write_env(seq{}, my_prop{}), rcvr{}));
  std::optional<op_t> op;
  op.emplace(stdexec::__emplace_from{[] {
    return exec::subscribe(stdexec::write_env(seq{}, my_prop{std::string(64, 'x')}), rcvr{});
  }});
  stdexec::start(*op); // the write_env sender temporary is already gone here
}
ERROR: AddressSanitizer: heap-use-after-free
READ of size 2 ...
    #2 in seq::op<exec::__sequence_sndr::__adaptor_rcvr<rcvr, my_prop const&>>::start()
freed by thread T0 here:
    ...
    #2 in my_prop::~my_prop()
    #4 in stdexec::__tup::__tuple<stdexec::__write_env_t, my_prop, seq>::~__tuple()
    #7 in exec::__sequence_sndr::subscribe_t::operator()<...>

The receiver type shows it directly: __adaptor_rcvr<rcvr, my_prop const&>.

Direction for a fix

The child env needs to own the data, as it does on the regular write_env path. As a quick check of that direction, decay-copying in __child_env_fn (__join_env_t<_Data, _Env> / __join(_Data(__data), ...)) makes the repro run clean under ASAN, and test_write_env_sequence.cpp still passes (built standalone, 6/6). I haven't run the full suite.

That two-line change isn't a complete fix, though:

  • It breaks move-only environments. A write_env whose env holds a std::unique_ptr compiles today, but fails with the copy. The data would need to be moved out of an rvalue sender. Right now the hook only receives _Data const &, and the runtime branch forwards decltype(__data) instead of the sender's value category.
  • __child_env_fn::operator() is unconditionally noexcept, so a throwing copy or move would terminate. The noexcept computed for the transparent branch of subscribe would also have to account for constructing the env, not only the child's subscribe.

Since the traits hook is open, it may also be worth documenting that whatever a __child_env_fn returns gets stored in the operation state and has to stay valid for that operation state's whole lifetime.

2. exec::iterate with a range that owns its elements

exec::iterate(range) decay-copies the range into the sender. Its subscribe, however, builds the operation state from ranges::begin/ranges::end of that sender-owned copy, and the operation stores only the iterator and sentinel, not the range. For views such as std::span or a subrange over caller-owned data that is fine, because the iterators point at the caller's storage. When the range itself owns its elements (a std::vector, say), the elements live in the sender, and destroying the sender after subscribe leaves the operation holding dangling iterators.

Where

  • iterate_t::operator() (include/exec/sequence/iterate.hpp:199-204) stores __decay_t<_Range> in the sender.
  • iterate_t::subscribe (:228-234) → __subscribe_fn::operator() (:181-191) constructs __operation{ranges::begin(__range), ranges::end(__range), rcvr} from that member.
  • __operation_base_base (:54-58) holds only __iterator_, __sentinel_, and the trampoline scheduler.

Repro (lvalue subscribe)

#include <exec/sequence/iterate.hpp>
#include <exec/sequence_senders.hpp>
#include <stdexec/execution.hpp>

#include <cstdio>
#include <optional>
#include <vector>

struct printing_receiver {
  using receiver_concept = stdexec::receiver_tag;

  template <class Item> auto set_next(Item &&item) & noexcept {
    return stdexec::then(static_cast<Item &&>(item),
                         [](int v) noexcept { std::printf("item %d\n", v); });
  }

  void set_value() noexcept { std::puts("sequence done"); }
  void set_error(std::exception_ptr) noexcept { std::puts("sequence error"); }
  void set_stopped() noexcept { std::puts("sequence stopped"); }
  auto get_env() const noexcept { return stdexec::env<>{}; }
};

int main() {
  using seq_t = decltype(exec::iterate(std::vector<int>{}));
  using op_t = exec::subscribe_result_t<seq_t &, printing_receiver>;

  std::optional<op_t> op;
  {
    std::vector<int> v(64);
    for (int i = 0; i < 64; ++i) v[i] = i;
    seq_t seq = exec::iterate(std::move(v)); // the sender owns the vector
    op.emplace(stdexec::__emplace_from{
      [&] { return exec::subscribe(seq, printing_receiver{}); }});
  } // the sender is destroyed here; the op still holds iterators into its vector
  stdexec::start(*op);
}
ERROR: AddressSanitizer: heap-use-after-free ... READ of size 4
    #3 in exec::__iterate::__item_operation<std::__wrap_iter<int*>, ...>::start() iterate.hpp:81
    #11 in exec::__iterate::__operation<std::__wrap_iter<int*>, ..., printing_receiver>::__start_next() iterate.hpp:155
freed by thread T0 here:
    #5 in std::vector<int>::~vector()
    #7 in stdexec::__tup::__tuple<exec::__iterate::iterate_t, std::vector<int>>::~__tuple() __tuple.hpp:58
    #8 in exec::__seqexpr<...>::~__seqexpr() basic_sequence.hpp:43

rvalue subscribe doesn't compile

Subscribing an rvalue iterate(std::vector<int>{...}) fails at iterate.hpp:182 and :188 with call to deleted function call operator in type 'const __begin::__fn', because ranges::begin is called on an rvalue non-borrowed range. So for an owning container, the lvalue path is the only one that compiles, and it borrows from the sender. (For example, sync_wait(iterate(std::vector{...}) | ...) doesn't compile, while auto s = iterate(std::vector{...}) | ...; sync_wait(s); does, and works only because s outlives the operation.)

Possible direction

The operation could own the range, the same way the default get-state owns a sender's data: copy it from an lvalue sender and move it from an rvalue one. That would also make rvalue subscribe well-formed. Some details I noticed:

  • The iterators are currently stored in a base class (__operation_base_base), so a range member added in a derived class can't be initialized before them. The range has to be constructed in the operation's final storage first, then begin/end taken from that stored range. The layout needs to change, or the iterators need to be initialized after the range exists.
  • Destruction order matters: the child item operations must be destroyed before the range they iterate.
  • The operation is currently immovable (its child __optional is), so moving the op doesn't invalidate iterators today. Any refactor should keep that property.
  • For a const lvalue sender, the owned copy is mutable, so its iterator/reference types can differ from the original's. item_types need to be computed consistently with what the operation actually iterates.
  • The noexcept specification needs to cover constructing the range as well as begin/end.

Another option would be an API restriction requiring the stored (decayed) range type to be a borrowed_range. Checking the incoming forwarding-reference type wouldn't be enough, since std::vector<int>& is a borrowed range even though the sender stores a std::vector. That would reject owning containers that the factory accepts today, so it's a design question rather than a ready fix.

Related: #2053, #2294.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions