diff --git a/Core/AppRuntime/Source/AppRuntime.cpp b/Core/AppRuntime/Source/AppRuntime.cpp index 78ebe2d9..77d9c4aa 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -34,6 +34,7 @@ namespace Babylon } std::optional m_env{}; + std::shared_ptr m_jsRuntimeState{}; std::optional> m_suspensionLock{}; arcana::cancellation_source m_cancelSource{}; arcana::manual_dispatcher<128> m_dispatcher{}; @@ -54,7 +55,7 @@ namespace Babylon m_impl->m_thread = std::thread{[this] { RunPlatformTier(); }}; Dispatch([this](Napi::Env env) { - JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); }); + m_impl->m_jsRuntimeState = JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); }).m_state; Internal::DelayedTaskScheduler::SetForJavaScript(env, GetDelayedTaskScheduler()); m_impl->m_delayedTaskSchedulerRegistered = true; }); @@ -94,6 +95,11 @@ namespace Babylon } Napi::HandleScope scope{env}; + + // Stop native completions before discarding work, while captures can still + // safely release environment-owned values. Do not rely on JS finalizer order. + JsRuntime::Close(m_impl->m_jsRuntimeState); + ShutdownEnvironment(env); if (m_impl->m_delayedTaskSchedulerRegistered) diff --git a/Core/JsRuntime/Include/Babylon/JsRuntime.h b/Core/JsRuntime/Include/Babylon/JsRuntime.h index 75a7f4d9..da11b8df 100644 --- a/Core/JsRuntime/Include/Babylon/JsRuntime.h +++ b/Core/JsRuntime/Include/Babylon/JsRuntime.h @@ -4,7 +4,7 @@ #include #include -#include +#include namespace Babylon { @@ -45,9 +45,15 @@ namespace Babylon JsRuntime& operator=(const JsRuntime&) = delete; private: + friend class AppRuntime; + friend class JsRuntimeScheduler; + JsRuntime(Napi::Env, DispatchFunctionT); + ~JsRuntime(); + + static void Dispatch(const std::shared_ptr&, std::function); + static void Close(const std::shared_ptr&); - DispatchFunctionT m_dispatchFunction{}; - std::mutex m_mutex{}; + std::shared_ptr m_state; }; } diff --git a/Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h b/Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h index b123611d..6f5cf6da 100644 --- a/Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h +++ b/Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h @@ -2,29 +2,45 @@ #include "JsRuntime.h" +#include +#include + namespace Babylon { /** * Scheduler that invokes continuations via JsRuntime::Dispatch. * Intended to be consumed by arcana.cpp tasks. + * Copies can outlive the runtime; dispatch after shutdown is discarded. */ class JsRuntimeScheduler { public: explicit JsRuntimeScheduler(JsRuntime& runtime) - : m_runtime{runtime} + : m_runtimeState{runtime.m_state} { } template void operator()(CallableT&& callable) const { - m_runtime.Dispatch([callable{std::forward(callable)}](Napi::Env) { - callable(); + JsRuntime::Dispatch(m_runtimeState, [callable{std::forward(callable)}](Napi::Env env) mutable { + // Preserve the original const, zero-argument invocation when available. + if constexpr (std::is_invocable_v) + { + std::as_const(callable)(); + } + else if constexpr (std::is_invocable_v) + { + callable(); + } + else + { + callable(env); + } }); } private: - JsRuntime& m_runtime; + std::shared_ptr m_runtimeState; }; } diff --git a/Core/JsRuntime/Source/JsRuntime.cpp b/Core/JsRuntime/Source/JsRuntime.cpp index 27db82b0..8c7155bc 100644 --- a/Core/JsRuntime/Source/JsRuntime.cpp +++ b/Core/JsRuntime/Source/JsRuntime.cpp @@ -1,8 +1,22 @@ #include "JsRuntime.h" #include "Babylon/DebugTrace.h" +#include +#include + namespace Babylon { + struct JsRuntime::InternalState + { + explicit InternalState(DispatchFunctionT dispatchFunction) + : DispatchFunction{std::move(dispatchFunction)} + { + } + + DispatchFunctionT DispatchFunction; + std::mutex Mutex; + }; + namespace { static constexpr auto JS_RUNTIME_NAME = "runtime"; @@ -10,7 +24,7 @@ namespace Babylon } JsRuntime::JsRuntime(Napi::Env env, DispatchFunctionT dispatchFunction) - : m_dispatchFunction{std::move(dispatchFunction)} + : m_state{std::make_shared(std::move(dispatchFunction))} { auto global = env.Global(); @@ -28,6 +42,22 @@ namespace Babylon DEBUG_TRACE("JsRuntime created"); } + JsRuntime::~JsRuntime() + { + Close(m_state); + } + + void JsRuntime::Close(const std::shared_ptr& state) + { + DispatchFunctionT dispatchFunction; + if (state) + { + std::scoped_lock lock{state->Mutex}; + dispatchFunction = std::exchange(state->DispatchFunction, {}); + } + // Captured objects may dispatch from their destructors. Release them unlocked. + } + JsRuntime& BABYLON_API JsRuntime::CreateForJavaScript(Napi::Env env, DispatchFunctionT dispatchFunction) { auto* runtime = new JsRuntime(env, std::move(dispatchFunction)); @@ -45,8 +75,20 @@ namespace Babylon void JsRuntime::Dispatch(std::function function) { - std::scoped_lock lock{m_mutex}; - m_dispatchFunction([function = std::move(function)](Napi::Env env) { + Dispatch(m_state, std::move(function)); + } + + void JsRuntime::Dispatch(const std::shared_ptr& state, std::function function) + { + // Keep the host enqueue and Close mutually exclusive: copying the dispatch + // function out of the lock would allow it to run after the host is gone. + std::scoped_lock lock{state->Mutex}; + if (!state->DispatchFunction) + { + return; + } + + state->DispatchFunction([function = std::move(function)](Napi::Env env) { function(env); // The environment will be in a pending exceptional state if diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt index f3676d7d..0133ed77 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -9,6 +9,7 @@ file(GLOB ASSETS "${CMAKE_CURRENT_SOURCE_DIR}/Assets/*") set(SOURCES "Shared/DelayedTaskScheduler.cpp" + "Shared/JsRuntimeScheduler.cpp" "Shared/StandardStreamLogger.cpp" "Shared/TimeoutDispatcher.cpp" "Shared/Shared.cpp" diff --git a/Tests/UnitTests/Shared/JsRuntimeScheduler.cpp b/Tests/UnitTests/Shared/JsRuntimeScheduler.cpp new file mode 100644 index 00000000..a17d39a2 --- /dev/null +++ b/Tests/UnitTests/Shared/JsRuntimeScheduler.cpp @@ -0,0 +1,305 @@ +#include +#include +#include "DelayedTaskScheduler.h" + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +using namespace std::chrono_literals; + +namespace +{ + template + T GetWithTimeout(std::future& future) + { + if (future.wait_for(5s) != std::future_status::ready) + { + throw std::runtime_error{"Runtime callback timed out"}; + } + return future.get(); + } + + // Include destruction in the deadline. A deadlocked worker may be detached, + // so every test and runtime callback owns its state, never a caller's stack. + template + void RunWithTimeout(CallbackT callback) + { + auto completed = std::make_shared>(); + auto future = completed->get_future(); + std::thread worker{[callback = std::move(callback), completed]() { + try + { + callback(); + completed->set_value(); + } + catch (...) + { + completed->set_exception(std::current_exception()); + } + }}; + + if (future.wait_for(15s) != std::future_status::ready) + { + worker.detach(); + FAIL() << "Runtime lifecycle did not finish within 15 seconds"; + } + worker.join(); + EXPECT_NO_THROW(future.get()); + } + + Babylon::JsRuntimeScheduler GetScheduler(Babylon::AppRuntime& runtime) + { + auto ready = std::make_shared>(); + auto future = ready->get_future(); + runtime.Dispatch([ready](Napi::Env env) { + ready->set_value(Babylon::JsRuntimeScheduler{Babylon::JsRuntime::GetFromJavaScript(env)}); + }); + return GetWithTimeout(future); + } + + struct OnDestruction + { + std::function Callback; + ~OnDestruction() + { + Callback(); + } + }; +} + +TEST(JsRuntimeScheduler, CopiesCanOutliveRuntime) +{ + RunWithTimeout([]() { + auto runtime = std::make_unique(); + const auto scheduler = GetScheduler(*runtime); + auto copy = scheduler; + auto moved = std::move(copy); + runtime.reset(); + + auto called = std::make_shared>(false); + scheduler([called]() { *called = true; }); + moved([called]() { *called = true; }); + if (called->load()) + { + throw std::runtime_error{"Callback ran after runtime destruction"}; + } + }); +} + +TEST(JsRuntimeScheduler, AcceptsMutableZeroArgumentLvalue) +{ + RunWithTimeout([]() { + Babylon::AppRuntime runtime; + const auto scheduler = GetScheduler(runtime); + auto completed = std::make_shared>(); + auto future = completed->get_future(); + auto callable = [completed, count = 0]() mutable { completed->set_value(++count); }; + scheduler(callable); + if (GetWithTimeout(future) != 1) + { + throw std::runtime_error{"Mutable callable was not invoked"}; + } + }); +} + +TEST(JsRuntimeScheduler, ForwardsEnvironmentAndAllowsNestedDispatch) +{ + RunWithTimeout([]() { + Babylon::AppRuntime runtime; + auto completed = std::make_shared>(); + auto future = completed->get_future(); + runtime.Dispatch([completed](Napi::Env env) { + const Babylon::JsRuntimeScheduler scheduler{Babylon::JsRuntime::GetFromJavaScript(env)}; + scheduler([completed, scheduler, expected = static_cast(env), count = 0](Napi::Env callbackEnv) mutable { + const bool correct = static_cast(callbackEnv) == expected && ++count == 1; + scheduler([completed, correct]() { completed->set_value(correct); }); + }); + }); + if (!GetWithTimeout(future)) + { + throw std::runtime_error{"Scheduler forwarded the wrong environment"}; + } + }); +} + +TEST(JsRuntimeScheduler, PrefersExistingZeroArgumentOverload) +{ + struct Callable + { + std::shared_ptr> Completed; + void operator()() const { Completed->set_value(true); } + void operator()(Napi::Env) const { Completed->set_value(false); } + }; + + RunWithTimeout([]() { + Babylon::AppRuntime runtime; + auto completed = std::make_shared>(); + auto future = completed->get_future(); + const Callable callable{completed}; + GetScheduler(runtime)(callable); + if (!GetWithTimeout(future)) + { + throw std::runtime_error{"Existing zero-argument overload was bypassed"}; + } + }); +} + +TEST(JsRuntimeScheduler, RunsArcanaContinuation) +{ + RunWithTimeout([]() { + Babylon::AppRuntime runtime; + auto completed = std::make_shared>(); + auto future = completed->get_future(); + auto scheduler = GetScheduler(runtime); + arcana::task_from_result().then( + scheduler, arcana::cancellation::none(), [completed]() { completed->set_value(); }); + GetWithTimeout(future); + }); +} + +TEST(JsRuntimeScheduler, PreservesConstZeroArgumentInvocation) +{ + struct Callable + { + std::shared_ptr> Completed; + void operator()() const { Completed->set_value(true); } + void operator()() { Completed->set_value(false); } + }; + + RunWithTimeout([]() { + Babylon::AppRuntime runtime; + auto completed = std::make_shared>(); + auto future = completed->get_future(); + GetScheduler(runtime)(Callable{completed}); + if (!GetWithTimeout(future)) + { + throw std::runtime_error{"Existing const invocation was bypassed"}; + } + }); +} + +TEST(JsRuntimeScheduler, DispatchCanRaceRuntimeTeardown) +{ + RunWithTimeout([]() { + auto runtime = std::make_unique(); + const auto scheduler = GetScheduler(*runtime); + auto started = std::make_shared>(); + auto future = started->get_future(); + auto called = std::make_shared>(0); + auto stop = std::make_shared>(false); + std::thread producer{[scheduler, started, called, stop]() { + const auto deadline = std::chrono::steady_clock::now() + 5s; + size_t attempts{}; + while (!stop->load() && std::chrono::steady_clock::now() < deadline) + { + scheduler([called]() { called->fetch_add(1); }); + if (++attempts == 100) + { + started->set_value(); + } + std::this_thread::yield(); + } + }}; + auto joinProducer = gsl::finally([&producer, stop]() { + *stop = true; + producer.join(); + }); + GetWithTimeout(future); + runtime.reset(); + + const auto countAfterTeardown = called->load(); + scheduler([called]() { called->fetch_add(1); }); + if (called->load() != countAfterTeardown) + { + throw std::runtime_error{"Callback ran after concurrent teardown"}; + } + }); +} + +TEST(JsRuntimeScheduler, ClosesBeforeDiscardingPendingWork) +{ + RunWithTimeout([]() { + auto runtime = std::make_unique(); + auto ready = std::make_shared>(); + auto future = ready->get_future(); + auto rejected = std::make_shared>(false); + runtime->Dispatch([ready, rejected](Napi::Env env) { + const Babylon::JsRuntimeScheduler scheduler{Babylon::JsRuntime::GetFromJavaScript(env)}; + auto probe = std::make_shared(); + probe->Callback = [scheduler, rejected]() { + auto token = std::make_shared(0); + std::weak_ptr weak = token; + scheduler([token = std::move(token)]() {}); + *rejected = weak.expired(); + }; + // Shutdown discards this capture before clearing the runtime queue. + Babylon::Internal::DelayedTaskScheduler::GetFromJavaScript(env)->Schedule(1h, [probe]() {}); + // Shutdown must not retrieve its state through a mutable JS property. + env.Global().Set("savedNative", env.Global().Get("_native")); + env.Global().Set("_native", env.Undefined()); + ready->set_value(); + }); + GetWithTimeout(future); + runtime.reset(); + if (!rejected->load()) + { + throw std::runtime_error{"Shutdown accepted work while discarding pending captures"}; + } + }); +} + +TEST(JsRuntimeScheduler, RejectedCaptureCanRedispatchFromDestructor) +{ + RunWithTimeout([]() { + auto runtime = std::make_unique(); + const auto scheduler = GetScheduler(*runtime); + runtime.reset(); + auto released = std::make_shared>(false); + auto probe = std::make_shared(); + probe->Callback = [scheduler, released]() { + scheduler([]() {}); + *released = true; + }; + scheduler([probe = std::move(probe)]() {}); + if (!released->load()) + { + throw std::runtime_error{"Rejected callback retained its captures"}; + } + }); +} + +TEST(JsRuntimeScheduler, FinalizerReleasesDispatchCapturesOutsideLock) +{ + RunWithTimeout([]() { + auto runtime = std::make_unique(); + auto ready = std::make_shared>(); + auto future = ready->get_future(); + auto released = std::make_shared>(false); + runtime->Dispatch([ready, released](Napi::Env env) { + auto probe = std::make_shared(); + // An independently created JsRuntime exercises finalizer-only closure, + // including breaking a dispatch capture that owns a scheduler copy. + auto& nativeRuntime = Babylon::JsRuntime::CreateForJavaScript(env, [probe](auto) {}); + probe->Callback = [scheduler = Babylon::JsRuntimeScheduler{nativeRuntime}, released]() { + scheduler([]() {}); + *released = true; + }; + ready->set_value(); + }); + GetWithTimeout(future); + runtime.reset(); + if (!released->load()) + { + throw std::runtime_error{"Finalizer retained the dispatch function"}; + } + }); +}