Skip to content

Commit 36a4661

Browse files
bghgarybghgary
committed
Merge AppRuntime and WorkQueue to fix race condition
1 parent 423ed5e commit 36a4661

18 files changed

+243
-244
lines changed

Diff for: ‎Core/AppRuntime/CMakeLists.txt

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ set(SOURCES
22
"Include/Babylon/Dispatchable.h"
33
"Include/Babylon/AppRuntime.h"
44
"Source/AppRuntime.cpp"
5-
"Source/AppRuntime_${NAPI_JAVASCRIPT_ENGINE}.cpp"
6-
"Source/AppRuntime_${BABYLON_NATIVE_PLATFORM}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}"
7-
"Source/WorkQueue.cpp"
8-
"Source/WorkQueue.h")
5+
"Source/AppRuntimeImpl.h"
6+
"Source/AppRuntimeImpl.cpp"
7+
"Source/AppRuntimeImpl_${NAPI_JAVASCRIPT_ENGINE}.cpp"
8+
"Source/AppRuntimeImpl_${BABYLON_NATIVE_PLATFORM}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}")
99

1010
add_library(AppRuntime ${SOURCES})
1111
warnings_as_errors(AppRuntime)

Diff for: ‎Core/AppRuntime/Include/Babylon/AppRuntime.h

+8-24
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
#pragma once
22

3+
#include <napi/env.h>
4+
35
#include "Dispatchable.h"
4-
#include <napi/napi.h>
56

67
#include <memory>
78
#include <functional>
89
#include <exception>
910

1011
namespace Babylon
1112
{
12-
class WorkQueue;
13+
class AppRuntimeImpl;
1314

1415
class AppRuntime final
1516
{
@@ -18,33 +19,16 @@ namespace Babylon
1819
AppRuntime(std::function<void(const std::exception&)> unhandledExceptionHandler);
1920
~AppRuntime();
2021

22+
// Move semantics
23+
AppRuntime(AppRuntime&&) noexcept;
24+
AppRuntime& operator=(AppRuntime&&) noexcept;
25+
2126
void Suspend();
2227
void Resume();
2328

2429
void Dispatch(Dispatchable<void(Napi::Env)> callback);
2530

2631
private:
27-
// These three methods are the mechanism by which platform- and JavaScript-specific
28-
// code can be "injected" into the execution of the JavaScript thread. These three
29-
// functions are implemented in separate files, thus allowing implementations to be
30-
// mixed and matched by the build system based on the platform and JavaScript engine
31-
// being targeted, without resorting to virtuality. An important nuance of these
32-
// functions is that they are all intended to call each other: RunPlatformTier MUST
33-
// call RunEnvironmentTier, which MUST create the initial Napi::Env and pass it to
34-
// Run. This arrangement allows not only for an arbitrary assemblage of platforms,
35-
// but it also allows us to respect the requirement by certain platforms (notably V8)
36-
// that certain program state be allocated and stored only on the stack.
37-
void RunPlatformTier();
38-
void RunEnvironmentTier(const char* executablePath = ".");
39-
void Run(Napi::Env);
40-
41-
// This method is called from Dispatch to allow platform-specific code to add
42-
// extra logic around the invocation of a dispatched callback.
43-
void Execute(Dispatchable<void()> callback);
44-
45-
static void DefaultUnhandledExceptionHandler(const std::exception& error);
46-
47-
std::unique_ptr<WorkQueue> m_workQueue{};
48-
std::function<void(const std::exception&)> m_unhandledExceptionHandler{};
32+
std::unique_ptr<AppRuntimeImpl> m_impl;
4933
};
5034
}

Diff for: ‎Core/AppRuntime/Source/AppRuntime.cpp

+11-40
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,36 @@
11
#include "AppRuntime.h"
2-
#include "WorkQueue.h"
3-
#include <Babylon/JsRuntime.h>
2+
#include "AppRuntimeImpl.h"
43

