Sequence operation states that don't retain sender-owned data: `write_env` over a sequence (#2294) and `exec::iterate`
Maintainer antworten meist innerhalb von 1 Tag
Bewertung
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Anfängerfreundlichkeit
- 35/100
Rechercherichtung
Start with include/exec/sequence_senders.hpp (__child_env_fn and transparent subscribe) and include/exec/sequence/iterate.hpp (__subscribe_fn and operation storage); the issue includes ASAN repros for both lifetime bugs. Read the regular write_env path and the notes on ownership, value categories, and destruction order before choosing an approach. Done means operations remain valid after their senders are destroyed, including owning ranges and move-only environments; run test_write_env_sequence.cpp and the repros under ASAN.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Beschreibung
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_envwhose env holds astd::unique_ptrcompiles 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 forwardsdecltype(__data)instead of the sender's value category. __child_env_fn::operator()is unconditionallynoexcept, so a throwing copy or move would terminate. Thenoexceptcomputed for the transparent branch ofsubscribewould also have to account for constructing the env, not only the child'ssubscribe.
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, thenbegin/endtaken 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
__optionalis), 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_typesneed to be computed consistently with what the operation actually iterates. - The
noexceptspecification needs to cover constructing the range as well asbegin/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.
- Vorherrschende Sprache
- C++
- Sterne
- 2.5k
- Forks
- 276
- Ø Merge
- 1 T. 16 Std.
- Gemergte PRs (30 T.)
- 45
Entwicklungsumgebung
Dieses Projekt bietet weder Dev-Container noch Dockerfile noch Beitragsleitfaden – die Einrichtung liegt bei Ihnen. Beginnen Sie mit der README; die allgemeinen Schritte stehen in unserem Leitfaden für den ersten Beitrag.
Erste Schritte
- Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
- Forken Sie das Repository und arbeiten Sie in einem Branch.
- Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.
Mehr aus NVIDIA/stdexec
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 68/100
NVIDIA/stdexec#1751 · 2 Kommentare ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 3/5 1-2 Tage Anfängerfreundlichkeit 54/100
Maintainer antworten meist innerhalb von 1 Tag
-
continues_on: hop opstate not connected with secondary environmentEvtl. vergeben @ericniebler hat das vor 19 Tagen übernommen. Offen
Schwierigkeit 4/5 3-5 Tage Anfängerfreundlichkeit 45/100
NVIDIA/stdexec#2268 · 4 Kommentare ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 4/5 3-5 Tage Anfängerfreundlichkeit 48/100
NVIDIA/stdexec#2240 · 5 Kommentare ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 4/5 3-5 Tage Anfängerfreundlichkeit 65/100
Maintainer antworten meist innerhalb von 1 Tag
Ähnliche Issues
-
torch_ops/logprob.cu does not compile with the serving container's nvcc (13.3.73); check_torch_ops.py cannot run as shippedEvtl. vergeben Ein verknüpfter Pull Request ist offen oder bereits gemergt. Offen
Schwierigkeit 2/5 Unter einer Stunde Anfängerfreundlichkeit 72/100
ashhart/TensorFold#535 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 66/100
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
Maintainer antworten meist innerhalb von 1 Tag
-
agent:Windows bug
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 62/100
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 72/100
Maintainer antworten meist innerhalb von 1 Tag