Visitar URL original
fix(imaging): Keep a shared BitmapImage download alive when one requester cancels, drop failed loads from the cache (backport #25101) by agneszitte · Pull Request #25915 · unoplatform/uno · GitHub
Skip to content

fix(imaging): Keep a shared BitmapImage download alive when one requester cancels, drop failed loads from the cache (backport #25101) - #25915

Merged
agneszitte merged 4 commits into
servicing/6.8from
dev/agzi/backport-25101-shared-bitmapimage-download-6.8
Nov 2, 2026
Merged

fix(imaging): Keep a shared BitmapImage download alive when one requester cancels, drop failed loads from the cache (backport #25101)#25915
agneszitte merged 4 commits into
servicing/6.8from
dev/agzi/backport-25101-shared-bitmapimage-download-6.8

Conversation

@agneszitte

@agneszitte agneszitte commented Nov 2, 2026

Copy link
Copy Markdown
Member

GitHub Issue: Related to #25096

PR Type:

🐞 Bugfix

What changed? 🚀

Manual backport of #25101 to servicing/6.8, for 6.7 SR3. It builds on #25911 (the IgnoreImageCache fix, already on this branch).

  • Before: on Skia, BitmapImages that open the same URI share one cached download. When one of them cancelled its open (for example a recycled ListView container), the shared download was cancelled too, and the other images stayed blank. A failed download also stayed in the cache, so later images for the same URI failed as well.
  • After: a shared download keeps running when one image cancels, and a failed download is dropped from the cache, so the next request downloads again.
  • Why not Mergify: on 6.x the code is in BitmapImage.skia.cs, LRUCache is under Uno.UWP/Helpers, and the test is under Tests/Windows_UI_Xaml_Media_Imaging. The 4 commits are master's with only those paths changed; the changed lines are the same.

PR Checklist ✅

Local validation (Skia Desktop, Windows)

Runtime tests on this branch, filtered to Tests.Windows_UI_Xaml_Media_Imaging and Given_Image: 55 of 57 passed, including the 4 Given_BitmapImage tests (3 of them new in this PR).

  • Given_WriteableBitmap.When_WriteableBitmap_Assigned_With_Data_Present failed on its pixel check, the same as on this branch without the change. It doesn't use BitmapImage.
  • Given_RenderTargetBitmap.When_Render_Border_GetPixelsAsync was inconclusive: it needs 100% display scaling.

🤖 Generated with Claude Code

mikernet and others added 4 commits November 1, 2026 22:57
@mikernet @claude @agneszitte
fix(imaging): Keep a shared BitmapImage download alive when one reque…
9804660 
…ster cancels

The in-flight load cached by URI and decode size is awaited by every BitmapImage opening the same source, but it ran on the first requester's cancellation token. When that requester re-opened (a recycled ListView container binding a new UriSource), the shared task completed with a cancellation error, so every other BitmapImage for that URI raised ImageFailed and stayed blank, and nothing retried.

The shared load now runs on no requester's token, as WinUI's ImageCache does when a request is released, and each requester waits under its own token so a cancelled open neither fails nor applies a stale result. Uncached loads (IgnoreImageCache or cache disabled) still cancel with their sole requester.

Fixes #25096

Backport note: 6.x paths (BitmapImage.skia.cs, Uno.UWP/Helpers/LRUCache.cs,
Tests/Windows_UI_Xaml_Media_Imaging); the changed lines are the same as on master.

(cherry picked from commit a06c0a8)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikernet @claude @agneszitte
test(imaging): Cover a shared BitmapImage download surviving a cancel…
d6064f7 
…led requester

Uses a loopback server that holds the image bytes until released, so the test cancels the first requester while the download is deterministically in flight.

Backport note: 6.x paths (BitmapImage.skia.cs, Uno.UWP/Helpers/LRUCache.cs,
Tests/Windows_UI_Xaml_Media_Imaging); the changed lines are the same as on master.

(cherry picked from commit cedc6b0)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikernet @claude @agneszitte
fix(imaging): Drop a failed shared BitmapImage load from the cache
998caa6 
A cached load that completes with an error stayed in the LRU until eviction, so one transient failure (offline moment, 503, DNS hiccup) kept failing every later BitmapImage for that URI with no retry. WinUI's provider store does not own its ImageCache: a failed one is dropped as soon as its requesters release it and the next request downloads again. The LRU owns its entries, so a continuation on the shared task now removes the entry when the load fails, guarded so it only removes the entry while it still holds that very task, as ImageProvider::RemoveImageCache does.

Fixes #25096

Backport note: 6.x paths (BitmapImage.skia.cs, Uno.UWP/Helpers/LRUCache.cs,
Tests/Windows_UI_Xaml_Media_Imaging); the changed lines are the same as on master.

(cherry picked from commit b77bdb4)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikernet @claude @agneszitte
test(imaging): Cover a failed BitmapImage download not poisoning late…
5dc1d00 
…r requesters

The gated server can answer the next request with an HTTP 500, so a BitmapImage opened after one failed must download again and open.

Backport note: 6.x paths (BitmapImage.skia.cs, Uno.UWP/Helpers/LRUCache.cs,
Tests/Windows_UI_Xaml_Media_Imaging); the changed lines are the same as on master.

(cherry picked from commit 5254005)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@agneszitte

agneszitte commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

@Mergifyio backport release/stable/6.7

@mergify

mergify Bot commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

backport release/stable/6.7

✅ Backports have been created

Details

@github-actions github-actions Bot added the area/skia ✏️ Categorizes an issue or PR as relevant to Skia label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @agneszitte's task in 2m 2s —— View job

Review: backport of #25101 to servicing/6.8

Verdict: ship — no inline findings.

I diffed this branch (git diff origin/servicing/6.8...HEAD) against gh pr diff 25101's merged diff. The two are line-for-line identical in logic, only repathed as the PR description states: BitmapImage.cs → BitmapImage.skia.cs, Uno.WinRT/Helpers/LRUCache.cs → Uno.UWP/Helpers/LRUCache.cs, and the test landing in the existing Tests/Windows_UI_Xaml_Media_Imaging/Given_BitmapImage.cs (appended after the IgnoreImageCache test from #25911) instead of a new file. No logic drift.

Specific checks performed, no issues found:

  • Cancellation discipline: shared (cached) loads run on CancellationToken.None and each requester awaits via imageDataTask.WaitAsync(ct) + an explicit ct.ThrowIfCancellationRequested() afterward — correctly compensates for Task.WaitAsync's fast path, which returns an already-completed antecedent without honoring an already-canceled token. Uncached loads still run on the sole requester's token, unchanged.
  • LRUCache.Remove(key, value): the ReferenceEquals identity check correctly guards against a stale continuation evicting a newer entry that replaced it under the same key. Confirmed it's only invoked with Task<ImageData> (a reference type) at both existing call sites in this diff — other LRUCache<,> instantiations in the repo (Shape.wasm.cs's Rect, TextBlock.Android.cs's Java.Lang.String) don't use this overload, so no latent misuse today.
  • Root-cause vs. guard: this is a root-cause fix (cancellation token ownership + cache entry lifecycle), not a symptom-level guard.
  • Test coverage: 3 new runtime tests (Given_BitmapImage) each have a red/fix/green story per the PR description, use a deterministic gated loopback HTTP server instead of timing hacks, and are correctly scoped to SkiaDesktop via [PlatformCondition] (matching where the fix applies) with [GitHubWorkItem] linking issue [Skia] BitmapImage: a cached in-flight download is cancelled by its first requester, failing every other BitmapImage sharing it #25096. No [Ignore]/Assert.Inconclusive introduced.
  • Style/platform conventions: tabs, braces, file suffix (.skia.cs) correct for Skia-only behavior change; no new event Action, no .Result/.GetAwaiter().GetResult(), no async-void.
  • Confirmed TestHelper.RetryAssert (used by the new failed-download test) already exists on servicing/6.8, so the test compiles as backported.

No cross-cutting concerns beyond the above. This is a clean, minimal, test-covered backport.

Note: validation here is code-review/diff-comparison only — no build or runtime test execution was performed in this session. The PR description's own local validation (55/57 passed, including the 4 Given_BitmapImage tests) stands as the runtime evidence.

@agneszitte
agneszitte merged commit c7d9174 into servicing/6.8 Nov 2, 2026
13 of 23 checks passed
@agneszitte
agneszitte deleted the dev/agzi/backport-25101-shared-bitmapimage-download-6.8 branch November 2, 2026 04:12
@mergify mergify Bot mentioned this pull request Nov 2, 2026
Merged
2 of 5 tasks
agneszitte added a commit that referenced this pull request Nov 2, 2026
@agneszitte
Merge pull request #25916 from unoplatform/mergify/bp/release/stable/…
5ea9096 
…6.7/pr-25915

fix(imaging): Keep a shared BitmapImage download alive when one requester cancels, drop failed loads from the cache (backport #25101) (backport #25915)
@unodevops

unodevops commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here:

@agneszitte agneszitte mentioned this pull request Nov 2, 2026
Merged
mikernet added a commit to Singulink/uno that referenced this pull request Nov 14, 2026
@mikernet
Merge servicing/6.8 at 5254ace into custom/6.8
d4b8bf0 
Takes Uno's servicing/6.8 from 6951c2c up to 5254ace (42 pull
requests). Conflicts, by area:

IME and TextBox (unoplatform#25984, backport of unoplatform#25804; unoplatform#25952, backport of unoplatform#25619;
unoplatform#25976, backport of unoplatform#25818)
- Takes upstream's composition design (unoplatform#25804): the browser's input is
  the source of truth for the text, compositions included. This drops
  the composition parts of the unoplatform#24502 port (90746b8): its
  TypeScript composition handling, the StartIndex/TextChangePending
  event arguments, the IsReadOnly forwarding to the hidden input, and
  its Given_TextBox.BrowserSoftKeyboard tests.
- Keeps the non-IME browser fixes on top of it, in the form their
  master pull requests have on unoplatform#25804: Enter raised once per press and
  tracked by key code (unoplatform#25174, with Given_TextBox.BrowserEnterKey from
  master in place of the Enter tests that lived in the dropped file),
  the iOS refocus on tap (unoplatform#25185), pastes without text and isIOS from
  BrowserInputHelper.
- Moves the unoplatform#26005 MaxLength fixes onto unoplatform#25804's native input path, as
  master has them: a refused browser edit keeps the TextBox's selection,
  TextBoxView.UpdateTextFromNative reports whether the edit went in, and
  the IME keeps its range on the applied text (master be85699, which
  d226897 had ported onto the unoplatform#24502 IME). Adds that commit's browser
  and platform-applied IME tests.
- TextBox.pointers.skia.cs: upstream's file (unoplatform#25619 iOS touch editing,
  unoplatform#25818, unoplatform#25804), with the unoplatform#25126 port (the hold acts when it starts,
  a touch right-tap keeps the caret) in the form master's unoplatform#25126 branch
  has it on unoplatform#25619, including focusing before the iOS hold timer acts
  and its iOS not-focusable test.

Pointers (unoplatform#25976, backport of unoplatform#25818)
- Tapped and RightTapped are raised once the PointerReleased bubbled
  (unoplatform#25818), with the unoplatform#25732 port's window coordinates (2-argument
  routed event args, RawPosition for ContextRequested), as master's
  unoplatform#25732 branch has them.
- The release wrapper of unoplatform#25818 keeps the unoplatform#25126 port's
  StopContextMenuTimer at the start of the release.

DependencyObject (unoplatform#26142, backport of unoplatform#26123)
- Takes upstream, then the code of unoplatform#25839 (97a2f17), which replaces
  this branch's unoplatform#25102 ports (800247f, be41da6, fd40bf9,
  7546379). DependencyObjectStore.Binder.cs, .alc.cs,
  Application.Alc.cs, ResourceDictionary.alc.cs and their tests now
  match unoplatform#25839; this drops 800247f's own
  SingleParentNonFE_Parent_Removes_Value_Then_Next_Parent_Starts_Clean
  test, which unoplatform#25839 doesn't have.

Imaging (unoplatform#25597, unoplatform#25915, unoplatform#25911, unoplatform#25897)
- Keeps this branch's side: it has the later forms of these fixes
  (unoplatform#25124, unoplatform#25559, unoplatform#25633 and unoplatform#26370 ports), where upstream has the
  6.8-only unoplatform#25597 design and earlier snapshots. Upstream's shared
  download, IgnoreImageCache and failed-load fixes are covered by
  SharedImageDecode. Adds upstream's When_Source_Outlives_Brushes test.

ToolTip (unoplatform#26098, backport of unoplatform#25137 and unoplatform#26096)
- Keeps this branch's unoplatform#25137 port and adds unoplatform#26096 in master's form: the
  service releases only a ToolTip it opened, and leaves one the app
  opened during the show delay to the app. Keeps both sides' tests.

Flyout (unoplatform#26062, backport of unoplatform#26060)
- This branch had the same commits already; keeps the "Keep
  non-resizable flyouts in window" port and its tests.

Also removes a helper pair in Given_AccessibleTextBox that both sides
added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@agneszitte agneszitte

Labels

area/skia ✏️ Categorizes an issue or PR as relevant to Skia

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@agneszitte @unodevops @mikernet