Skip to content

Commit 31248b6

Browse files
committed
Merge branch 'async-fixes' of https://github.com/bghgary/BabylonNative into async-fixes
2 parents 11ca510 + ad88d30 commit 31248b6

File tree

12 files changed

+137
-102
lines changed

12 files changed

+137
-102
lines changed

Core/AppRuntime/Source/AppRuntime.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ namespace Babylon
1212
: m_workQueue{std::make_unique<WorkQueue>([this] { RunPlatformTier(); })}
1313
, m_unhandledExceptionHandler{unhandledExceptionHandler}
1414
{
15-
Dispatch([this](Napi::Env env) {
15+
Dispatch([this](Napi::Env env)
16+
{
1617
JsRuntime::CreateForJavaScript(env, [this](auto func) { Dispatch(std::move(func)); });
1718
});
1819
}
@@ -45,8 +46,10 @@ namespace Babylon
4546

4647
void AppRuntime::Dispatch(Dispatchable<void(Napi::Env)> func)
4748
{
48-
m_workQueue->Append([this, func{std::move(func)}](Napi::Env env) mutable {
49-
Execute([this, env, func{std::move(func)}]() mutable {
49+
m_workQueue->Append([this, func{std::move(func)}](Napi::Env env) mutable
50+
{
51+
Execute([this, env, func{std::move(func)}]() mutable
52+
{
5053
try
5154
{
5255
func(env);

Core/AppRuntime/Source/WorkQueue.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@ namespace Babylon
4848
m_dispatcher.blocking_tick(m_cancellationSource);
4949
}
5050

51-
// There should not be any outstanding work during the shutdown sequence
52-
// which should be the only way exit the while loop above.
51+
// Drain the queue to complete work dispatched after cancellation.
52+
m_dispatcher.tick(arcana::cancellation::none());
53+
54+
// There should no longer be any outstanding work once the queue is drained.
5355
assert(m_dispatcher.empty());
5456
}
5557
}

Core/AppRuntime/Source/WorkQueue.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Babylon
2222
if (m_cancellationSource.cancelled())
2323
{
2424
// There is likely a coding error if this exception is thrown.
25-
throw std::runtime_error{"Cannot append to the work queue after it is canceled"};
25+
throw std::runtime_error{"Cannot append to the work queue while shutting down"};
2626
}
2727

2828
// Manual dispatcher queueing requires a copyable CallableT, we use a shared pointer trick to make a
@@ -55,6 +55,6 @@ namespace Babylon
5555
arcana::cancellation_source m_cancellationSource{};
5656
arcana::manual_dispatcher<128> m_dispatcher{};
5757

58-
std::thread m_thread;
58+
std::thread m_thread{};
5959
};
6060
}

Core/JsRuntime/Include/Babylon/JsRuntime.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,20 @@ namespace Babylon
4242
// Gets the JsRuntime from the given N-API environment.
4343
static JsRuntime& GetFromJavaScript(Napi::Env);
4444

45+
struct IDisposingCallback
46+
{
47+
virtual void Disposing() = 0;
48+
};
49+
4550
// Notifies the JsRuntime that the JavaScript environment will begin shutting down.
4651
// Calling this function will signal callbacks registered with RegisterDisposing.
4752
static void NotifyDisposing(JsRuntime&);
4853

4954
// Registers a callback for when the JavaScript environment will begin shutting down.
50-
static void RegisterDisposing(JsRuntime&, std::function<void()>);
55+
static void RegisterDisposing(JsRuntime&, IDisposingCallback*);
56+
57+
// Unregisters a callback for when the JavaScript environment will begin shutting down.
58+
static void UnregisterDisposing(JsRuntime&, IDisposingCallback*);
5159

5260
// Dispatches work onto the JavaScript thread and provides access to the N-API
5361
// environment.
@@ -58,6 +66,6 @@ namespace Babylon
5866

5967
std::mutex m_mutex{};
6068
DispatchFunctionT m_dispatchFunction{};
61-
std::vector<std::function<void()>> m_disposingCallbacks{};
69+
std::vector<IDisposingCallback*> m_disposingCallbacks{};
6270
};
6371
}

Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,26 @@ namespace Babylon
5353
// the continuation, must capture a persistent reference to the N-API object itself to prevent the GC from
5454
// collecting the N-API object during the asynchronous operation. Failing to do so will result in a hang
5555
// when `Rundown()` is called in the N-API class destructor.
56-
class JsRuntimeScheduler
56+
class JsRuntimeScheduler : public JsRuntime::IDisposingCallback
5757
{
5858
public:
5959
explicit JsRuntimeScheduler(JsRuntime& runtime)
6060
: m_runtime{&runtime}
6161
, m_scheduler{*this}
6262
{
63-
JsRuntime::RegisterDisposing(*m_runtime, [this]()
64-
{
65-
m_runtime = nullptr;
66-
});
63+
std::scoped_lock lock{m_mutex};
64+
JsRuntime::RegisterDisposing(*m_runtime, this);
6765
}
6866

6967
~JsRuntimeScheduler()
7068
{
7169
assert(m_count == 0 && "Schedulers for the JavaScript thread are not yet invoked");
70+
71+
std::scoped_lock lock{m_mutex};
72+
if (m_runtime != nullptr)
73+
{
74+
JsRuntime::UnregisterDisposing(*m_runtime, this);
75+
}
7276
}
7377

7478
// Wait until all of the schedulers are invoked.
@@ -88,6 +92,12 @@ namespace Babylon
8892
}
8993

9094
private:
95+
void Disposing() override
96+
{
97+
std::scoped_lock lock{m_mutex};
98+
m_runtime = nullptr;
99+
}
100+
91101
class SchedulerImpl
92102
{
93103
public:
@@ -108,6 +118,8 @@ namespace Babylon
108118
template<typename CallableT>
109119
void Dispatch(CallableT&& callable)
110120
{
121+
std::scoped_lock lock{m_mutex};
122+
111123
if (m_runtime != nullptr)
112124
{
113125
m_runtime->Dispatch([callable{std::forward<CallableT>(callable)}](Napi::Env)
@@ -123,7 +135,10 @@ namespace Babylon
123135
--m_count;
124136
}
125137

126-
JsRuntime* m_runtime;
138+
mutable std::mutex m_mutex{};
139+
JsRuntime* m_runtime{};
140+
std::function<void()> m_disposingCallback{};
141+
127142
SchedulerImpl m_scheduler;
128143
std::atomic<int> m_count{0};
129144
arcana::manual_dispatcher<128> m_disposingDispatcher{};

Core/JsRuntime/Source/JsRuntime.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "JsRuntime.h"
2+
#include <cassert>
23

34
namespace Babylon
45
{
@@ -43,15 +44,27 @@ namespace Babylon
4344
void JsRuntime::NotifyDisposing(JsRuntime& runtime)
4445
{
4546
auto callbacks = std::move(runtime.m_disposingCallbacks);
46-
for (const auto& callback : callbacks)
47+
for (auto* callback : callbacks)
4748
{
48-
callback();
49+
callback->Disposing();
4950
}
5051
}
5152

52-
void JsRuntime::RegisterDisposing(JsRuntime& runtime, std::function<void()> callback)
53+
void JsRuntime::RegisterDisposing(JsRuntime& runtime, IDisposingCallback* callback)
5354
{
54-
runtime.m_disposingCallbacks.push_back(std::move(callback));
55+
auto& callbacks = runtime.m_disposingCallbacks;
56+
assert(std::find(callbacks.begin(), callbacks.end(), callback) == callbacks.end());
57+
callbacks.push_back(callback);
58+
}
59+
60+
void JsRuntime::UnregisterDisposing(JsRuntime& runtime, IDisposingCallback* callback)
61+
{
62+
auto& callbacks = runtime.m_disposingCallbacks;
63+
auto it = std::find(callbacks.begin(), callbacks.end(), callback);
64+
if (it != callbacks.end())
65+
{
66+
callbacks.erase(it);
67+
}
5568
}
5669

5770
void JsRuntime::Dispatch(std::function<void(Napi::Env)> function)

Plugins/NativeEngine/Source/NativeEngine.cpp

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,26 +1127,25 @@ namespace Babylon
11271127
});
11281128
}
11291129

1130-
void NativeEngine::CopyTexture(const Napi::CallbackInfo& /*info*/)
1131-
{
1132-
//const auto textureDestination = info[0].As<Napi::Pointer<Graphics::Texture>>().Get();
1133-
//const auto textureSource = info[1].As<Napi::Pointer<Graphics::Texture>>().Get();
1134-
1135-
// TODO: this code makes no sense
1136-
//arcana::make_task(m_graphicsUpdate.Scheduler(), m_cancellationSource, [this, textureDestination, textureSource]()
1137-
//{
1138-
// return arcana::make_task(m_runtimeScheduler.Get(), m_cancellationSource, [this, textureDestination, textureSource, updateToken = m_graphicsUpdate.GetUpdateToken()]() mutable
1139-
// {
1140-
// bgfx::Encoder* encoder = updateToken.GetEncoder();
1141-
// GetBoundFrameBuffer(*encoder).Blit(*encoder, textureDestination->Handle(), 0, 0, textureSource->Handle());
1142-
// }).then(arcana::inline_scheduler, m_cancellationSource, [this, thisRef = Napi::Persistent(info.Env())](const arcana::expected<void, std::exception_ptr>& result)
1143-
// {
1144-
// if (result.has_error())
1145-
// {
1146-
// Napi::Error::New(Env(), result.error()).ThrowAsJavaScriptException();
1147-
// }
1148-
// });
1149-
//});
1130+
void NativeEngine::CopyTexture(const Napi::CallbackInfo& info)
1131+
{
1132+
const auto textureDestination = info[0].As<Napi::Pointer<Graphics::Texture>>().Get();
1133+
const auto textureSource = info[1].As<Napi::Pointer<Graphics::Texture>>().Get();
1134+
1135+
arcana::make_task(m_update.Scheduler(), m_cancellationSource, [this, textureDestination, textureSource]() mutable
1136+
{
1137+
return arcana::make_task(m_runtimeScheduler.Get(), m_cancellationSource, [this, textureDestination, textureSource, updateToken = m_update.GetUpdateToken()]() mutable
1138+
{
1139+
bgfx::Encoder* encoder = updateToken.GetEncoder();
1140+
GetBoundFrameBuffer(*encoder).Blit(*encoder, textureDestination->Handle(), 0, 0, textureSource->Handle());
1141+
}).then(arcana::inline_scheduler, m_cancellationSource, [this](const arcana::expected<void, std::exception_ptr>& result)
1142+
{
1143+
if (result.has_error())
1144+
{
1145+
Napi::Error::New(Env(), result.error()).ThrowAsJavaScriptException();
1146+
}
1147+
});
1148+
});
11501149
}
11511150

11521151
void NativeEngine::LoadRawTexture(const Napi::CallbackInfo& info)

Polyfills/Canvas/Source/Canvas.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,27 @@ namespace Babylon::Polyfills::Internal
5252

5353
Napi::Value NativeCanvas::LoadTTFAsync(const Napi::CallbackInfo& info)
5454
{
55-
const auto fontName = info[0].As<Napi::String>().Utf8Value();
55+
auto fontName = info[0].As<Napi::String>().Utf8Value();
5656
const auto buffer = info[1].As<Napi::ArrayBuffer>();
5757
std::vector<uint8_t> fontBuffer(buffer.ByteLength());
5858
memcpy(fontBuffer.data(), (uint8_t*)buffer.Data(), buffer.ByteLength());
5959

6060
auto& runtime = JsRuntime::GetFromJavaScript(info.Env());
6161
auto deferred = Napi::Promise::Deferred::New(info.Env());
62+
auto promise = deferred.Promise();
6263

6364
auto& deviceContext = Graphics::DeviceContext::GetFromJavaScript(info.Env());
6465
auto update = deviceContext.GetUpdate("update");
65-
arcana::make_task(update.Scheduler(), arcana::cancellation::none(), [fontName = std::move(fontName), fontData = std::move(fontBuffer), &runtime, deferred]()
66+
arcana::make_task(update.Scheduler(), arcana::cancellation::none(), [fontName = std::move(fontName), fontData = std::move(fontBuffer), &runtime, deferred = std::move(deferred)]() mutable
6667
{
6768
fontsInfos[fontName] = fontData;
68-
runtime.Dispatch([deferred](Napi::Env env)
69+
runtime.Dispatch([deferred = std::move(deferred)](Napi::Env env)
6970
{
7071
deferred.Resolve(env.Undefined());
7172
});
7273
});
7374

74-
return deferred.Promise();
75+
return promise;
7576
}
7677

7778
Napi::Value NativeCanvas::GetContext(const Napi::CallbackInfo& info)

Polyfills/Window/Source/TimeoutDispatcher.cpp

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,46 +14,29 @@ namespace Babylon::Polyfills::Internal
1414
}
1515
}
1616

17-
struct TimeoutDispatcher::Timeout
18-
{
19-
TimeoutId id;
20-
21-
// Make this non-shared when JsRuntime::Dispatch supports it.
22-
std::shared_ptr<Napi::FunctionReference> function;
23-
24-
TimePoint time;
25-
26-
Timeout(TimeoutId id, std::shared_ptr<Napi::FunctionReference> function, TimePoint time)
27-
: id{id}
28-
, function{std::move(function)}
29-
, time{time}
30-
{
31-
}
32-
33-
Timeout(const Timeout&) = delete;
34-
Timeout(Timeout&&) = delete;
35-
};
36-
3717
TimeoutDispatcher::TimeoutDispatcher(Babylon::JsRuntime& runtime)
38-
: m_runtime{runtime}
18+
: m_runtimeScheduler{runtime}
3919
, m_thread{std::thread{&TimeoutDispatcher::ThreadFunction, this}}
4020
{
4121
}
4222

4323
TimeoutDispatcher::~TimeoutDispatcher()
4424
{
4525
{
46-
std::unique_lock<std::mutex> lk{m_mutex};
26+
std::unique_lock<std::mutex> lock{m_mutex};
4727
m_idMap.clear();
4828
m_timeMap.clear();
4929
}
5030

51-
m_shutdown = true;
31+
m_cancellationSource.cancel();
5232
m_condVariable.notify_one();
5333
m_thread.join();
34+
35+
// Wait for async operations to complete.
36+
m_runtimeScheduler.Rundown();
5437
}
5538

56-
TimeoutDispatcher::TimeoutId TimeoutDispatcher::Dispatch(std::shared_ptr<Napi::FunctionReference> function, std::chrono::milliseconds delay)
39+
TimeoutId TimeoutDispatcher::Dispatch(std::shared_ptr<Napi::FunctionReference> function, std::chrono::milliseconds delay)
5740
{
5841
if (delay.count() < 0)
5942
{
@@ -70,7 +53,8 @@ namespace Babylon::Polyfills::Internal
7053

7154
if (time <= earliestTime)
7255
{
73-
m_runtime.Dispatch([this](Napi::Env) {
56+
m_runtimeScheduler.Get()([this]()
57+
{
7458
m_condVariable.notify_one();
7559
});
7660
}
@@ -102,7 +86,7 @@ namespace Babylon::Polyfills::Internal
10286
}
10387
}
10488

105-
TimeoutDispatcher::TimeoutId TimeoutDispatcher::NextTimeoutId()
89+
TimeoutId TimeoutDispatcher::NextTimeoutId()
10690
{
10791
while (true)
10892
{
@@ -122,11 +106,11 @@ namespace Babylon::Polyfills::Internal
122106

123107
void TimeoutDispatcher::ThreadFunction()
124108
{
125-
while (!m_shutdown)
109+
while (!m_cancellationSource.cancelled())
126110
{
127-
std::unique_lock<std::mutex> lk{m_mutex};
128-
TimePoint nextTimePoint{};
111+
std::unique_lock<std::mutex> lock{m_mutex};
129112

113+
TimePoint nextTimePoint{};
130114
while (!m_timeMap.empty())
131115
{
132116
nextTimePoint = m_timeMap.begin()->second->time;
@@ -135,7 +119,7 @@ namespace Babylon::Polyfills::Internal
135119
break;
136120
}
137121

138-
m_condVariable.wait_until(lk, nextTimePoint);
122+
m_condVariable.wait_until(lock, nextTimePoint);
139123
}
140124

141125
while (!m_timeMap.empty() && m_timeMap.begin()->second->time == nextTimePoint)
@@ -147,9 +131,9 @@ namespace Babylon::Polyfills::Internal
147131
CallFunction(std::move(function));
148132
}
149133

150-
while (!m_shutdown && m_timeMap.empty())
134+
while (!m_cancellationSource.cancelled() && m_timeMap.empty())
151135
{
152-
m_condVariable.wait(lk);
136+
m_condVariable.wait(lock);
153137
}
154138
}
155139
}
@@ -158,7 +142,10 @@ namespace Babylon::Polyfills::Internal
158142
{
159143
if (function)
160144
{
161-
m_runtime.Dispatch([function = std::move(function)](Napi::Env) { function->Call({}); });
145+
m_runtimeScheduler.Get()([function = std::move(function)]()
146+
{
147+
function->Call({});
148+
});
162149
}
163150
}
164151
}

0 commit comments

Comments
 (0)