Skip to content

viewer: detach view from RenderManager before destroying it on dispose#170

Open
mushogenshin wants to merge 1 commit into
nmfisher:developfrom
mushogenshin:fix/viewer-detach-before-destroy
Open

viewer: detach view from RenderManager before destroying it on dispose#170
mushogenshin wants to merge 1 commit into
nmfisher:developfrom
mushogenshin:fix/viewer-detach-before-destroy

Conversation

@mushogenshin
Copy link
Copy Markdown
Contributor

Problem

`ThermionViewerFFI.dispose()` destroys the underlying Filament `View` but doesn't first remove it from `FFIRenderManager`'s Dart-side `_attachments` map:

```dart
@OverRide
Future dispose() async {
...
View_setScene(view.getNativeHandle(), nullptr);
await FilamentApp.instance!.destroyScene(scene);
await FilamentApp.instance!.destroyView(view); // ← view destroyed without detaching
...
}
```

`_attachments` keys each swap chain to a list of `(renderOrder, View)` tuples. If a view is destroyed while still listed, any later `attach` / `detach` / `_syncViews` call from a different viewer will iterate the now-stale tuple, retrieve the freed view's native pointer, and pass it to `RenderManager_setRenderableRenderThread`. Filament then does a `handle_cast` and aborts with the generic:

```
Postcondition: corrupted heap Handle ... tag=(no tag)
```

Reproduction

Multi-viewer Flutter app where one viewer is disposed concurrently with another viewer being mounted:

  • Viewer A's `dispose()` runs (queued onto whatever the host serialises on, e.g. a static `_createSerial` Future chain).
  • Viewer B's `createTextureAndBindToView` calls `renderManager.attach(viewBView, viewBSwapChain)`, which calls `_syncViews()` — this walks every entry in `_attachments`, including the entry containing the just-destroyed viewer A's view.
  • The freed pointer hits Filament's handle table → SIGABRT.

Symptom on Android (most multi-viewer-heavy platform in practice) is an immediate crash on toggling viewer count. iOS / macOS happen to work because their host plugins don't require the same reattach pattern, but the bug is real on every platform — the symptom is just less reproducible.

Fix

Insert `await FilamentApp.instance!.renderManager.detach(view)` before `destroyView(view)` in `ThermionViewerFFI.dispose()`. `detach(view)` (no swap chain argument) iterates `_attachments` and removes the view from every entry, then runs `_syncViews()` to push the cleaned state to C++ RenderManager. After that, destroying the view is safe — no other code path will try to dereference its handle.

Verification

  • Spike app on Android: stress-test viewer-count toggle (1 → 4 → 1) no longer crashes during the dispose phase.
  • Existing single-viewer and same-viewer-resize paths unaffected — `detach(view)` on a view that's no longer attached is a no-op.

Related

`ThermionViewerFFI.dispose()` destroyed the underlying Filament
view without first removing it from `FFIRenderManager`'s Dart-side
`_attachments` map. The map keys each swap-chain entry to a list
of (renderOrder, View) tuples; if a view is destroyed while still
listed, any later `attach` / `detach` / `_syncViews` call from a
*different* viewer will iterate the now-stale tuple, retrieve the
freed view's native pointer, and pass it to
`RenderManager_setRenderableRenderThread`. Filament then does a
`handle_cast` and aborts with the generic
"Postcondition: corrupted heap Handle ... tag=(no tag)".

Reproduces reliably in multi-viewer Flutter apps when one viewer
is disposed concurrently with another viewer being mounted:

  - Old viewer.dispose() runs (queued onto whatever the host
    serialises on, e.g. a static `_createSerial` Future chain).
  - New viewer's createTextureAndBindToView calls
    renderManager.attach(newView, newSwapChain), which calls
    `_syncViews()` — this walks every entry in `_attachments`,
    including the entry containing the just-destroyed old view.
  - The freed pointer hits Filament's handle table → SIGABRT.

Symptom on Android (which is the most multi-viewer-heavy platform
in practice) is an immediate crash on toggling viewer count.
iOS / macOS happen to work because their host plugins don't
require the same reattach pattern, but the bug is real on every
platform — the symptom is just less reproducible.

Fix: call `FilamentApp.instance!.renderManager.detach(view)`
before `View_setScene(view, nullptr)` and `destroyView(view)`.
`detach(view)` (no swap chain argument) iterates `_attachments`
and removes the view from every entry, then runs `_syncViews()`
to push the cleaned state to C++ RenderManager. After that,
destroying the view is safe — no other code path will try to
dereference its handle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant