Support async WebAssembly: pump V8 foreground tasks + relax TextDecoder labels#170
Open
SergioRZMasson wants to merge 5 commits into
Open
Support async WebAssembly: pump V8 foreground tasks + relax TextDecoder labels#170SergioRZMasson wants to merge 5 commits into
SergioRZMasson wants to merge 5 commits into
Conversation
Babylon Native embeds V8 but never drained the engine's foreground task runner, so any work V8 schedules from background threads (notably async WebAssembly compile completions used by Draco / Basis / KTX2 emscripten glue) never ran. Awaiters of those Promises froze forever, with no exception surfaced to the embedder. Replace the dispatcher's blocking_tick with a non-blocking tick + 1ms sleep, and add a thread-local g_postTickHook drained every iteration. AppRuntime_V8 installs a hook that calls v8::platform::PumpMessageLoop (kDoNotWait) followed by PerformMicrotaskCheckpoint, ensuring deferred V8 work and microtasks make progress on the runtime thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Emscripten-generated WASM glue (notably the Draco decoder) calls
`new TextDecoder('utf8')` (no hyphen). The previous strict check
accepted only `utf-8` / `UTF-8` and threw, breaking any
async-WASM extension whose JS glue parses strings out of the WASM
heap (Draco, Basis, KTX2, ...).
Accept the full set of utf-8 aliases enumerated by the WHATWG
Encoding spec (case-insensitive): utf-8, utf8, unicode-1-1-utf-8,
unicode11utf8, unicode20utf8, x-unicode20utf8.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Annotate each engine-specific AppRuntime file with the mechanism by which WebAssembly.compile / .instantiate completion is delivered to the JS thread, complementing the V8 PumpMessageLoop fix. - AppRuntime_Chakra.cpp: ChakraCore's WASM compile runs synchronously inside the Promise's resolve step, fired through the existing JsSetPromiseContinuationCallback. No platform-task pump is required. - AppRuntime_JavaScriptCore.cpp: JSC uses background Wasm::Worklist threads and posts completion via the JS thread's microtask queue, which is drained automatically on every JS re-entry. Since AppRuntime hands control to JSC on each Dispatch, microtasks drain per tick. Document the only stall scenario (zero further dispatches between WASM call and resolution) and the host-side mitigation. - AppRuntime_JSI.cpp: V8JSI exposes V8's foreground task runner as an injectable JSITaskRunner. TaskRunnerAdapter routes V8's WASM compile completion tasks into AppRuntime.Dispatch, where they fire alongside microtask draining. This is the moral equivalent of the explicit PumpMessageLoop + PerformMicrotaskCheckpoint hook in AppRuntime_V8.cpp, but bridged through V8JSI's own foreground task runner contract rather than a direct PumpMessageLoop call. Also add a forward declaration of internal::SetPostTickHook in each file so future engine-specific pumping (if a Chakra or JSC host discovers a gap) can be wired without further infrastructure changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR unblocks async WebAssembly execution in the V8-backed AppRuntime by ensuring V8 foreground tasks and microtasks are drained during the host dispatcher loop, and it relaxes the TextDecoder polyfill’s UTF-8 label checking to accept WHATWG-specified aliases used by emscripten-generated WASM glue.
Changes:
- Update the AppRuntime dispatch loop to use non-blocking ticks and add a per-thread post-tick hook mechanism.
- In the V8 environment tier, install a post-tick hook that pumps V8 platform tasks and runs a microtask checkpoint so async WASM Promise continuations can resolve.
- Expand
TextDecoder’s accepted UTF-8 labels to include common aliases (e.g.,utf8) with case-insensitive matching; add engine-specific notes for non-V8 paths.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Polyfills/TextDecoder/Source/TextDecoder.cpp | Accept additional UTF-8 encoding labels/aliases case-insensitively to match common emscripten usage. |
| Core/AppRuntime/Source/AppRuntime.cpp | Switch dispatcher loop to tick + brief sleep and add a thread-local post-tick hook to allow engine task pumping. |
| Core/AppRuntime/Source/AppRuntime_V8.cpp | Install/uninstall a post-tick hook that pumps V8 foreground tasks and performs microtask checkpoints. |
| Core/AppRuntime/Source/AppRuntime_JSI.cpp | Add documentation describing why V8JSI already handles async-WASM task routing. |
| Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp | Add documentation about JSC async WASM continuation behavior and why no explicit pump is installed. |
| Core/AppRuntime/Source/AppRuntime_Chakra.cpp | Add documentation about Chakra promise continuation routing and why no platform-task pump is needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Centralize internal::SetPostTickHook declaration in a new
Source/PostTickHook.h private header. Previously the declaration
was duplicated across AppRuntime_V8.cpp, AppRuntime_Chakra.cpp,
and AppRuntime_JavaScriptCore.cpp; centralizing eliminates the
risk of signature drift / ODR issues and matches the reviewer's
suggested layout.
- New header lives under Source/ (private, not exposed to public
consumers) and is referenced from CMakeLists SOURCES so it
appears in IDE source groups.
- All three engine files now `#include "PostTickHook.h"` and
drop their local forward declaration.
- AppRuntime.cpp also includes the header so its definition of
SetPostTickHook is type-checked against the canonical
declaration at compile time.
2. Correct the misleading lambda-capture comment in
AppRuntime_V8.cpp's teardown block. The capture list is
`[platformPtr, isolate]` -- both are captured by value. Update
the comment to describe the actual lifetime hazard: the
captured isolate pointer becomes dangling at isolate->Dispose()
immediately after this scope, so the hook must be cleared while
the isolate is still valid.
Verified the AppRuntime change rebuilds cleanly against the V8
toolchain and the Babylon Native Playground's
"GLTF Buggy with Draco Mesh Compression" validation test continues
to pass post-refactor.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The original test was a regression guard for issue BabylonJS#147 -- a deadlock that could occur in the old blocking_drain-based dispatcher when ~AppRuntime's push(no-op) raced against the worker's wait() inside blocking_drain. It depended on arcana::test_hooks::blocking_concurrent_queue::set_before_wait_callback to position the worker inside the vulnerable wait() window. The dispatcher loop now uses non-blocking tick + brief sleep + a post-tick hook (so engine-side task queues such as V8's foreground task runner -- used to deliver async WebAssembly compile completions -- can be drained between AppRuntime ticks). With that change, the worker thread never enters blocking_drain's wait() path, so: - The before_wait_callback never fires. - The old test hangs in workerInHook.get_future().wait() and the gtest timeout reports a spurious "deadlock". - The specific race the original test guarded against no longer exists in the new code path. Rewrite the test to validate the same invariant (~AppRuntime completes promptly) without depending on internals of the blocking_drain wait state. The new test covers three worker-thread states at the moment of destruction: 1. Idle worker -- destroy immediately after construction; worker is in the tick + sleep loop with no pending work. 2. Callback in flight -- destroy while a Dispatch handler is mid execution on the worker thread; exercises cancel-during-callback. 3. Backlog at destruction -- many no-op dispatches queued without waiting for any to complete; the destructor must drop the unprocessed work without joining on it. Each scenario runs the full lifecycle on a separate thread and the gtest thread detects deadlock via a 5 s wait_for() timeout (same mechanism the original test used). With the new dispatcher loop all three scenarios complete within ~70 ms on a Release V8 build. Also drop the now-unused <arcana/threading/blocking_concurrent_queue.h> include. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds asynchronous WebAssembly support to Babylon Native's V8 path and fixes a strict
TextDecoderpolyfill check that broke emscripten-generated WASM glue. This unblocks every Babylon.js loader/extension that relies on async WASM — most notablyKHR_draco_mesh_compression, but alsoKHR_texture_basisu, KTX2 textures, and any future emscripten-glued path.Tested end-to-end with the Babylon Native
IntegrationTestagainst the Khronos Buggy/Draco model and an internal Yeti GLB (6 Draco-compressed primitives) — load and render now complete cleanly where they previously hung indefinitely.What was broken
Two distinct gaps in Babylon Native's V8 embedding caused
KHR_draco_mesh_compressionmodel loads to silently freeze with no exception surfaced to the embedder:1. V8 foreground task runner was never pumped
WebAssembly.instantiatereturns a Promise that resolves once V8's background WASM compile worker finishes. V8 publishes that completion by posting a task onto its foreground task runner, which the embedder owns and must drain by callingv8::platform::PumpMessageLoop. The previous dispatcher loop usedarcana::manual_dispatcher::blocking_tick, which only woke on AppRuntime-side dispatches — V8 platform tasks were never observed, so the resolving Promise's continuation was never invoked.This presented as a hard freeze inside
BI_createSceneAsync→ImportMeshAsync→DracoDecoder._createModuleAsyncwith no JS exception and no diagnostics. The same gap also affected any other async-WASM extension (Basis, KTX2, future emscripten-glued paths).2.
TextDecoderpolyfill rejected the'utf8'aliasEmscripten-generated WASM glue (notably
draco_wasm_wrapper_gltf.js) callsnew TextDecoder('utf8')(no hyphen) when reading strings out of the WASM heap. The polyfill's encoding check only accepted the exact strings'utf-8'/'UTF-8'and threw, breaking the WASM module bootstrap immediately after the task-pump fix above unblocked it.Per the WHATWG Encoding spec § names and labels,
utf8,utf-8,UTF-8,unicode-1-1-utf-8,unicode11utf8,unicode20utf8, andx-unicode20utf8are all valid aliases for the same encoding, case-insensitive.Changes
Core/AppRuntime/Source/AppRuntime.cppm_dispatcher.blocking_tickwith non-blockingtick+ a 1 ms sleep when no AppRuntime work ran. This lets the dispatcher loop periodically observe engine-side task queues without busy-spinning.internal::g_postTickHookdrained every iteration. The hook is intentionally engine-agnostic: each engine'sRunEnvironmentTiercan install one if it needs to pump an engine-specific queue.Core/AppRuntime/Source/AppRuntime_V8.cppInstalls a post-tick hook that:
v8::platform::PumpMessageLoop(platform, isolate, kDoNotWait)in a loop until the platform task queue is empty.isolate->PerformMicrotaskCheckpoint()so promise.thencontinuations queued by the pumped tasks run immediately on the same tick.The hook is uninstalled before the isolate is destroyed.
Polyfills/TextDecoder/Source/TextDecoder.cppEncoding label comparison is now case-insensitive and accepts the full set of WHATWG-spec utf-8 aliases. Unsupported labels still throw with the same error string as before.
Other engines — annotated, no behavior change
The remaining three engines have a different async-WASM story; each
AppRuntime_*.cppis annotated documenting the situation so future maintainers know why no analogous pump is required:AppRuntime_JSI.cpp): V8JSI exposes V8's foreground task runner as an injectableJSITaskRunner. The existingTaskRunnerAdapteralready routes V8 platform tasks (including WASM compile completions) throughAppRuntime.Dispatch.AppRuntime_Chakra.cpp): WASM compile runs synchronously inside the Promise's resolve step, which fires through the existingJsSetPromiseContinuationCallback. No separate platform-task pump is required.AppRuntime_JavaScriptCore.cpp): JSC'sWasm::Worklistbackground completion lands on the JS thread's microtask queue, which JSC drains automatically on every JS re-entry — andAppRuntime.Dispatchhands control to JSC on every tick. The one documented edge case (zero further dispatches between WASM call and resolution) is noted with a host-side mitigation.Verification
TextDecoderunit tests inTests/UnitTests/Scripts/tests.tspass unchanged; the new alias acceptance does not affect existing assertions (they exercise default-construct +'utf-8'paths).IntegrationTest(windowed) loadsDRACO_MODEL.glb(Yeti, ~4.3 MB, 6 Draco-compressed primitives) end-to-end. Logs confirm the full WASM bootstrap completes:WASM async probe: PASS→module: script loaded→_createModuleAsync RESOLVED→ all 6 primitives decoded →BI_createSceneAsync RESOLVED. Without these fixes the same model hangs indefinitely inside_createModuleAsync.Risk
TextDecoderchange is strictly additive (more labels accepted) and matches the WHATWG spec, so no callers can regress.Related
BabylonJS/BabylonNativewill bump its JsRuntimeHost pin once this PR lands.