[libcxx-commits] [libcxx] [libc++] Don't try to wait on a thread that hasn't started in std::async, take 2 (PR #130145)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

<details>
<summary>Changes</summary>

If the creation of a thread fails, this causes an idle loop that will never end because the thread wasn't started in the first place.

This also adds a test for the regression reported in #<!-- -->125433 to make sure we're not reintroducing it later.

Fixes #<!-- -->125428


---
Full diff: https://github.com/llvm/llvm-project/pull/130145.diff


3 Files Affected:

- (modified) libcxx/include/future (+10-1) 
- (added) libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp (+60) 
- (added) libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp (+37) 


``````````diff
diff --git a/libcxx/include/future b/libcxx/include/future
index 3dfcce80a977d..d791a6f3a5581 100644
--- a/libcxx/include/future
+++ b/libcxx/include/future
@@ -1823,7 +1823,16 @@ 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();
+#if _LIBCPP_HAS_EXCEPTIONS
+  try {
+#endif
+    std::thread(&__async_assoc_state<_Rp, _Fp>::__execute, __h.get()).detach();
+#if _LIBCPP_HAS_EXCEPTIONS
+  } catch (...) {
+    __h->__make_ready();
+    throw;
+  }
+#endif
   return future<_Rp>(__h.get());
 }
 
diff --git a/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp b/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp
new file mode 100644
index 0000000000000..9ab8296d49af1
--- /dev/null
+++ b/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp
@@ -0,0 +1,60 @@
+//===----------------------------------------------------------------------===//
+//
+// 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, no-exceptions
+
+// ASan seems to try to create threadsm which obviouly doesn't work in this test.
+// UNSUPPORTED: asan
+
+// UNSUPPORTED: c++03
+
+// There is no way to limit the number of threads on windows
+// UNSUPPORTED: windows
+
+// AIX and macOS seem to limit the number of processes, not threads via RLIMIT_NPROC
+// XFAIL: target={{.+}}-aix{{.*}}
+// XFAIL: target={{.+}}-apple-{{.*}}
+
+// This test makes sure that we fail gracefully in care the thread creation fails. This is only reliably possible on
+// systems that allow limiting the number of threads that can be created. See https://llvm.org/PR125428 for more details
+
+#include <cassert>
+#include <future>
+#include <system_error>
+
+#if __has_include(<sys/resource.h>)
+#  include <sys/resource.h>
+#  ifdef RLIMIT_NPROC
+void force_thread_creation_failure() {
+  rlimit lim = {1, 1};
+  assert(setrlimit(RLIMIT_NPROC, &lim) == 0);
+}
+#  else
+#    error "No known way to force only one thread being available"
+#  endif
+#else
+#  error "No known way to force only one thread being available"
+#endif
+
+int main(int, char**) {
+  force_thread_creation_failure();
+
+  try {
+    std::future<int> fut = std::async(std::launch::async, [] { return 1; });
+    assert(false);
+  } catch (const std::system_error&) {
+  }
+
+  try {
+    std::future<void> fut = std::async(std::launch::async, [] { return; });
+    assert(false);
+  } catch (const std::system_error&) {
+  }
+
+  return 0;
+}
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..ff771a68c6bef
--- /dev/null
+++ b/libcxx/test/std/thread/futures/futures.async/wait_on_destruct.pass.cpp
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// This test uses std::atomic interfaces that are only available in C++20
+// UNSUPPORTED: c++11, c++14, c++17
+
+// 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;
+  std::unique_lock lock(mux);
+  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);
+  lock.unlock();
+}

``````````

</details>


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