Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for shutting down during async operations #1141

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
43dce92
WIP: Fixes for shutting down during async operations
bghgary Sep 29, 2022
925d243
Change to explicitly call Rundown
bghgary Sep 29, 2022
c999153
Cannot throw in destructors
bghgary Sep 30, 2022
204efb9
Merge remote-tracking branch 'origin/master' into async-fixes
bghgary Oct 26, 2022
9d1e098
Fix merge issues
bghgary Oct 26, 2022
124def4
Merge with new timeout code with work queue fixes
bghgary Oct 27, 2022
309c4d9
Fix Canvas
bghgary Oct 28, 2022
ad88d30
Fix Android build
bghgary Oct 28, 2022
11ca510
Merge remote-tracking branch 'origin/master' into async-fixes
bghgary Mar 29, 2023
31248b6
Merge branch 'async-fixes' of https://github.com/bghgary/BabylonNativ…
bghgary Mar 29, 2023
9322360
Update arcana.cpp
bghgary Apr 4, 2023
b4a3c73
Temp fixes for MediaStream
bghgary Apr 5, 2023
9305ca8
Add missing std::forward calls for perfect forwarding
bghgary Apr 19, 2023
df246e2
Work queue shutdown fixes
bghgary Apr 28, 2023
7350715
Miscellaneous Windows AppRuntime fixes
bghgary Apr 28, 2023
77dc8f3
Fix typo in AppRuntime for Win32
bghgary Apr 28, 2023
def047c
Better fix for work queue shutdown issue
bghgary May 2, 2023
bdaf1b9
Merge remote-tracking branch 'origin/master' into async-fixes
bghgary May 4, 2023
0ae6cf4
Fix build issues from merge
bghgary May 4, 2023
71efaf4
Update arcana.cpp to include continuation fix
bghgary May 9, 2023
a0db419
Minor style fixes
bghgary May 9, 2023
6909f55
Update comment
bghgary May 9, 2023
e9d229d
Update comment 2
bghgary May 9, 2023
423ed5e
More style fixes
bghgary May 9, 2023
36a4661
Merge AppRuntime and WorkQueue to fix race condition
bghgary May 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Core/AppRuntime/Source/WorkQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ namespace Babylon

void WorkQueue::Run(Napi::Env env)
{
m_env = std::make_optional(env);
m_dispatcher.set_affinity(std::this_thread::get_id());

m_env = std::make_optional(env);

while (!m_cancellationSource.cancelled())
{
m_dispatcher.blocking_tick(m_cancellationSource);
Expand All @@ -53,5 +54,10 @@ namespace Babylon

// There should no longer be any outstanding work once the queue is drained.
assert(m_dispatcher.empty());

// Clear the shutdown queue to make sure the callables are destroyed on this thread.
m_shutdownQueue.clear();
bghgary marked this conversation as resolved.
Show resolved Hide resolved

m_env.reset();
}
}
41 changes: 26 additions & 15 deletions Core/AppRuntime/Source/WorkQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,33 @@ namespace Babylon
template<typename CallableT>
void Append(CallableT callable)
{
if (m_cancellationSource.cancelled())
{
// There is likely a coding error if this exception is thrown.
throw std::runtime_error{"Cannot append to the work queue while shutting down"};
}

// Manual dispatcher queueing requires a copyable CallableT, we use a shared pointer trick to make a
// copyable callable if necessary.
if constexpr (std::is_copy_constructible<CallableT>::value)
{
m_dispatcher.queue([this, callable = std::move(callable)]() { Invoke(callable); });
if (m_cancellationSource.cancelled())
{
m_shutdownQueue.push([callable = std::move(callable)] {});
}
else
{
m_dispatcher.queue([this, callable = std::move(callable)]() {
callable(m_env.value());
});
}
}
else
{
m_dispatcher.queue([this, callablePtr = std::make_shared<CallableT>(std::move(callable))]() { Invoke(*callablePtr); });
if (m_cancellationSource.cancelled())
{
m_shutdownQueue.push([callablePtr = std::make_shared<CallableT>(std::move(callable))] {});
}
else
{
m_dispatcher.queue([this, callablePtr = std::make_shared<CallableT>(std::move(callable))]() {
(*callablePtr)(m_env.value());
});
}
}
}

Expand All @@ -42,18 +54,17 @@ namespace Babylon
void Run(Napi::Env);

private:
template<typename CallableT>
void Invoke(CallableT& callable)
{
callable(m_env.value());
}

std::optional<Napi::Env> m_env{};

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

arcana::cancellation_source m_cancellationSource{};
arcana::manual_dispatcher<128> m_dispatcher{};

using DispatcherT = arcana::manual_dispatcher<128>;
DispatcherT m_dispatcher{};

// Put the callables in a separate queue during shutdown to ensure the callables are destroyed on the right thread.
arcana::blocking_concurrent_queue<DispatcherT::callback_t> m_shutdownQueue{};

std::thread m_thread{};
};
Expand Down