Merge pull request #966 from msimberg/when-all-vector-fix-num-predece…

…ssors-access

Fix potential use-after-free in `when_all_vector`

libs/pika/execution/include/pika/execution/algorithms/when_all_vector.hpp

@@ -338,7 +338,12 @@ namespace pika::when_all_vector_detail {
                 // the predecessors to signal completion.
                 else
                 {
-                    for (std::size_t i = 0; i < os.num_predecessors; ++i)
+                    // After the call to start on the last child operation state the current
+                    // when_all_vector operation state may already have been released. We read the
+                    // number of predecessors from the operation state into a stack-local variable
+                    // so that the loop can end without reading freed memory.
+                    auto const num_predecessors = os.num_predecessors;
+                    for (std::size_t i = 0; i < num_predecessors; ++i)
                     {
                         pika::execution::experimental::start(os.op_states.get()[i].value());
                     }

libs/pika/execution/tests/unit/algorithm_when_all_vector.cpp

@@ -211,6 +211,20 @@ int main()
         PIKA_TEST(set_value_called);
     }
 
+    // This test checks that when_all_vector::start doesn't access the number of predecessors after
+    // the child operation states have been started. Starting the last operation state may release
+    // the when_all_vector operation state after which the number of predecessors can't be accessed
+    // anymore. Using start_detached here forces a heap allocation so that reading the number of
+    // predecessors is likely to trigger a segfault or failure with sanitizers or valgrind.
+    {
+        std::vector<ex::unique_any_sender<int>> senders;
+        senders.emplace_back(ex::just(42));
+        senders.emplace_back(ex::just(43));
+        senders.emplace_back(ex::just(44));
+        auto s = ex::when_all_vector(std::move(senders));
+        ex::start_detached(std::move(s));
+    }
+
     // Test a combination with when_all
     {
         std::atomic<bool> set_value_called{false};