54
namespace Babylon
65
{
76
AppRuntime::AppRuntime()
8-
: AppRuntime{DefaultUnhandledExceptionHandler}
7+
: m_impl{std::make_unique<AppRuntimeImpl>()}
98
{
109
}
1110

1211
AppRuntime::AppRuntime(std::function<void(const std::exception&)> unhandledExceptionHandler)
13-
: m_workQueue{std::make_unique<WorkQueue>([this] { RunPlatformTier(); })}
14-
, m_unhandledExceptionHandler{unhandledExceptionHandler}
12+
: m_impl{std::make_unique<AppRuntimeImpl>(unhandledExceptionHandler)}
1513
{
16-
Dispatch([this](Napi::Env env) {
17-
JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); });
18-
});
1914
}
2015

21-
AppRuntime::~AppRuntime()
22-
{
23-
// Notify the JsRuntime on the JavaScript thread that the JavaScript
24-
// runtime shutdown sequence has begun. The JsRuntimeScheduler will
25-
// use this signal to gracefully cancel asynchronous operations.
26-
Dispatch([](Napi::Env env) {
27-
JsRuntime::NotifyDisposing(JsRuntime::GetFromJavaScript(env));
28-
});
29-
}
16+
AppRuntime::~AppRuntime() = default;
3017

31-
void AppRuntime::Run(Napi::Env env)
32-
{
33-
m_workQueue->Run(env);
34-
}
18+
// Move semantics
19+
AppRuntime::AppRuntime(AppRuntime&&) noexcept = default;
20+
AppRuntime& AppRuntime::operator=(AppRuntime&&) noexcept = default;
3521

3622
void AppRuntime::Suspend()
3723
{
38-
m_workQueue->Suspend();
24+
m_impl->Suspend();
3925
}
4026

4127
void AppRuntime::Resume()
4228
{
43-
m_workQueue->Resume();
29+
m_impl->Resume();
4430
}
4531

46-
void AppRuntime::Dispatch(Dispatchable<void(Napi::Env)> func)
32+
void AppRuntime::Dispatch(Dispatchable<void(Napi::Env)> callback)
4733
{
48-
m_workQueue->Append([this, func{std::move(func)}](Napi::Env env) mutable {
49-
Execute([this, env, func{std::move(func)}]() mutable {
50-
try
51-
{
52-
func(env);
53-
}
54-
catch (const std::exception& error)
55-
{
56-
m_unhandledExceptionHandler(error);
57-
}
58-
catch (...)
59-
{
60-
std::abort();
61-
}
62-
});
63-
});
34+
m_impl->Dispatch(std::move(callback));
6435
}
6536
}

