Fix WorkQueue destructor deadlock

.gitignore

@@ -1 +1,2 @@
 /Build
+Tests/UnitTests/dist/

CMakeLists.txt

@@ -15,7 +15,7 @@ FetchContent_Declare(AndroidExtensions
     EXCLUDE_FROM_ALL)
 FetchContent_Declare(arcana.cpp
     GIT_REPOSITORY https://github.com/microsoft/arcana.cpp.git
-    GIT_TAG 7c6be8aaa29effbc8ee1a2217388fb6e399150d2
+    GIT_TAG d5dd03cc6dd138fc17c277f61abbe2bc388444af
     EXCLUDE_FROM_ALL)
 FetchContent_Declare(asio
     GIT_REPOSITORY https://github.com/chriskohlhoff/asio.git
@@ -113,6 +113,10 @@ endif()
 
 # --------------------------------------------------
 
+if(JSRUNTIMEHOST_TESTS)
+    add_compile_definitions(ARCANA_TESTING_HOOKS)
+endif()
+
 FetchContent_MakeAvailable_With_Message(arcana.cpp)
 set_property(TARGET arcana PROPERTY FOLDER Dependencies)
 

Core/AppRuntime/Include/Babylon/AppRuntime.h

@@ -61,7 +61,7 @@ namespace Babylon
         // extra logic around the invocation of a dispatched callback.
         void Execute(Dispatchable<void()> callback);
 
-        std::unique_ptr<WorkQueue> m_workQueue;
         Options m_options;
+        std::unique_ptr<WorkQueue> m_workQueue;
     };
 }

Core/AppRuntime/Source/AppRuntime.cpp

@@ -10,8 +10,8 @@ namespace Babylon
     }
 
     AppRuntime::AppRuntime(Options options)
-        : m_workQueue{std::make_unique<WorkQueue>([this] { RunPlatformTier(); })}
-        , m_options{std::move(options)}
+        : m_options{std::move(options)}
+        , m_workQueue{std::make_unique<WorkQueue>([this] { RunPlatformTier(); })}
     {
         Dispatch([this](Napi::Env env) {
             JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); });

Core/AppRuntime/Source/WorkQueue.cpp

@@ -14,8 +14,17 @@ namespace Babylon
             Resume();
         }
 
+        // Cancel immediately so pending work is dropped promptly, then append
+        // a no-op work item to wake the worker thread from blocking_tick. The
+        // no-op goes through push() which acquires the queue mutex, avoiding
+        // the race where a bare notify_all() can be missed by wait().
+        //
+        // NOTE: This preserves the existing shutdown behavior where pending
+        // callbacks are dropped on cancellation. A more complete solution
+        // would add cooperative shutdown (e.g. NotifyDisposing/Rundown) so
+        // consumers can finish cleanup work before the runtime is destroyed.
         m_cancelSource.cancel();
-        m_dispatcher.cancelled();
+        Append([](Napi::Env) {});
 
         m_thread.join();
     }

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

@@ -9,6 +9,7 @@ get_filename_component(UNIT_TESTS_DIR "${CMAKE_CURRENT_LIST_DIR}/../../../../.."
 get_filename_component(TESTS_DIR "${UNIT_TESTS_DIR}/.." ABSOLUTE)
 get_filename_component(REPO_ROOT_DIR "${TESTS_DIR}/.." ABSOLUTE)
 
+set(JSRUNTIMEHOST_TESTS ON CACHE BOOL "" FORCE)
 add_subdirectory(${REPO_ROOT_DIR} "${CMAKE_CURRENT_BINARY_DIR}/JsRuntimeHost")
 
 FetchContent_MakeAvailable_With_Message(googletest)
@@ -21,6 +22,7 @@ add_library(UnitTestsJNI SHARED
     ${UNIT_TESTS_DIR}/Shared/Shared.cpp)
 
 target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}")
+target_compile_definitions(UnitTestsJNI PRIVATE ARCANA_TESTING_HOOKS)
 
 target_include_directories(UnitTestsJNI
     PRIVATE ${UNIT_TESTS_DIR})

Tests/UnitTests/Shared/Shared.cpp

