Skip to content

Commit 9190b96

Browse files
bghgaryCopilot
andcommitted
Add deterministic test for WorkQueue destructor deadlock race
Add a test that deterministically triggers the lost-wakeup race condition in WorkQueue::~WorkQueue(). Uses a global arcana testing hook to sleep while holding the queue mutex before wait(), guaranteeing the worker is between the condition check and wait() when the destructor fires. The test is expected to FAIL (deadlock timeout) with the current code, proving the bug exists. The fix will be in a separate PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 08b5ecc commit 9190b96

File tree

4 files changed

+94
-4
lines changed

4 files changed

+94
-4
lines changed

CMakeLists.txt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ FetchContent_Declare(AndroidExtensions
1414
GIT_TAG 2d5af72259cc73e5f249d3c99bee2010be9cb042
1515
EXCLUDE_FROM_ALL)
1616
FetchContent_Declare(arcana.cpp
17-
GIT_REPOSITORY https://github.com/microsoft/arcana.cpp.git
18-
GIT_TAG 7c6be8aaa29effbc8ee1a2217388fb6e399150d2
17+
GIT_REPOSITORY https://github.com/bghgary/arcana.cpp.git
18+
GIT_TAG 8328898f159c1778cb16fa41c7ad57994961e5e8
1919
EXCLUDE_FROM_ALL)
2020
FetchContent_Declare(asio
2121
GIT_REPOSITORY https://github.com/chriskohlhoff/asio.git
@@ -113,6 +113,10 @@ endif()
113113

114114
# --------------------------------------------------
115115

116+
if(JSRUNTIMEHOST_TESTS)
117+
add_compile_definitions(ARCANA_TESTING_HOOKS)
118+
endif()
119+
116120
FetchContent_MakeAvailable_With_Message(arcana.cpp)
117121
set_property(TARGET arcana PROPERTY FOLDER Dependencies)
118122

Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ get_filename_component(UNIT_TESTS_DIR "${CMAKE_CURRENT_LIST_DIR}/../../../../.."
99
get_filename_component(TESTS_DIR "${UNIT_TESTS_DIR}/.." ABSOLUTE)
1010
get_filename_component(REPO_ROOT_DIR "${TESTS_DIR}/.." ABSOLUTE)
1111

12+
set(JSRUNTIMEHOST_TESTS ON CACHE BOOL "" FORCE)
1213
add_subdirectory(${REPO_ROOT_DIR} "${CMAKE_CURRENT_BINARY_DIR}/JsRuntimeHost")
1314

1415
FetchContent_MakeAvailable_With_Message(googletest)

Tests/UnitTests/Shared/Shared.cpp

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,14 @@
1111
#include <Babylon/Polyfills/Blob.h>
1212
#include <Babylon/Polyfills/TextDecoder.h>
1313
#include <gtest/gtest.h>
14+
#ifdef ARCANA_TESTING_HOOKS
15+
#include <arcana/threading/blocking_concurrent_queue.h>
16+
#endif
17+
#include <atomic>
18+
#include <chrono>
1419
#include <future>
1520
#include <iostream>
21+
#include <thread>
1622

1723
namespace
1824
{
@@ -125,6 +131,83 @@ TEST(Console, Log)
125131
done.get_future().get();
126132
}
127133

134+
#ifdef ARCANA_TESTING_HOOKS
135+
TEST(AppRuntime, DestroyDoesNotDeadlock)
136+
{
137+
// Deterministic test for the race condition in the AppRuntime destructor.
138+
//
139+
// A global hook sleeps WHILE HOLDING the queue mutex, right before
140+
// condition_variable::wait(). We synchronize so the worker is definitely
141+
// in the hook before triggering destruction.
142+
//
143+
// Old (broken) code: cancel() + notify_all() fire without the mutex,
144+
// so the notification is lost while the worker sleeps → deadlock.
145+
// Fixed code: Append(cancel) calls push() which NEEDS the mutex,
146+
// so it blocks until the worker enters wait() → notification delivered.
147+
for (int i = 0; i < 3; i++)
148+
{
149+
// Shared state for hook synchronization
150+
std::atomic<bool> hookEnabled{false};
151+
std::atomic<bool> hookSignaled{false};
152+
std::promise<void> workerInHook;
153+
154+
// Set the callback. It checks hookEnabled so we control
155+
// when it actually sleeps.
156+
arcana::set_before_wait_callback([&]() {
157+
if (hookEnabled.load() && !hookSignaled.exchange(true))
158+
workerInHook.set_value();
159+
if (hookEnabled.load())
160+
std::this_thread::sleep_for(std::chrono::milliseconds(200));
161+
});
162+
163+
auto runtime = std::make_unique<Babylon::AppRuntime>();
164+
165+
// Dispatch work and wait for completion
166+
std::promise<void> ready;
167+
runtime->Dispatch([&ready](Napi::Env) {
168+
ready.set_value();
169+
});
170+
ready.get_future().wait();
171+
172+
// Enable the hook and dispatch a no-op to wake the worker,
173+
// ensuring it cycles through the hook on its way back to idle
174+
hookEnabled.store(true);
175+
runtime->Dispatch([](Napi::Env) {});
176+
177+
// Wait for the worker to be in the hook (holding mutex, sleeping)
178+
auto hookStatus = workerInHook.get_future().wait_for(std::chrono::seconds(5));
179+
ASSERT_NE(hookStatus, std::future_status::timeout)
180+
<< "Worker thread did not enter before-wait hook on iteration " << i;
181+
182+
// Destroy on a detachable thread so the test doesn't hang if the
183+
// destructor deadlocks (std::async's future destructor would block).
184+
auto runtimePtr = std::make_shared<std::unique_ptr<Babylon::AppRuntime>>(std::move(runtime));
185+
std::promise<void> destroyDone;
186+
auto destroyFuture = destroyDone.get_future();
187+
std::thread destroyThread([runtimePtr, &destroyDone]() {
188+
runtimePtr->reset();
189+
destroyDone.set_value();
190+
});
191+
192+
auto status = destroyFuture.wait_for(std::chrono::seconds(5));
193+
if (status == std::future_status::timeout)
194+
{
195+
destroyThread.detach();
196+
}
197+
else
198+
{
199+
destroyThread.join();
200+
}
201+
202+
// Reset hook
203+
arcana::set_before_wait_callback([]() {});
204+
205+
ASSERT_NE(status, std::future_status::timeout)
206+
<< "Deadlock detected on iteration " << i;
207+
}
208+
}
209+
#endif
210+
128211
int RunTests()
129212
{
130213
testing::InitGoogleTest();

Tests/UnitTests/Win32/App.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#include "../Shared/Shared.h"
22
#include <Windows.h>
33
#include "Babylon/DebugTrace.h"
4+
#include <gtest/gtest.h>
45

5-
int main()
6+
int main(int argc, char** argv)
67
{
78
SetConsoleOutputCP(CP_UTF8);
89

910
Babylon::DebugTrace::EnableDebugTrace(true);
1011
Babylon::DebugTrace::SetTraceOutput([](const char* trace) { OutputDebugStringA(trace); });
1112

12-
return RunTests();
13+
testing::InitGoogleTest(&argc, argv);
14+
return RUN_ALL_TESTS();
1315
}

0 commit comments

Comments
 (0)