maikel
commented
1 year ago I've looked at this for about an hour and I am not sure if that is actually a code-gen problem of GCC-11. Still investigating.
Closed maikel closed 6 months ago
maikel
commented
1 year ago I've looked at this for about an hour and I am not sure if that is actually a code-gen problem of GCC-11. Still investigating.
ccotter
commented
6 months ago I think this also reproduces the same TSAN diagnostic:
#include <exec/single_thread_context.hpp>
#include <stdexec/execution.hpp>
int main() {
exec::single_thread_context ctx1;
exec::single_thread_context ctx2;
namespace ex = stdexec;
for (int i = 0; i != 10000; ++i) { // enough times to trigger the diagnostic
std::atomic<int> counter{0};
auto split_sndr = ex::just(50) | ex::split() | ex::then([](int x) { return x*2; });
auto sndr = ex::just() | ex::let_value([&counter, split_sndr]() {
++counter;
while (counter != 2) {
// wait
}
return split_sndr;
});
ex::sync_wait(
ex::when_all(
ex::on(ctx1.get_scheduler(), sndr),
ex::on(ctx2.get_scheduler(), sndr))
| ex::then([](auto&&...) { })
);
}
return 0;
}I can't explain why, but changing https://github.com/NVIDIA/stdexec/blob/08619a4/include/stdexec/execution.hpp#L3369 to memory_order_acq_rel silences the diagnostic for my example and the "split is thread-safe" test. From what I can tell, all of the code paths I can think of have a correct release-acquire ordering.
maikel
commented
6 months ago Seeing the potential culprit makes we wonder if we can reduce this into a small piece and see who needs a fix, (us, tsan, gcc)
ccotter
commented
6 months ago Seeing the potential culprit makes we wonder if we can reduce this into a small piece and see who needs a fix, (us, tsan, gcc)
Good idea. I'll see if I can do that.
ccotter
commented
6 months ago This mostly minimal reproducer mimics the same set of atomic operations as what split is doing in the test cases. It looks like GCC-11 flags the code, but GCC-12 does not. Similarly, clang-12 flags the code, but clang-13 does not. I couldn't find anything in my limited search through the commits between clang-12 and 13, but I'm assuming this was a false positive fixed in TSAN itself.
I also confirmed the test_split TSAN error (which I can reliably reproduce if I run the "split is thread-safe" test in an infinite loop) goes away with GCC-12, but is present with GCC-11.
While looking for data races TSAN threw in one run of test.stdexec the following verbose warning at
test_split.cpp:319(test: "split is thread-safe")Maybe this is related to #772