From b698069b75206584bd2191188008177a3bdef7ed Mon Sep 17 00:00:00 2001 From: Josh Baldwin Date: Fri, 2 Aug 2024 11:31:31 -0600 Subject: [PATCH 1/2] Upgrade supported opensuse/leap:15.6 (#275) 15.2 is having issues with nodejs and glibc /__e/node20/bin/node: /lib64/libm.so.6: version `GLIBC_2.27' not found (required by /__e/node20/bin/node Closes #274 --- .githooks/readme-template.md | 2 +- .github/workflows/ci-opensuse.yml | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.githooks/readme-template.md b/.githooks/readme-template.md index 3f6d796a..673fe343 100644 --- a/.githooks/readme-template.md +++ b/.githooks/readme-template.md @@ -433,7 +433,7 @@ Transfer/sec: 18.33MB * ubuntu:20.04, 22.04 * fedora:32-40 - * openSUSE/leap:15.2 + * openSUSE/leap:15.6 * Windows 2022 * Emscripten 3.1.45 * MacOS 12 diff --git a/.github/workflows/ci-opensuse.yml b/.github/workflows/ci-opensuse.yml index 1748f16a..7ae079ea 100644 --- a/.github/workflows/ci-opensuse.yml +++ b/.github/workflows/ci-opensuse.yml @@ -12,7 +12,7 @@ jobs: cxx_standard: [20] libcoro_feature_networking: [ {enabled: ON, tls: ON} ] container: - image: opensuse/leap:15.2 + image: opensuse/leap:15.6 steps: - name: zypper run: | diff --git a/README.md b/README.md index d6719952..4db94af6 100644 --- a/README.md +++ b/README.md @@ -1222,7 +1222,7 @@ Transfer/sec: 18.33MB * ubuntu:20.04, 22.04 * fedora:32-40 - * openSUSE/leap:15.2 + * openSUSE/leap:15.6 * Windows 2022 * Emscripten 3.1.45 * MacOS 12 From ee02dc8bd526ec741750f1bd8c4f327a145434f7 Mon Sep 17 00:00:00 2001 From: Josh Baldwin Date: Fri, 2 Aug 2024 11:51:42 -0600 Subject: [PATCH 2/2] Use lock for sync_wait completion (#272) * Use lock for sync_wait completion * release/acquire memory ordering has a race condition * also reproduced on seq_cst * requiring a lock around the std::condition_variable to properly and always wake up the waiting sync_wait thread, this is necessary for correctness over speed Closes #270 --- include/coro/sync_wait.hpp | 2 +- src/sync_wait.cpp | 9 +++++--- test/test_sync_wait.cpp | 46 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/include/coro/sync_wait.hpp b/include/coro/sync_wait.hpp index 5a20def4..b403e953 100644 --- a/include/coro/sync_wait.hpp +++ b/include/coro/sync_wait.hpp @@ -328,7 +328,7 @@ auto sync_wait(awaitable_type&& a) -> decltype(auto) // For non-trivial types (or possibly types that don't fit in a register) // the compiler will end up calling the ~return_type() when the promise // is destructed at the end of sync_wait(). This causes the return_type - // object to also be destructed causingn the final return/move from + // object to also be destructed causing the final return/move from // sync_wait() to be a 'use after free' bug. To work around this the result // must be moved off the promise object before the promise is destructed. // Other solutions could be heap allocating the return_type but that has diff --git a/src/sync_wait.cpp b/src/sync_wait.cpp index e610ad25..509aff1c 100644 --- a/src/sync_wait.cpp +++ b/src/sync_wait.cpp @@ -8,19 +8,22 @@ sync_wait_event::sync_wait_event(bool initially_set) : m_set(initially_set) auto sync_wait_event::set() noexcept -> void { - m_set.exchange(true, std::memory_order::release); + // issue-270 100~ task's on a thread_pool within sync_wait(when_all(tasks)) can cause a deadlock/hang if using + // release/acquire or even seq_cst. + m_set.exchange(true, std::memory_order::seq_cst); + std::unique_lock lk{m_mutex}; m_cv.notify_all(); } auto sync_wait_event::reset() noexcept -> void { - m_set.exchange(false, std::memory_order::release); + m_set.exchange(false, std::memory_order::seq_cst); } auto sync_wait_event::wait() noexcept -> void { std::unique_lock lk{m_mutex}; - m_cv.wait(lk, [this] { return m_set.load(std::memory_order::acquire); }); + m_cv.wait(lk, [this] { return m_set.load(std::memory_order::seq_cst); }); } } // namespace coro::detail diff --git a/test/test_sync_wait.cpp b/test/test_sync_wait.cpp index 109956da..2099d2c1 100644 --- a/test/test_sync_wait.cpp +++ b/test/test_sync_wait.cpp @@ -3,6 +3,8 @@ #include #include +#include +#include TEST_CASE("sync_wait simple integer return", "[sync_wait]") { @@ -62,3 +64,47 @@ TEST_CASE("sync_wait task that throws", "[sync_wait]") REQUIRE_THROWS(coro::sync_wait(f())); } + +TEST_CASE("sync_wait very rarely hangs issue-270", "[sync_wait]") +{ + coro::thread_pool tp{}; + + const int ITERATIONS = 100; + + std::unordered_set data{}; + data.reserve(ITERATIONS); + + std::random_device dev; + std::mt19937 rng(dev()); + std::uniform_int_distribution dist(0, ITERATIONS); + + for (int i = 0; i < ITERATIONS; ++i) + { + data.insert(dist(rng)); + } + + std::atomic count{0}; + + auto make_task = [&](int i) -> coro::task + { + co_await tp.schedule(); + + if (data.find(i) != data.end()) + { + count.fetch_add(1); + } + + co_return; + }; + + std::vector> tasks{}; + tasks.reserve(ITERATIONS); + for (int i = 0; i < ITERATIONS; ++i) + { + tasks.emplace_back(make_task(i)); + } + + coro::sync_wait(coro::when_all(std::move(tasks))); + + REQUIRE(count > 0); +}