Skip to content

Commit 2cea4fa

Browse files
bghgaryCopilot
andcommitted
Add deterministic test for AppRuntime destructor deadlock race
Add a test that deterministically triggers the lost-wakeup race condition in WorkQueue::~WorkQueue(). Uses arcana testing hooks to sleep while holding the queue mutex before wait(), guaranteeing the worker is between the condition check and wait() when the destructor fires cancel+notify. 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 2cea4fa

File tree

6 files changed

+89
-4
lines changed

6 files changed

+89
-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 d5804f8a51de0e37d78bdea469c21823eb7d27ce
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

Core/AppRuntime/Include/Babylon/AppRuntime.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,10 @@ namespace Babylon
6363

6464
std::unique_ptr<WorkQueue> m_workQueue;
6565
Options m_options;
66+
67+
#ifdef ARCANA_TESTING_HOOKS
68+
public:
69+
void SetBeforeWaitCallback(std::function<void()> callback);
70+
#endif
6671
};
6772
}

Core/AppRuntime/Source/AppRuntime.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,11 @@ namespace Babylon
5757
});
5858
});
5959
}
60+
61+
#ifdef ARCANA_TESTING_HOOKS
62+
void AppRuntime::SetBeforeWaitCallback(std::function<void()> callback)
63+
{
64+
m_workQueue->SetBeforeWaitCallback(std::move(callback));
65+
}
66+
#endif
6067
}

Core/AppRuntime/Source/WorkQueue.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,13 @@ namespace Babylon
5454
arcana::manual_dispatcher<128> m_dispatcher{};
5555

5656
std::thread m_thread;
57+
58+
#ifdef ARCANA_TESTING_HOOKS
59+
public:
60+
void SetBeforeWaitCallback(std::function<void()> callback)
61+
{
62+
m_dispatcher.set_before_wait_callback(std::move(callback));
63+
}
64+
#endif
5765
};
5866
}

Tests/UnitTests/Shared/Shared.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@
1111
#include <Babylon/Polyfills/Blob.h>
1212
#include <Babylon/Polyfills/TextDecoder.h>
1313
#include <gtest/gtest.h>
14+
#include <atomic>
15+
#include <chrono>
1416
#include <future>
1517
#include <iostream>
18+
#include <thread>
1619

1720
namespace
1821
{
@@ -125,6 +128,62 @@ TEST(Console, Log)
125128
done.get_future().get();
126129
}
127130

131+
TEST(AppRuntime, DestroyDoesNotDeadlock)
132+
{
133+
// Deterministic test for the race condition in the AppRuntime destructor.
134+
//
135+
// A per-instance hook sleeps WHILE HOLDING the queue mutex, right before
136+
// condition_variable::wait(). We synchronize so the worker is definitely
137+
// in the hook before triggering destruction.
138+
//
139+
// Old (broken) code: cancel() + notify_all() fire without the mutex,
140+
// so the notification is lost while the worker sleeps → deadlock.
141+
// Fixed code: Append(cancel) calls push() which NEEDS the mutex,
142+
// so it blocks until the worker enters wait() → notification delivered.
143+
for (int i = 0; i < 3; i++)
144+
{
145+
// Shared state for hook synchronization
146+
std::atomic<bool> hookEnabled{false};
147+
std::atomic<bool> hookSignaled{false};
148+
std::promise<void> workerInHook;
149+
150+
auto runtime = std::make_unique<Babylon::AppRuntime>();
151+
152+
// Set the callback once, before any dispatches. The callback checks
153+
// hookEnabled so we control when it actually sleeps.
154+
runtime->SetBeforeWaitCallback([&]() {
155+
if (hookEnabled.load() && !hookSignaled.exchange(true))
156+
workerInHook.set_value();
157+
if (hookEnabled.load())
158+
std::this_thread::sleep_for(std::chrono::milliseconds(200));
159+
});
160+
161+
// Dispatch work and wait for completion
162+
std::promise<void> ready;
163+
runtime->Dispatch([&ready](Napi::Env) {
164+
ready.set_value();
165+
});
166+
ready.get_future().wait();
167+
168+
// Enable the hook and dispatch a no-op to wake the worker,
169+
// ensuring it cycles through the hook on its way back to idle
170+
hookEnabled.store(true);
171+
runtime->Dispatch([](Napi::Env) {});
172+
173+
// Wait for the worker to be in the hook (holding mutex, sleeping)
174+
workerInHook.get_future().wait();
175+
176+
// NOW destroy
177+
auto destroyFuture = std::async(std::launch::async, [&runtime]() {
178+
runtime.reset();
179+
});
180+
181+
auto status = destroyFuture.wait_for(std::chrono::seconds(5));
182+
ASSERT_NE(status, std::future_status::timeout)
183+
<< "Deadlock detected on iteration " << i;
184+
}
185+
}
186+
128187
int RunTests()
129188
{
130189
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)