windows: fix multi-viewer runtime hang (UI-thread Blit + FFI callback orphan)#172
Open
mushogenshin wants to merge 13 commits into
Open
windows: fix multi-viewer runtime hang (UI-thread Blit + FFI callback orphan)#172mushogenshin wants to merge 13 commits into
mushogenshin wants to merge 13 commits into
Conversation
The build hook in thermion_dart/hook/build.dart enumerates every
.cpp file under native/src/ and applies platform-specific exclusions
for `windows`, `d3d`, and `linux` paths. There is no analogous
exclusion for `vulkan/`, so on iOS the hook compiles
`native/src/vulkan/VulkanUtils.cpp` and
`native/src/vulkan/BaseVulkanTexture.cpp`. Both reference symbols in
the `bluevk::` namespace, but iOS is Metal-only and the platform
branch never adds `bluevk` to its `libs` list.
Result on iOS device builds (Xcode 15, Flutter master):
Undefined symbols for architecture arm64:
"bluevk::vkMapMemory", referenced from:
thermion::vulkan::readVkImageToBitmap(...) in VulkanUtils-*.o
"bluevk::vkFreeMemory", referenced from: ...
... (~50 more bluevk:: symbols)
ld: symbol(s) not found for architecture arm64
The whole `flutter build ios` fails inside `dart_build` because the
shared lib never links.
Fix: skip vulkan-prefixed source paths when targetOS is iOS, mirroring
the structure of the existing windows / linux exclusions a few lines
above. macOS still gets the vulkan sources (it links `bluevk`),
Android still gets them (also links `bluevk`), Linux still gets them.
Only iOS changes.
Verified: `flutter build ios --debug --no-codesign` now produces a
working `Runner.app` on top of upstream develop.
The mobile gesture handler in `_MobileListenerWidget` used a regular `GestureDetector` whose internal `ScaleGestureRecognizer` waits for movement to cross `kPanSlop` before claiming the gesture arena. When a Thermion view sits inside an ancestor `Scrollable` (e.g. ListView, PageView, CustomScrollView), the ancestor's `VerticalDragGesture- Recognizer` reaches its acceptance threshold first and wins the arena — touches starting on the viewer get interpreted as page scrolls, not viewport gestures. The result on iOS is "sometimes tumbles, sometimes scrolls" depending on drag direction and speed. Fix: switch to `RawGestureDetector` with a `_EagerScaleGesture- Recognizer` subclass that calls `resolve(GestureDisposition.accepted)` inside `addAllowedPointer`. The arena is claimed on PointerDown, so ancestor scrollables never get the chance to win. Single-finger orbit and pinch-zoom both resolve to the viewport regardless of how the viewer is composed. Tap and double-tap can no longer be detected via separate `TapGestureRecognizer` / `DoubleTapGestureRecognizer` entries (the eager scale wins arena before they ever accept), so they're synthesized inside the scale callbacks: a "tap" is a scale gesture whose total focal-point movement stayed below 8 px and whose duration stayed below 250 ms; a "double-tap" is two such taps within 300 ms of each other and within 8 px of each other. Both still feed `InputHandler` via the existing `TouchEvent(tap, ...)` / `TouchEvent( doubleTap, ...)` events, so callers see no API change. Verified: `flutter build ios --debug --no-codesign` succeeds; running on iPhone the multi-viewer stress test (8 viewers in a vertical ListView) now lets the parent scroll only when drags start in the gutter and routes every touch starting on a viewer to the viewport deterministically. Filing alongside the iOS vulkan-source exclusion (PR nmfisher#160).
The Windows branch of the build hook gathers cl.exe options (includes, defines, response file with sources) but stops short of adding the /link separator and /LIBPATH:$libDir. Without those, the linker has no idea where the Filament .lib artifacts that pub get downloaded actually live, so it fails with LNK1104 / LNK2019 on every Filament symbol reference and cl.exe exits 2. The library *inputs* themselves don't need to be added on the command line because native/include/ThermionWin32.h declares all of them via #pragma comment(lib, "filament.lib") and friends, and that header is transitively included by the Windows vulkan/d3d sources plus the generic c_api headers (TCamera.h, TRenderer.cpp, TRenderManager.cpp). The pragma directives emit linker directives that the linker honours automatically — but it still has to find the .lib files via a search path, hence /LIBPATH. Uncomment /link and /LIBPATH:$libDir; leave /DLL out (CBuilder.library already passes /LD which makes /DLL redundant) and leave the individual sources out (they're already in the response file). Verification needed on Windows: I don't have a Windows dev box. Filing this as a separate fork branch so it can be rebased / dropped independently of the iOS fixes (nmfisher#160 vulkan-source exclusion, nmfisher#163 eager scale).
Thermion's build hook routes all log output through a Logger that writes to .dart_tool/thermion_dart/log/build.log. native_toolchain_c captures subprocess (cl.exe, clang, ld) stdout/stderr and routes the stderr through `logger.severe(...)`. On a build failure runProcess throws a ProcessException whose message is just the command line and exit code — the actual compiler / linker output stays inside that build.log file in the pub cache, where it is invisible to CI logs and to anyone running `flutter build` who isn't already poking around in `.dart_tool/`. Result: compile / link failures look completely opaque downstream. The exception output shows the cl.exe command, exit code 2, and nothing else. Has been blocking diagnosis of the Windows native build for several CI iterations. Mirror SEVERE-level records to stderr inside the existing log handler so the real error reaches whoever's watching. Successful builds aren't noisier — compilers don't emit much stderr on success — and the build.log file behavior is unchanged.
native_toolchain_c.runCl already injects /Fe:, /LD, its own /link
separator, /MACHINE, and /LIBPATH (from libraryDirectories) when
dynamicLibrary is set. By manually adding our own '/link
/LIBPATH:$libDir' to the user flags list, cl.exe's parser stopped
reading compile-time flags at our separator — pushing the toolchain's
auto-injected /LD and /Fe: into LINK's argument vector, where LINK
ignored them as LNK4044. With /LD never reaching cl.exe, no /DLL was
forwarded to the linker, which then tried to produce an EXE and bailed
out with LNK1561 ("entry point must be defined").
Drop the redundant /link /LIBPATH and let `libraryDirectories: [libDir]`
do its job. Also drop /VERBOSE from the compile flag list — it's a
linker option that cl.exe parses as deprecated /V<string> (warning
D9035); if the verbose link map is ever needed, it belongs after
native_toolchain_c's own /link separator.
Diagnosed via the stderr-tee added in the prior commit; the smoking
gun was the cl.exe command dump in build.log showing two /link
separators and unrecognized cl flags trailing the first one.
Windows filesystems are case-insensitive, so the basename collision
between native/src/scene/Gizmo.cpp and native/include/material/gizmo.c
caused both source files to compile to the same gizmo.obj path. The
material was added to the source list AFTER the scene class, so its
.obj write-clobbered the class's, leaving thermion::Gizmo's
constructor, pick, highlight, and unhighlight symbols out of the link.
TGizmo.cpp's c_api wrappers then failed with four LNK2019s and a
final LNK1120 ("4 unresolved externals"). LINK's earlier LNK4042
warning ("object specified more than once; extras ignored") was the
breadcrumb.
Rename the material .c file to gizmo_material.c (gizmo.h stays put,
since only the .c basename collided). materialSources entry in
build.dart updated to match. The "gizmo" key still drives the
GIZMO_ENABLED=1 define so existing call sites and tests are
unaffected.
Verified with `flutter test` on Windows after both this and the
preceding cl/link fix: 500/500 tests pass, thermion_dart.dll
produced.
The link hook called CLinker.library(... LinkerOptions.manual(...))
with no `sources`. native_toolchain_c.runCl translates that into:
cl.exe /O2 /LD /Fe:thermion_dart.dll /link /OPT:REF /MACHINE:X64
/LIBPATH:<empty link/ output dir>
i.e. cl.exe with no inputs at all, which exits immediately with
`cl : Command line error D8003 : missing source filename`. Under
`flutter test` the link hook is not invoked, so this only surfaces
during a real `flutter build windows --release` — where it shows up
as the terse "Linking native assets failed" through MSBuild, with
the actual D8003 stranded inside the per-package build.log in the
pub cache.
The link phase here is optional: the build hook already produced
thermion_dart.dll and added a CodeAsset for it. Pass those
build-hook assets through unchanged on Windows. Keep the CLinker
call on platforms where it currently works.
…call
`thermion_flutter_plugin`'s own translation units include Filament
headers (transitively, via `WindowsVulkanContext.h` → `Platform.h`),
but `target_include_directories(... INTERFACE ...)` only exposes the
DART_PKG_HEADERS paths to consumers, not to the plugin itself. cl.exe
then could not find `<utils/...>` headers when compiling the plugin,
producing C3083 / C2039 / C4430 on filament headers.
Switch the scope to PUBLIC so the plugin's own compile and any
consumer both see the headers.
Also drop the immediately-following
include_directories(${PLUGIN_NAME} INTERFACE ...)
call. `include_directories()` accepts only path arguments — the
target name and `INTERFACE` keyword are silently treated as bogus
"directory" entries, so the call doesn't do what its author
intended; it just adds two non-existent paths to the directory-level
include set.
Filament's `backend/Platform.h` friend-declares
`utils::io::ostream& operator<<(...)` without first declaring the
nested namespace. Sibling Filament headers like
`backend/DriverEnums.h` carry the forward declaration
namespace utils::io { class ostream; }
so the rest of Filament compiles cleanly. The plugin's translation
unit reaches `Platform.h` (via `WindowsVulkanContext.h`) before any
of those siblings, leaving the namespace undeclared and cl.exe
rejecting the friend with C3083 ("the symbol to the left of '::'
must be a type"), C2039 ("'ostream': is not a member of 'utils'"),
and C4430.
Mirror Filament's own forward declaration in the plugin header,
just before the WindowsVulkanContext include. Pulling the full
`<utils/ostream.h>` would also work but drags in more than the
friend needs; the namespace + class fwd-decl matches what
DriverEnums.h does.
…missing
`thermion_flutter`'s build hook produces `generated_headers.cmake`
from `thermion_dart`'s metadata during `flutter assemble dart_build`,
which runs as a custom build step inside MSBuild AFTER cmake config
completes. On a fresh checkout the file doesn't exist when cmake
first reads `windows/CMakeLists.txt`, so the unconditional `include()`
errored with "include could not find requested file" and aborted
cmake — leaving the build stuck because the dart_build step never
got to run, so the file was never produced. The only way out was to
fire the hook out of band first (e.g. `flutter test`) and re-run
`flutter run`.
Include conditionally and register the file as a configure
dependency. On a clean checkout this means a two-pass build:
1. First pass: file missing, cmake succeeds with empty
DART_PKG_HEADERS, dart_build runs and writes the file. Plugin
compile fails because the Filament headers are not on the
include path yet.
2. Second pass: cmake re-runs (CMAKE_CONFIGURE_DEPENDS noticed
the new file), DART_PKG_HEADERS is populated, plugin compile
succeeds.
Subsequent builds are no-ops once the file is settled. Status
message points the user at the recovery path so the first-pass
plugin failure is self-explanatory.
…call withVoidCallback was reassigning the file-scope `_voidCallbackNativeCallable` on every invocation, orphaning the previous NativeCallable. Native code calling back through the previous trampoline pointer would then race the Dart GC: once the orphan was finalized, the trampoline's entry in the VM's FFI callback metadata table was removed, and the next invocation hit a `RELEASE_ASSERT` inside `dart::DLRT_GetFfiCallbackMetadata` and called `abort()`, taking down the embedder. Surfaced reliably on Windows multi-viewer (8 viewers × ~60 fps × multiple FFI awaits per frame): GC swept a stale trampoline within ~20 s and the Flutter window died as "Not Responding" while VS caught "Unhandled exception at flutter_windows.dll: Fatal program exit requested." The call stack chain was `abort` → `dart::Assert::Fail` → `dart::DLRT_GetFfiCallbackMetadata` → JIT-compiled Dart trampoline → `thermion_dart` callback site. The module-scoped NativeCallable initialised on its `late` declaration is permanently referenced and its handler (`_voidCallbackHandler`) is stateless — it dispatches by `requestId` through the `_requests` map. A single trampoline is sufficient and correct; reassignment was unnecessary and unsafe. Regression origin: upstream commit 760ae8e ("add makeInt32List method") introduced the reassignment as a drive-by change inside an unrelated commit; no rationale documented. This restores the pre-760ae8ed8 lifetime discipline, matching withPointerCallback / withBoolCallback / etc. which all use local NativeCallables and close() them after the round-trip. Verified on Windows 11 + Intel UHD Graphics + Flutter stable: 8-viewer Thermion stress test ran indefinitely without VM abort.
WindowsVulkanContext::Blit's two vkWaitForFences calls used
UINT64_MAX (infinite wait), and its post-blit graveyard-drain path
called vkDeviceWaitIdle (no timeout parameter at the Vulkan API
level). Blit runs on the Flutter UI thread (called from
ThermionFlutterPlugin::HandleMethodCall for markTextureFrameAvailable),
and the Vulkan→D3D11 shared-handle fence chain between this thread
and Flutter's compositor on its raster thread is mutually dependent:
the GPU can't signal Blit's fence until the compositor has consumed
the previous frame, and the compositor needs the UI thread to pump
Win32 messages to advance. With unbounded waits, a three-way wedge
hung the entire app — UI thread → GPU → compositor → UI thread —
manifesting as "Not Responding" after sustained multi-viewer load.
Both fence waits now bound to 50 ms:
- Pre-submit wait timeout: return early, skip this frame's blit.
Next frame retries. Visual effect under sustained backpressure
is one stale frame, recoverable; previously the entire window
locked up.
- Post-submit wait timeout: log and continue. The submit was
accepted by the queue; the shared texture's keyed mutex
serialises D3D11's read on its own thread, so returning early
doesn't tear. The next call's pre-submit wait reconciles.
If the post-submit wait timed out, the graveyard drain is skipped
entirely. Otherwise we'd fall into vkDeviceWaitIdle which has no
timeout parameter in the Vulkan API — when the GPU is busy enough
that our fence wait just timed out, vkDeviceWaitIdle would block
the UI thread forever, recreating the original hang on a different
line. Graveyard items accumulate briefly; drain resumes the moment
a Blit completes cleanly.
This fix alone doesn't eliminate UI-thread blocking — it makes the
worst case bounded (~50 ms per call) rather than infinite. The
follow-up commit moves Blit off the UI thread entirely via a
dedicated worker thread + bounded queue, which is what restores
responsiveness under load.
…queue) The preceding commit bounded Blit's vkWaitForFences to 50 ms, but under sustained multi-viewer load (8 viewers × Filament frame completions) the Flutter UI thread was still spending most of its wall-clock budget inside markTextureFrameAvailable → Blit → fence wait. The Win32 message pump couldn't keep up; Windows declared the window "Not Responding" even though the process kept rendering. First attempt at moving the work off the UI thread spawned a detached std::thread per call. That immediately crashed inside igvk64.dll (Intel's Vulkan driver) with a 0xC0000005 access violation, because WindowsVulkanContext::Blit reuses a shared command pool / queue / fence / command buffer that the Vulkan spec requires the application to externally synchronise — multiple threads calling Blit at once raced on those resources and the Intel driver dereferenced cleared state. Correct fix: one dedicated worker thread, one std::queue<BlitJob>, one Blit in flight at any instant. The UI thread enqueues (handle + flutterTextureId + context + registrar captured by value) and returns immediately; the worker drains the queue, calls context->Blit(handle) then registrar->MarkTextureFrameAvailable(fid) serially, so the per-Blit Vulkan resources are never touched concurrently. The queue is bounded to kMaxBlitQueueDepth (16). When the worker falls behind the producer (i.e. the GPU/compositor is saturated), the oldest enqueued job is dropped. Visual effect under overload is one stale frame from a previous render reaching the compositor — graceful degradation in place of a hard hang or driver crash. The worker is lazy-started on the first EnqueueBlit call via a compare_exchange_strong on an atomic<bool> guard, so apps that never enter a Thermion screen never spin up the worker. The plugin destructor signals _blitWorkerShouldStop, notifies the cv, and joins — clean teardown across hot restart and app exit. The worker drains the queue before exiting. Verified on Windows 11 + Intel UHD Graphics: 8-viewer Thermion stress screen runs indefinitely (including across a Windows lock screen / unlock cycle) with no "Not Responding", no Intel driver crash, no UI thread saturation. flutter::TextureRegistrar::MarkTextureFrameAvailable and flutter::MethodResult::Success are both documented thread-safe in the Windows embedder, so calling them from the worker thread and from the UI thread respectively is correct.
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
1 participant
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.
Problem
Three distinct bugs that together caused the Flutter window to become unresponsive (
musculature.exe (Not Responding)) on Windows under sustained multi-viewer load (8 viewers × ~60 fps × multiple FFI awaits per frame). Single-viewer was unaffected; macOS / iOS / Android were unaffected.The chain — each fix exposed the next:
Bug #1 — Dart VM
abort()fromdart::DLRT_GetFfiCallbackMetadataDiagnosed via Visual Studio native-debug attach. The
flutter_windows.dllthread that took the unhandled exception was insideabort()←dart::Assert::Fail←dart::DLRT_GetFfiCallbackMetadata← JIT-compiled Dart trampoline ←thermion_dart'swithVoidCallback.DLRT_GetFfiCallbackMetadatais the Dart VM's per-invocation lookup for FFI callback trampolines andRELEASE_ASSERTs when the trampoline pointer is missing from its metadata table — exactly the shape of "native code holds a callback whose Dart-sideNativeCallablewas GC'd."Root cause in
thermion_dart/lib/src/bindings/src/ffi.dart:withVoidCallbackreassigns the file-scope_voidCallbackNativeCallableon every call. Each reassignment orphans the previousNativeCallable; the native side still holds the previous trampoline pointer from a previous round-trip; Dart GC sweeps the orphan, finalises its trampoline, removes its entry from the FFI metadata table; the next callback invocation from native trips the release-assert and the embedder dies.Reliably visible on Windows multi-viewer at sustained load — orphans pile up faster than the GC's grace period. Other platforms / single viewer / lower call rates survive because either churn is lower or GC heuristics differ.
Regression origin: commit
760ae8ed8("add makeInt32List method") added the reassignment as a drive-by change inside an unrelated commit with no documented rationale. Sibling helpers (withPointerCallback,withBoolCallback, …) use localNativeCallables and call.close()after the round-trip — the pattern this commit drifted from.Bug #2 —
WindowsVulkanContext::Blitdeadlocks the Flutter UI threadAfter #1 was fixed, the app no longer
abort()d but became unresponsive instead. Diagnosed via VS Break-All on Main Thread →ThermionFlutterPlugin::HandleMethodCall(markTextureFrameAvailable)→WindowsVulkanContext::Blit→vulkan-1.dll!vkWaitForFences.Blitruns synchronously on the Flutter UI thread (the Dart-sidemarkTextureFrameAvailableis dispatched viaflutter::MethodChannel, which lands on the platform thread). Its twovkWaitForFencescalls usedUINT64_MAX(infinite timeout). The Vulkan→D3D11 shared-handle fence chain is mutually dependent: the GPU can't signal Blit's fence until Flutter's compositor (raster thread) consumes the previous frame, and the compositor needs the UI thread to pump Win32 messages to advance. Three-way wedge: UI thread → GPU → compositor → UI thread.The graveyard-drain branch at the bottom of
Blitalso calledvkDeviceWaitIdle, which has no timeout parameter at the Vulkan API level — so any GPU backpressure wedged the UI thread indefinitely.Bug #3 — Bounded
Blitis still on the UI thread → window stays "Not Responding"After #2's bounded waits (50 ms ceiling), the deadlock was gone but the UI thread was still spending essentially all its wall-clock budget inside
markTextureFrameAvailable → Blit → fence wait. At 8 viewers × Filament frame completions, the Win32 message pump can't pump fast enough; Windows declares the window "Not Responding" even thoughflutter_windows.dllis doing work.The first attempt at moving the work off the UI thread spawned a
std::threadper call. That immediately crashed insideigvk64.dll(Intel's Vulkan driver) with0xC0000005access violation, becauseWindowsVulkanContext::Blitreuses a shared command pool, queue, fence, and command buffer that the Vulkan spec requires the application to externally synchronise. Two concurrent Blits raced those resources and the Intel driver dereferenced cleared state. (We observed 12 simultaneous worker threads all in the driver's command-buffer handling, all crashing; the viewers went blank becauseMarkTextureFrameAvailablewas never being called.)Fixes
Three commits, intentionally split so the diagnostic chain bisects cleanly:
f2145bd9—ffi: don't reassign _voidCallbackNativeCallable per withVoidCallback callDrop the per-call reassignment. The module-scoped
_voidCallbackNativeCallableinitialised on itslatedeclaration is permanently referenced and its handler dispatches byrequestIdthrough_requests, so one trampoline serves every call. Restores the pre-760ae8ed8lifetime discipline, matchingwithPointerCallback/withBoolCallback/ etc.80cba47c—windows: bound Blit's vkWaitForFences; skip graveyard drain on timeoutBoth fence waits bounded to 50 ms. Pre-submit timeout: skip this frame's blit (next frame retries; Flutter shows previous frame for one cycle). Post-submit timeout: log and continue (the submit was accepted; the keyed mutex on the shared texture serialises D3D11's read on its own thread). If the post-submit wait timed out, skip the graveyard drain entirely — otherwise
vkDeviceWaitIdlewould recreate the same deadlock on a different line.6b253b22—windows: move Blit off Flutter UI thread (dedicated worker + bounded queue)One dedicated Blit-worker thread with a bounded
std::queue<BlitJob>(depth 16, drop-oldest backpressure). The UI thread captures(context, registrar, handle, flutterTextureId)by value, pushes to the queue under a mutex, notifies the condition variable, and returns immediately — microseconds total. The worker drains the queue serially; only ever one Blit in flight, so the Vulkan resources stay single-threaded.Lazy-started on first
EnqueueBlitcall (atomic CAS guard); cleanly stopped + joined in the plugin destructor (drains pending jobs first).flutter::TextureRegistrar::MarkTextureFrameAvailableandflutter::MethodResult::Successare both documented thread-safe in the Windows embedder, so calling them from the worker / UI thread respectively is correct.What's affected
WindowsVulkanContext)760ae8ed8discipline)The three commits are independently bisectable. #2 alone is meaningful (eliminates the hard hang). #3 builds on #2 (and is what restores actual responsiveness). #1 is independent of #2/#3.
Verification
Tested on Windows 11 + Intel UHD Graphics, Flutter
stable3.41.2 with--enable-native-assets, in an app pinningthermion_flutter+thermion_dartto this branch:A second Dart-side regression check via the existing
withPointerCallback/withBoolCallbackpaths (which already used local NativeCallables with.close()) confirms that the pattern restored in commit 1 is the upstream lifetime discipline for FFI callbacks in this file — not a new invention.Related
utils::io::ostreamfwd-decl + CMakeOPTIONAL include). This PR depends on windows: fix native build hook (cl/link arg conflict + gizmo.obj name collision) #164's branch for build correctness on Windows — see "branched offfix/windows-cl-link-conflict" in the commit graph. Conceptually orthogonal to runtime fixes.Together with #160, #163, #164, and #167, this completes the cross-platform Thermion multi-viewer spike (iOS, Android, macOS, Windows) for a real downstream Flutter app. The downstream context: Kineograph, an educational app shipped on iOS/Android/macOS/Windows, evaluating Thermion as a replacement for an existing WebView + Three.js voxel renderer with feature parity across the platform matrix.