Diff for: ‎Core/AppRuntime/Source/AppRuntimeImpl.cpp

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
#include "AppRuntimeImpl.h"
2+
#include <Babylon/JsRuntime.h>
3+
4+
namespace Babylon
5+
{
6+
AppRuntimeImpl::AppRuntimeImpl(std::function<void(const std::exception&)> unhandledExceptionHandler)
7+
: m_unhandledExceptionHandler{std::move(unhandledExceptionHandler)}
8+
, m_thread{[this] { RunPlatformTier(); }}
9+
{
10+
Dispatch([this](Napi::Env env) {
11+
JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); });
12+
});
13+
}
14+
15+
AppRuntimeImpl::~AppRuntimeImpl()
16+
{
17+
if (m_suspensionLock.has_value())
18+
{
19+
m_suspensionLock.reset();
20+
}
21+
22+
Dispatch([this](Napi::Env env) {
23+
// Notify the JsRuntime on the JavaScript thread that the JavaScript runtime shutdown sequence has
24+
// begun. The JsRuntimeScheduler will use this signal to gracefully cancel asynchronous operations.
25+
JsRuntime::NotifyDisposing(JsRuntime::GetFromJavaScript(env));
26+
27+
// Cancel on the JavaScript thread to signal the Run function to gracefully end. It must be
28+
// dispatched and not canceled directly to ensure that existing work is executed and executed in
29+
// the correct order.
30+
m_cancellationSource.cancel();
31+
});
32+
33+
m_thread.join();
34+
}
35+
36+
void AppRuntimeImpl::Suspend()
37+
{
38+
auto suspensionMutex = std::make_shared<std::mutex>();
39+
m_suspensionLock.emplace(*suspensionMutex);
40+
Append([suspensionMutex = std::move(suspensionMutex)](Napi::Env) {
41+
std::scoped_lock lock{*suspensionMutex};
42+
});
43+
}
44+
45+
void AppRuntimeImpl::Resume()
46+
{
47+
m_suspensionLock.reset();
48+
}
49+
50+
void AppRuntimeImpl::Dispatch(Dispatchable<void(Napi::Env)> func)
51+
{
52+
Append([this, func{std::move(func)}](Napi::Env env) mutable {
53+
Execute([this, env, func{std::move(func)}]() mutable {
54+
try
55+
{
56+
func(env);
57+
}
58+
catch (const std::exception& error)
59+
{
60+
m_unhandledExceptionHandler(error);
61+
}
62+
catch (...)
63+
{
64+
std::abort();
65+
}
66+
});
67+
});
68+
}
69+
70+
void AppRuntimeImpl::Run(Napi::Env env)
71+
{
72+
m_env = std::make_optional(env);
73+
74+
m_dispatcher.set_affinity(std::this_thread::get_id());
75+
76+
while (!m_cancellationSource.cancelled())
77+
{
78+
m_dispatcher.blocking_tick(m_cancellationSource);
79+
}
80+
81+
// The dispatcher can be non-empty if something is dispatched after cancellation.
82+
// For example, Chakra's JsSetPromiseContinuationCallback may potentially dispatch
83+
// a continuation after cancellation.
84+
m_dispatcher.clear();
85+
86+
m_env.reset();
87+
}
88+
}

Diff for: ‎Core/AppRuntime/Source/AppRuntimeImpl.h

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
#pragma once
2+
3+
#include "AppRuntime.h"
4+
#include <optional>
5+
#include <mutex>
6+
#include <arcana/threading/dispatcher.h>
7+
8+
namespace Babylon
9+
{
10+
class AppRuntimeImpl
11+
{
12+
public:
13+
AppRuntimeImpl(std::function<void(const std::exception&)> unhandledExceptionHandler = DefaultUnhandledExceptionHandler);
14+
~AppRuntimeImpl();
15+
16+
void Suspend();
17+
void Resume();
18+
19+
void Dispatch(Dispatchable<void(Napi::Env)> func);
20+
21+
private:
22+
static void DefaultUnhandledExceptionHandler(const std::exception& error);
23+
24+
template<typename CallableT>
25+
void Append(CallableT callable)
26+
{
27+
// Manual dispatcher queueing requires a copyable CallableT, we use a shared pointer trick to make a
28+
// copyable callable if necessary.
29+
if constexpr (std::is_copy_constructible<CallableT>::value)
30+
{
31+
m_dispatcher.queue([this, callable = std::move(callable)]() {
32+
callable(m_env.value());
33+
});
34+
}
35+
else
36+
{
37+
m_dispatcher.queue([this, callablePtr = std::make_shared<CallableT>(std::move(callable))]() {
38+
(*callablePtr)(m_env.value());
39+
});
40+
}
41+
}
42+
43+
// These three methods are the mechanism by which platform- and JavaScript-specific
44+
// code can be "injected" into the execution of the JavaScript thread. These three
45+
// functions are implemented in separate files, thus allowing implementations to be
46+
// mixed and matched by the build system based on the platform and JavaScript engine
47+
// being targeted, without resorting to virtuality. An important nuance of these
48+
// functions is that they are all intended to call each other: RunPlatformTier MUST
49+
// call RunEnvironmentTier, which MUST create the initial Napi::Env and pass it to
50+
// Run. This arrangement allows not only for an arbitrary assemblage of platforms,
51+
// but it also allows us to respect the requirement by certain platforms (notably V8)
52+
// that certain program state be allocated and stored only on the stack.
53+
void RunPlatformTier();
54+
void RunEnvironmentTier(const char* executablePath = ".");
55+
void Run(Napi::Env);
56+
57+
// This method is called from Dispatch to allow platform-specific code to add
58+
// extra logic around the invocation of a dispatched callback.
59+
void Execute(Dispatchable<void()> callback);
60+
61+
std::function<void(const std::exception&)> m_unhandledExceptionHandler{};
62+
std::optional<Napi::Env> m_env{};
63+
std::optional<std::scoped_lock<std::mutex>> m_suspensionLock{};
64+
arcana::cancellation_source m_cancellationSource{};
65+
arcana::manual_dispatcher<128> m_dispatcher{};
66+
std::thread m_thread{};
67+
};
68+
}

