Skip to content

Fix render-thread crash when a bound ItemsSource is mutated (SkiaLayout snapshot)#301

Open
LennoxP90 wants to merge 1 commit into
taublast:mainfrom
LennoxP90:fix/skialayout-itemssource-render-race
Open

Fix render-thread crash when a bound ItemsSource is mutated (SkiaLayout snapshot)#301
LennoxP90 wants to merge 1 commit into
taublast:mainfrom
LennoxP90:fix/skialayout-itemssource-render-race

Conversation

@LennoxP90

Copy link
Copy Markdown

Fixes #300.

Problem

ViewsAdapter stores the live, bound ItemsSource in _dataContexts and indexes it from the render
thread (GetOrCreateViewForIndexInternal). When a consumer mutates that collection on the UI thread
(Insert/RemoveAt — e.g. a windowed/virtualized list that pages data in and out during scroll), the
render thread tears a read of the resizing backing array → SIGSEGV in the runtime (100% Mono backtrace,
no GPU frames). Reproduces on Accelerated (GL thread) and Default (main thread).

Fix

Index an immutable snapshot of the items from the render thread instead of the live collection:

  • _dataContexts becomes a property whose setter refreshes a volatile object[] _renderSnapshot.
  • GetOrCreateViewForIndexInternal reads _renderSnapshot (atomic reference read; out-of-range simply
    yields no view that frame and self-heals).
  • The snapshot is rebuilt on the UI thread on each data change — per change, not per rendered frame,
    so no per-frame allocations.

Single focused change in src/Shared/Draw/Layout/SkiaLayout.ViewsAdapter.cs. No features removed, no
public API added.

Testing

  • Verified on Android (net10): the minimal repro (scroll-driven moving window — append at edge +
    RemoveAt(0)) crashes reliably on the stock package and no longer crashes with this change; built into
    and exercised via a MAUI app.
  • The change is platform-agnostic C# (no TFM-/platform-specific code). I could not run the full
    iOS/MacCatalyst matrix on a Windows-only host — happy for CI / a Mac to confirm the rest.

Notes / for discussion

  • Behavior: the render reads items as of the last change (the snapshot), refreshed on every
    CollectionChanged; please confirm this composes with the incremental-update paths
    (_HandleSmartCollectionChange etc.).
  • The snapshot rebuild assumes collection mutation happens on the UI/dispatcher thread (the supported
    scenario); it guards defensively if not.

…ut snapshot)

ViewsAdapter stored the live, bound ItemsSource in _dataContexts and indexed it from the
render thread (GetOrCreateViewForIndexInternal). A UI-thread Insert/RemoveAt on that
collection — e.g. a windowed/virtualized list paging data in/out during scroll — races the
render-thread read and tears the resizing backing array, crashing with SIGSEGV in the runtime
(100% Mono backtrace, no GPU frames; reproduces on Accelerated and Default).

Index an immutable snapshot of the data contexts from the render thread instead, rebuilt on
the UI thread on each collection change (per change, not per rendered frame). Single change in
ViewsAdapter.cs; no features removed, no public API added.
taublast added a commit that referenced this pull request Jun 1, 2026
Indexes an immutable snapshot, rebuilt on the
  mutating thread (the one raising CollectionChanged) and swapped in
  atomically via a volatile reference. Covers all render reads: index,
  AttachView, iterator, count.

  Fixes #300
  Refs #301
  Co-authored-by: LennoxP90 <LennoxP90@users.noreply.github.com>
@taublast

taublast commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Hi @LennoxP90 thanks so much for this!

I found that the render thread also indexes the live collection in three more spots beyond GetOrCreateViewForIndexInternal:

  • AttachView — _dataContexts[index] → view.BindingContext (the original crash site)
  • DataContextIterator.MoveNext — _dataContexts[_currentIndex]
  • GetChildrenCount — .Count (can mismatch the snapshot length → IndexOutOfRange)

So I created a derivative version of the fix, which snapshots all four read paths (each captures the snapshot ref once).
Crediting you as co-author on the commit #301 d4281b5 and will credit in the upcoming bulk PR very soon, please let me assemble everything, I see you have another PR incoming, I haven't checked it yet :)
Will close this in favor of the above commit when the final PR is merged. Thanks again for the clean repro and the correct fix direction!

I will look into the repro you provided to make this use-case work well, and make needed changes to the lib to handle this optional use-case. As i see it, it's something like a "two-directional LoadMore with optional removal of already loaded data". This is a nice use-case that could totally be handled internally to minimize the number of remeasurings and boilerplate coding. And by the way, the Scrolled event is not fired on the UI thread.

LennoxP90 added a commit to LennoxP90/DrawnUi that referenced this pull request Jun 4, 2026
Minimal fork patches enabling WFMC's chat-detail virtualized list with
RecyclingTemplate=Enabled (bounded-memory recycling) + MeasureVisible:

1. ViewsAdapter.InitializeSoft - refresh the immutable render snapshot on
   incremental collection change. It was only rebuilt on an ItemsSource REFERENCE
   change; appends to the same ObservableCollection left it frozen at the old
   length, so GetViewForIndex could not realise appended indices (the "only the
   first page renders" pagination bug).

2. SkiaLayout.ApplyAddChange (MeasureVisible, add-at-end) - re-kick background
   measurement from LastMeasuredIndex+1. The one-shot task idles after the initial
   set, so appended (LoadMore) rows were never measured. Cancel first so a stale
   still-flagged task does not dedup-skip the restart.

3. SkiaDrawnCell.ApplyBindingContext - re-measure on recycle (WasMeasured=false +
   InvalidateMeasureInternal). SetContent runs inside LockUpdate, suppressing its
   re-measure, so a recycled cell kept the donor message width (variable-width
   bubbles rendered wrong). Forces re-measure for the new content.

Verified emulator-5554, 914-msg thread: correct render + bounded memory
(~645MB@914, flat on re-scroll; vs ~1.3GB grow-only). Carries PRs taublast#301/taublast#303.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LennoxP90 pushed a commit to LennoxP90/DrawnUi that referenced this pull request Jun 4, 2026
Indexes an immutable snapshot, rebuilt on the
  mutating thread (the one raising CollectionChanged) and swapped in
  atomically via a volatile reference. Covers all render reads: index,
  AttachView, iterator, count.

  Fixes taublast#300
  Refs taublast#301
  Co-authored-by: LennoxP90 <LennoxP90@users.noreply.github.com>

(cherry picked from commit d4281b5)
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.

SkiaLayout render thread indexes the live ItemsSource - SIGSEGV when the bound collection is mutated during scroll

2 participants