From 25b9b4d27d5c5909c7553774e4590e4f64403fc7 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 27 Jul 2026 10:29:57 +0100 Subject: [PATCH] Fix JobQueue_test resume() contract violation and Coroutine_test timeout hazards - PostCoroTest2 called runner->resume() directly from the test thread, violating the documented precondition on CoroTaskRunner::resume() (a post() must precede every resume) and driving runCount_ negative. Rewrite the loop as post()+join(); the non-atomic yieldCount now also verifies the happens-before edge join() provides via mutexRun_. - PostCoroTest3 wrote false into an already-false flag, so it could not detect the coroutine running after stop(). Write true and assert the flag stays false. - Coroutine_test: early-return when a Gate waitFor() times out instead of falling through to c->join()/a[i]->join(), which would deref a null shared_ptr (correctOrder, threadSpecificStorage first loop) or block indefinitely (second loop). --- src/test/core/Coroutine_test.cpp | 9 ++++++--- src/test/core/JobQueue_test.cpp | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/test/core/Coroutine_test.cpp b/src/test/core/Coroutine_test.cpp index d86e3b7463..e6180f2161 100644 --- a/src/test/core/Coroutine_test.cpp +++ b/src/test/core/Coroutine_test.cpp @@ -78,7 +78,8 @@ public: g2p->signal(); co_return; }); - BEAST_EXPECT(g1.waitFor(5s)); + if (!BEAST_EXPECT(g1.waitFor(5s))) + return; c->join(); c->post(); BEAST_EXPECT(g2.waitFor(5s)); @@ -158,13 +159,15 @@ public: this->BEAST_EXPECT(**lvp == id); co_return; }); - BEAST_EXPECT(g.waitFor(5s)); + if (!BEAST_EXPECT(g.waitFor(5s))) + return; a[i]->join(); } for (auto const& c : a) { c->post(); - BEAST_EXPECT(g.waitFor(5s)); + if (!BEAST_EXPECT(g.waitFor(5s))) + return; c->join(); } for (auto const& c : a) diff --git a/src/test/core/JobQueue_test.cpp b/src/test/core/JobQueue_test.cpp index 85c4e99d16..1ab52e59f4 100644 --- a/src/test/core/JobQueue_test.cpp +++ b/src/test/core/JobQueue_test.cpp @@ -87,7 +87,7 @@ class JobQueue_test : public beast::unit_test::Suite BEAST_EXPECT(yieldCount == 4); } { - // Test repeated resume()s until the coroutine completes. + // Test repeated post()+join()s until the coroutine completes. int yieldCount{0}; auto const runner = jQueue.postCoroTask( JtClient, "PostCoroTest2", [ycp = &yieldCount](auto runner) -> CoroTask { @@ -106,11 +106,18 @@ class JobQueue_test : public beast::unit_test::Suite // Wait for the Job to run and yield. runner->join(); - // Now resume until the CoroTaskRunner says it is done. + // Now post()+join() until the CoroTaskRunner says it is done. + // resume() requires a prior post() (see the precondition on + // CoroTaskRunner::resume()), so the posted job performs the + // resume and join() blocks until it completes. yieldCount is + // deliberately not atomic: the mutexRun_ handoff inside join() + // must provide the happens-before edge that makes the + // increment visible to this thread. int old = yieldCount; while (runner->runnable()) { - runner->resume(); // Resume runs synchronously on this thread. + BEAST_EXPECT(runner->post()); + runner->join(); BEAST_EXPECT(++old == yieldCount); } BEAST_EXPECT(yieldCount == 4); @@ -128,10 +135,11 @@ class JobQueue_test : public beast::unit_test::Suite bool unprotected = false; auto const runner = jQueue.postCoroTask( JtClient, "PostCoroTest3", [up = &unprotected](auto) -> CoroTask { - *up = false; + *up = true; co_return; }); BEAST_EXPECT(runner == nullptr); + BEAST_EXPECT(unprotected == false); } }