Diff for: ‎Core/AppRuntime/Source/AppRuntime_Android.cpp renamed to ‎Core/AppRuntime/Source/AppRuntimeImpl_Android.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
1-
#include "AppRuntime.h"
1+
#include "AppRuntimeImpl.h"
22
#include <exception>
33
#include <sstream>
44
#include <android/log.h>
55

66
namespace Babylon
77
{
8-
void AppRuntime::RunPlatformTier()
8+
void AppRuntimeImpl::RunPlatformTier()
99
{
1010
RunEnvironmentTier();
1111
}
1212

13-
void AppRuntime::DefaultUnhandledExceptionHandler(const std::exception& error)
13+
void AppRuntimeImpl::DefaultUnhandledExceptionHandler(const std::exception& error)
1414
{
1515
std::stringstream ss{};
1616
ss << "Uncaught Error: " << error.what() << std::endl;
1717
__android_log_write(ANDROID_LOG_ERROR, "BabylonNative", ss.str().data());
1818
}
1919

20-
void AppRuntime::Execute(Dispatchable<void()> callback)
20+
void AppRuntimeImpl::Execute(Dispatchable<void()> callback)
2121
{
2222
callback();
2323
}

Diff for: ‎Core/AppRuntime/Source/AppRuntime_Chakra.cpp renamed to ‎Core/AppRuntime/Source/AppRuntimeImpl_Chakra.cpp

+12-14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include "AppRuntime.h"
1+
#include "AppRuntimeImpl.h"
22

33
#include <napi/env.h>
44

@@ -20,25 +20,23 @@ namespace Babylon
2020
}
2121
}
2222

23-
void AppRuntime::RunEnvironmentTier(const char*)
23+
void AppRuntimeImpl::RunEnvironmentTier(const char*)
2424
{
2525
JsRuntimeHandle jsRuntime;
2626
ThrowIfFailed(JsCreateRuntime(JsRuntimeAttributeNone, nullptr, &jsRuntime));
2727
JsContextRef context;
2828
ThrowIfFailed(JsCreateContext(jsRuntime, &context));
2929
ThrowIfFailed(JsSetCurrentContext(context));
30-
ThrowIfFailed(JsSetPromiseContinuationCallback(
31-
[](JsValueRef task, void* callbackState) {
32-
auto* pThis = reinterpret_cast<AppRuntime*>(callbackState);
33-
ThrowIfFailed(JsAddRef(task, nullptr));
34-
pThis->Dispatch([task](auto) {
35-
JsValueRef global;
36-
ThrowIfFailed(JsGetGlobalObject(&global));
37-
ThrowIfFailed(JsCallFunction(task, &global, 1, nullptr));
38-
ThrowIfFailed(JsRelease(task, nullptr));
39-
});
40-
},
41-
this));
30+
ThrowIfFailed(JsSetPromiseContinuationCallback([](JsValueRef task, void* callbackState) {
31+
auto* pThis = reinterpret_cast<AppRuntimeImpl*>(callbackState);
32+
ThrowIfFailed(JsAddRef(task, nullptr));
33+
pThis->Dispatch([task](auto) {
34+
JsValueRef global;
35+
ThrowIfFailed(JsGetGlobalObject(&global));
36+
ThrowIfFailed(JsCallFunction(task, &global, 1, nullptr));
37+
ThrowIfFailed(JsRelease(task, nullptr));
38+
});
39+
}, this));
4240
ThrowIfFailed(JsProjectWinRTNamespace(L"Windows"));
4341

4442
#if defined(_DEBUG)

0 commit comments

Comments
 (0)