[libcxx-commits] [libcxx] [libc++] Fix std::future not waiting until the thread is finished to clean up (PR #130145)

[Nikolas Klauser via libcxx-commits]

https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/130145

This has been introduced by #125433. Now, instead of checking for `__constructed`, we're setting the `__async_assoc_state` to `ready` in case of a thread creation failure to ensure that we're not trying to wait.


>From 5397c2e71519e362b4edba6f5af69011ee7eb8e2 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Thu, 6 Mar 2025 18:14:59 +0100
Subject: [PATCH] [libc++] Fix std::future not waiting until the thread is
 finished to clean up

---
 libcxx/include/future                         | 13 ++++---
 .../futures.async/wait_on_destruct.pass.cpp   | 34 +++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp

diff --git a/libcxx/include/future b/libcxx/include/future
index 22b387d3ba094..fc4ca216a6b40 100644
--- a/libcxx/include/future
+++ b/libcxx/include/future
@@ -865,8 +865,7 @@ void __async_assoc_state<_Rp, _Fp>::__execute() {
 
 template <class _Rp, class _Fp>
 void __async_assoc_state<_Rp, _Fp>::__on_zero_shared() _NOEXCEPT {
-  if (base::__state_ & base::__constructed)
-    this->wait();
+  this->wait();
   base::__on_zero_shared();
 }
 
@@ -903,8 +902,7 @@ void __async_assoc_state<void, _Fp>::__execute() {
 
 template <class _Fp>
 void __async_assoc_state<void, _Fp>::__on_zero_shared() _NOEXCEPT {
-  if (base::__state_ & base::__constructed)
-    this->wait();
+  this->wait();
   base::__on_zero_shared();
 }
 
@@ -1831,7 +1829,12 @@ template <class _Rp, class _Fp>
 _LIBCPP_HIDE_FROM_ABI future<_Rp> __make_async_assoc_state(_Fp&& __f) {
   unique_ptr<__async_assoc_state<_Rp, _Fp>, __release_shared_count> __h(
       new __async_assoc_state<_Rp, _Fp>(std::forward<_Fp>(__f)));
-  std::thread(&__async_assoc_state<_Rp, _Fp>::__execute, __h.get()).detach();
+  try {
+    std::thread(&__async_assoc_state<_Rp, _Fp>::__execute, __h.get()).detach();
+  } catch (...) {
+    __h->__make_ready();
+    throw;
+  }
   return future<_Rp>(__h.get());
 }
 
diff --git a/libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp b/libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp
new file mode 100644
index 0000000000000..5e68e1c6a8578
--- /dev/null
+++ b/libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp
@@ -0,0 +1,34 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: no-threads
+// UNSUPPORTED: c++03
+
+// Make sure that the `future` destructor keeps the data alive until the thread finished. This test fails by triggering
+// TSan. It may not be observable by normal means.
+
+#include <atomic>
+#include <future>
+#include <mutex>
+
+std::mutex mux;
+
+int main() {
+  using namespace std::chrono_literals;
+  mux.lock();
+  std::atomic<bool> in_async = false;
+  auto v = std::async(std::launch::async, [&in_async, value = 1]() mutable {
+    in_async = true;
+    in_async.notify_all();
+    std::scoped_lock thread_lock(mux);
+    value = 4;
+    (void)value;
+  });
+  in_async.wait(true);
+  mux.unlock();
+}