Skip to content

Commit 925d243

Browse files
bghgarybghgary
committed
Change to explicitly call Rundown
1 parent 43dce92 commit 925d243

File tree

15 files changed

+197
-157
lines changed

15 files changed

+197
-157
lines changed

‎Core/AppRuntime/Source/WorkQueue.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace Babylon
1818
// It must be dispatched and not canceled directly to ensure that
1919
// existing work is executed and executed in the correct order.
2020
m_dispatcher([this]() {
21-
m_cancelSource.cancel();
21+
m_cancellationSource.cancel();
2222
});
2323

2424
m_thread.join();
@@ -43,9 +43,9 @@ namespace Babylon
4343
m_env = std::make_optional(env);
4444
m_dispatcher.set_affinity(std::this_thread::get_id());
4545

46-
while (!m_cancelSource.cancelled())
46+
while (!m_cancellationSource.cancelled())
4747
{
48-
m_dispatcher.blocking_tick(m_cancelSource);
48+
m_dispatcher.blocking_tick(m_cancellationSource);
4949
}
5050

5151
// There should not be any outstanding work during the shutdown sequence

‎Core/AppRuntime/Source/WorkQueue.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace Babylon
1919
template<typename CallableT>
2020
void Append(CallableT callable)
2121
{
22-
if (m_cancelSource.cancelled())
22+
if (m_cancellationSource.cancelled())
2323
{
2424
// There is likely a coding error if this exception is thrown.
2525
throw std::runtime_error{"Cannot append to the work queue after it is canceled"};
@@ -54,7 +54,7 @@ namespace Babylon
5454

5555
std::optional<std::scoped_lock<std::mutex>> m_suspensionLock{};
5656

57-
arcana::cancellation_source m_cancelSource{};
57+
arcana::cancellation_source m_cancellationSource{};
5858
arcana::manual_dispatcher<128> m_dispatcher{};
5959

6060
std::thread m_thread;

‎Core/JsRuntime/Include/Babylon/JsRuntimeScheduler.h‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,47 +11,48 @@ namespace Babylon
1111
// handling garbage collection and shutdown scenarios. This class provides and manages the schedulers intended
1212
// for a N-API object to use with arcana tasks. It is different than the typical scheduler as this class itself
1313
// is not a scheduler directly, but instead hands out scheduler via its `Get()` function. It provides special
14-
// handling for when the JsRuntime begins shutting down, i.e., when JsRuntime::NotifyDisposing is called:
15-
// 1. The destructor blocks if there are outstanding schedulers not yet invoked on the JavaScript thread.
14+
// handling for when the JsRuntime begins shutting down, i.e., when JsRuntime::NotifyDisposing is called.
15+
// 1. Calling `Rundown` blocks execution until all outstanding schedulers are invoked on the JavaScript thread.
1616
// 2. Once the JsRuntime begins shutting down, all schedulers will reroute its dispatch calls from the
1717
// JsRuntime to a separate dispatcher owned by the JsRuntimeScheduler itself. This class will then be able
1818
// to pump this dispatcher in its destructor to prevent deadlocks.
1919
//
2020
// The typical pattern for an arcana task will look something like this:
21-
//
2221
// class MyClass
2322
// {
2423
// public:
25-
// void MyFunction(const Napi::CallbackInfo& info);
24+
// ~MyClass()
25+
// {
26+
// m_cancellationSource.cancel();
27+
//
28+
// // Wait for asynchronous operations to complete.
29+
// m_runtimeScheduler.Rundown();
30+
// }
31+
//
32+
// void MyFunction(const Napi::CallbackInfo& info)
33+
// {
34+
// const auto callback{info[0].As<Napi::Function>()};
35+
//
36+
// arcana::make_task(arcana::threadpool_scheduler, m_cancellationSource, []() {
37+
// // do some asynchronous work
38+
// }).then(m_runtimeScheduler.Get(), m_cancellationSource, [thisRef = Napi::Persistent(info.This()), callback = Napi::Persistent(callback)]() {
39+
// callback.Call({});
40+
// });
41+
// }
2642
//
2743
// private:
2844
// arcana::cancellation_source m_cancellationSource;
29-
//
30-
// // Put this last so that it gets destructed first.
3145
// JsRuntimeScheduler m_runtimeScheduler;
3246
// };
3347
//
34-
// void MyClass::MyFunction(const Napi::CallbackInfo& info)
35-
// {
36-
// const auto callback{info[0].As<Napi::Function>()};
37-
//
38-
// arcana::make_task(arcana::threadpool_scheduler, m_cancellationSource, []()
39-
// {
40-
// // do some asynchronous work
41-
// }).then(m_runtimeScheduler.Get(), m_cancelSource, [thisRef = Napi::Persistent(info.This()), callback = Napi::Persistent(callback)]() {
42-
// {
43-
// callback.Call({});
44-
// });
45-
// }
46-
//
4748
// **IMPORTANT**:
48-
// 1. To prevent continuations from accessing destructed objects, declare the JsRuntimeScheduler at the end of
49-
// the N-API class. The destructor of the JsRuntimeScheduler will call `Rundown()` which will block until
50-
// all of its schedulers are invoked. If this is not possible, call `Rundown()` manually in the destructor
51-
// of the N-API class.
49+
// 1. To prevent continuations from accessing freed memory, the destructor of the N-API class is expected to call
50+
// `Rundown()` which blocks execution until all of its schedulers are invoked. Failing to do so will result in
51+
// an exception if there are outstanding schedulers not yet invoked.
5252
// 2. The last continuation that accesses members of the N-API object, including the cancellation associated with
53-
// the continuation must capture a persistent reference to the N-API object itself to prevent GC from collecting
54-
// the N-API object during the asynchronous operation.
53+
// the continuation, must capture a persistent reference to the N-API object itself to prevent the GC from
54+
// collecting the N-API object during the asynchronous operation. Failing to do so will result in a hang
55+
// when `Rundown()` is called in the N-API class destructor.
5556
class JsRuntimeScheduler
5657
{
5758
public:
@@ -67,7 +68,10 @@ namespace Babylon
6768

6869
~JsRuntimeScheduler()
6970
{
70-
Rundown();
71+
if (m_count > 0)
72+
{
73+
throw std::runtime_error{"Schedulers for the JavaScript thread are not yet invoked"};
74+
}
7175
}
7276

7377
// Wait until all of the schedulers are invoked.

‎Plugins/NativeCamera/Source/NativeVideo.cpp‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ namespace Babylon::Plugins
4343
{
4444
}
4545

46+
NativeVideo::~NativeVideo()
47+
{
48+
m_cancellationSource.cancel();
49+
50+
// Wait for async operations to complete.
51+
m_runtimeScheduler.Rundown();
52+
}
53+
4654
Napi::Value NativeVideo::GetVideoWidth(const Napi::CallbackInfo& /*info*/)
4755
{
4856
return Napi::Value::From(Env(), m_width);
@@ -133,34 +141,34 @@ namespace Babylon::Plugins
133141

134142
Napi::Value NativeVideo::Play(const Napi::CallbackInfo& info)
135143
{
136-
auto env{info.Env()};
137-
auto deferred{Napi::Promise::Deferred::New(env)};
144+
auto deferred = Napi::Promise::Deferred::New(info.Env());
138145

139146
if (!m_IsPlaying)
140147
{
141148
m_IsPlaying = true;
142149

143150
NativeCameraImpl->Open(m_maxWidth, m_maxHeight, m_frontCamera)
144-
.then(m_runtimeScheduler.Get(), arcana::cancellation::none(), [this, env, deferred](const arcana::expected<Camera::Impl::CameraDimensions, std::exception_ptr>& result)
151+
.then(m_runtimeScheduler.Get(), m_cancellationSource,
152+
[this, thisRef = Napi::Persistent(info.This()), deferred](const arcana::expected<Camera::Impl::CameraDimensions, std::exception_ptr>& result)
145153
{
146154
if (result.has_error())
147155
{
148-
deferred.Reject(Napi::Error::New(env, result.error()).Value());
156+
deferred.Reject(Napi::Error::New(thisRef.Env(), result.error()).Value());
149157
}
150158
else
151159
{
152-
auto cameraDimensions{result.value()};
160+
auto cameraDimensions = result.value();
153161
this->m_width = cameraDimensions.width;
154162
this->m_height = cameraDimensions.height;
155163
this->m_isReady = true;
156-
deferred.Resolve(env.Undefined());
164+
deferred.Resolve(thisRef.Env().Undefined());
157165
RaiseEvent("playing");
158166
}
159167
});
160168
}
161169
else
162170
{
163-
deferred.Resolve(env.Undefined());
171+
deferred.Resolve(info.Env().Undefined());
164172
}
165173

166174
return deferred.Promise();

‎Plugins/NativeCamera/Source/NativeVideo.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Babylon::Plugins
1717
static void Initialize(Napi::Env& env, std::shared_ptr<Plugins::Camera::Impl> nativeCameraImpl);
1818
static Napi::Object New(const Napi::CallbackInfo& info, uint32_t width, uint32_t height, bool frontCamera);
1919
NativeVideo(const Napi::CallbackInfo& info);
20-
~NativeVideo() = default;
20+
~NativeVideo();
2121

2222
void UpdateTexture(bgfx::TextureHandle textureHandle);
2323

@@ -35,6 +35,9 @@ namespace Babylon::Plugins
3535
Napi::Value GetReadyState(const Napi::CallbackInfo& info);
3636
Napi::Value GetHaveCurrentData(const Napi::CallbackInfo& info);
3737

38+
arcana::cancellation_source m_cancellationSource;
39+
JsRuntimeScheduler m_runtimeScheduler;
40+
3841
std::unordered_map<std::string, std::vector<Napi::FunctionReference>> m_eventHandlerRefs{};
3942
uint32_t m_maxWidth{};
4043
uint32_t m_maxHeight{};
@@ -46,9 +49,6 @@ namespace Babylon::Plugins
4649

4750
bool m_IsPlaying{};
4851

49-
// Put this last so that it gets destructed first.
50-
JsRuntimeScheduler m_runtimeScheduler;
51-
5252
static inline std::shared_ptr<Plugins::Camera::Impl> NativeCameraImpl{};
5353
};
5454
}

‎Plugins/NativeEngine/Source/NativeEngine.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,9 @@ namespace Babylon
513513
NativeEngine::~NativeEngine()
514514
{
515515
Dispose();
516+
517+
// Wait for async operations to complete.
518+
m_runtimeScheduler.Rundown();
516519
}
517520

518521
void NativeEngine::Dispose()

‎Plugins/NativeEngine/Source/NativeEngine.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ namespace Babylon
189189
Graphics::FrameBuffer& GetBoundFrameBuffer(bgfx::Encoder& encoder);
190190

191191
arcana::cancellation_source m_cancellationSource{};
192+
JsRuntimeScheduler m_runtimeScheduler;
192193

193194
ShaderCompiler m_shaderCompiler{};
194195

@@ -232,8 +233,5 @@ namespace Babylon
232233

233234
// TODO: This should be changed to a non-owning ref once multi-update is available.
234235
NativeDataStream* m_commandStream{};
235-
236-
// Put this last so that it gets destructed first.
237-
JsRuntimeScheduler m_runtimeScheduler;
238236
};
239237
}

0 commit comments

Comments
 (0)