@@ -11,8 +11,12 @@
 #include <Babylon/Polyfills/Blob.h>
 #include <Babylon/Polyfills/TextDecoder.h>
 #include <gtest/gtest.h>
+#include <arcana/threading/blocking_concurrent_queue.h>
+#include <atomic>
+#include <chrono>
 #include <future>
 #include <iostream>
+#include <thread>
 
 namespace
 {
@@ -125,6 +129,86 @@ TEST(Console, Log)
     done.get_future().get();
 }
 
+TEST(AppRuntime, DestroyDoesNotDeadlock)
+{
+    // Deterministic test for the race condition in the AppRuntime destructor.
+    //
+    // A global hook sleeps WHILE HOLDING the queue mutex, right before
+    // condition_variable::wait(). We synchronize so the worker is definitely
+    // in the hook before triggering destruction.
+    //
+    // Old (broken) code: cancel() + notify_all() fire without the mutex,
+    //   so the notification is lost while the worker sleeps → deadlock.
+    // Fixed code: cancel() + Append(no-op), where push() NEEDS the mutex,
+    //   so it blocks until the worker enters wait() → notification delivered.
+
+    // Shared state for hook synchronization
+    std::atomic<bool> hookEnabled{false};
+    std::atomic<bool> hookSignaled{false};
+    std::promise<void> workerInHook;
+
+    // Set the callback. It checks hookEnabled so we control
+    // when it actually sleeps.
+    arcana::set_before_wait_callback([&]() {
+        if (hookEnabled.load() && !hookSignaled.exchange(true))
+        {
+            workerInHook.set_value();
+        }
+        if (hookEnabled.load())
+        {
+            std::this_thread::sleep_for(std::chrono::milliseconds(200));
+        }
+    });
+
+    auto runtime = std::make_unique<Babylon::AppRuntime>();
+
+    // Dispatch work and wait for completion
+    std::promise<void> ready;
+    runtime->Dispatch([&ready](Napi::Env) {
+        ready.set_value();
+    });
+    ready.get_future().wait();
+
+    // Enable the hook and dispatch a no-op to wake the worker,
+    // ensuring it cycles through the hook on its way back to idle
+    hookEnabled.store(true);
+    runtime->Dispatch([](Napi::Env) {});
+
+    // Wait for the worker to be in the hook (holding mutex, sleeping)
+    auto hookStatus = workerInHook.get_future().wait_for(std::chrono::seconds(5));
+    if (hookStatus == std::future_status::timeout)
+    {
+        // Hook didn't fire — no deadlock risk, clean up normally
+        arcana::set_before_wait_callback([]() {});
+        FAIL() << "Worker thread did not enter before-wait hook";
+    }
+
+    // Worker is in the hook (holding mutex, sleeping). Destroy on a
+    // detachable thread so the test doesn't hang if the destructor deadlocks.
+    auto runtimePtr = std::make_shared<std::unique_ptr<Babylon::AppRuntime>>(std::move(runtime));
+    std::promise<void> destroyDone;
+    auto destroyFuture = destroyDone.get_future();
+    std::thread destroyThread([runtimePtr, &destroyDone]() {
+        runtimePtr->reset();
+        destroyDone.set_value();
+    });
+
+    auto status = destroyFuture.wait_for(std::chrono::seconds(5));
+    if (status == std::future_status::timeout)
+    {
+        destroyThread.detach();
+    }
+    else
+    {
+        destroyThread.join();
+    }
+
+    arcana::set_before_wait_callback([]() {});
+
+    ASSERT_NE(status, std::future_status::timeout)
+        << "Deadlock detected: AppRuntime destructor did not complete within 5 seconds";
+}
+
 int RunTests()
 {
     testing::InitGoogleTest();

Tests/UnitTests/Win32/App.cpp

@@ -1,13 +1,15 @@
 #include "../Shared/Shared.h"
 #include <Windows.h>
 #include "Babylon/DebugTrace.h"
+#include <gtest/gtest.h>
 
-int main()
+int main(int argc, char** argv)
 {
     SetConsoleOutputCP(CP_UTF8);
 
     Babylon::DebugTrace::EnableDebugTrace(true);
     Babylon::DebugTrace::SetTraceOutput([](const char* trace) { OutputDebugStringA(trace); });
 
-    return RunTests();
+    testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
 }