fix: backpack thumbnails exception handling & improvement on cancelled operations by lorux0 · Pull Request #8824 · decentraland/unity-explorer

lorux0 · 2026-05-19T18:59:09Z

Problem

The grid's cell-input layer is gated behind thumbnail load completion. The path from "cell unblocked" back to "thumbnail ready" had several failure points:

The process for resolving the thumbnail could spin forever when the thumbnail intention gets cancelled.
No consumer-side timeout. Even when the underlying streaming pipeline stalls for non-cancellation reasons, nothing bounded the consumer's wait.
The thumbnail was not retried when it was cancelled due to page switching.

Combined, a stalled or cancelled thumbnail load could leave the grid cell input gated forever, hence the symptoms.

Solution

When the thumbnail is cancelled (due to page switching), its resolved with cancelled result, instead of null.
Added a timeout (set by the consumer) that cancels the pending intention.
When the thumbnail is marked as cancelled, it allows the next load try to be performed instead of keeping it failed forever.
Backpack controller has better exception handling and properly sets a default thumbnail when the operation fails.

Test plan

Open the backpack either wearables or emotes. Switch fast through pages. Check that thumbnails load as expected and you can interact with the items.

github-actions · 2026-05-19T18:59:27Z

Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below.

Name	Link
Commit	`cc7b254`
Logs	https://github.com/decentraland/unity-explorer/actions/runs/26227564176
Download Windows	https://github.com/decentraland/unity-explorer/suites/70124682834/artifacts/7138025240
Download Windows S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/fix/unloaded-backpack-thumbnails/pr-22540-cc7b254/Decentraland_windows64.zip
Download Mac	https://github.com/decentraland/unity-explorer/suites/70124682834/artifacts/7137493187
Download Mac S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/fix/unloaded-backpack-thumbnails/pr-22540-cc7b254/Decentraland_macos.zip
Built on	2026-05-21T14:10:23Z

claude · 2026-05-19T18:59:53Z

Review in Progress

View job run

balducciv · 2026-05-19T21:57:31Z

✅ PR reviewed and approved by QA on both platforms following the PR test instructions.
✅ Smoke test performed on Windows and Mac to verify the normal flow is working as expected.

Build: v0.148.0-alpha-fix/unloaded-backpack-thumbnails-cc81be8
OS: Windows 11 (NVIDIA RTX 4060) / macOS 15 (Apple M3 Pro)

Test results:

Explorer loads successfully — Loading stage: Completed confirmed in Player.log on both platforms
Backpack opens correctly (wearables and emotes)
Fast page switching does not stall thumbnails — thumbnails load as expected after switching
Items remain interactable after rapid page switching (grid cell input is not gated)

Caveats:

BACKPACK, THUMBNAILS, UI, and MVC log categories are suppressed in this build, so log-level validation of the fix is limited. Verdict is based on functional verification.

Unrelated errors noted (do not affect verdict):

GPUInstancerPro.GPUIOcclusionCullingData NullReferenceException at startup — recurring GPU instancing noise, present on both platforms
SidebarController / MinimapController NullReferenceException at dispose time (Windows only) — known shutdown-time MVC cleanup exception
Failed to upload Crash Report to Cloud Diagnostics (Windows only) — cloud diagnostics upload failure, unrelated

Verdict: PASS ✅

Player 8824 windows.log
Player 8824 mac.log

Mac

19.05.2026_18.44.15_REC.-.PR.8824.mp4

Windows

19.05.2026_18.51.07_REC.PR-8824.mp4

lorux0 · 2026-05-20T21:29:08Z

@claude review

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Nicolas Lorusso <56365551+lorux0@users.noreply.github.com>

claude · 2026-05-21T12:51:10Z

Claude Code is working…

I'll analyze this and get back to you.

View job run

claude · 2026-05-21T13:02:12Z

Claude finished @lorux0's task in 3m 12s —— View job

Re-review Complete

Gather context (CLAUDE.md, docs/README.md)
Read updated ResolveAvatarAttachmentThumbnailSystem.cs
Verify blocking issue from previous review is resolved
Check remaining changed files
Post final review

Root Cause

The grid cell input was gated on thumbnail completion, but three independent failure modes could block that gate permanently: cancelled thumbnail intentions resolving with no signal to the consumer, no consumer-side timeout, and no retry path after a page-switch cancellation. The diff addresses all three correctly: a three-state result (Succeeded / Cancelled / Failed), a consumer-side timeout CTS, and resolvers that mark cancelled promises as retriable while preserving sticky failures.

Previous Blocking Issue — Fixed ✅

The unconditional wearable.ThumbnailAssetResult = CancelledResult() that defeated the guard in CompleteWearableThumbnailDownload is gone. Both resolver methods (CompleteWearableABThumbnailDownload and CompleteWearableThumbnailDownload) now correctly guard against overwriting a sticky Failed state:

if (wearable.ThumbnailAssetResult is not { IsInitialized: true })
    wearable.ThumbnailAssetResult = StreamableLoadingResult<SpriteData>.WithFallback.CancelledResult();

The race condition is resolved. The duplicate comment in CompleteWearableABThumbnailDownload is also cleaned up.

Non-Blocking Warning

promiseCts is never disposed in ECSThumbnailProvider.GetAsync (ECSThumbnailProvider.cs line 42)

CancellationTokenSource.CreateLinkedTokenSource(ct) registers a callback on ct. Without calling Dispose(), that registration is held until ct is cancelled or GC'd. Since ct here is the per-page fetch token (restarted by SafeRestart on page change), the leak is bounded to one page's lifetime and does not accumulate unboundedly. The timeoutCts is correctly wrapped in using on line 51 — promiseCts should get the same treatment.

Not blocking given the bounded lifetime, but worth a follow-up.

REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies async cancellation flow, UniTask/CancellationTokenSource lifecycle, a shared ECS loading result struct used across avatar and streaming pipelines, and two backpack grid controllers.
QA_REQUIRED: YES

github-actions · 2026-05-21T13:05:44Z

🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

m3taphysics · 2026-05-21T14:24:58Z

PR #8824, run #26231363611

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric	Change	Baseline	Delta	Improvement
Samples	2700	2700
CPU average	33.3 ms	33.3 ms	-0.0 ms	0.0%	⚪
CPU 1% worst	33.6 ms	33.9 ms	-0.3 ms	0.9%	⚪
CPU 0.1% worst	35.6 ms	37.2 ms	-1.5 ms	4.1%	🟢
GPU average	7.9 ms	7.7 ms	0.1 ms	-1.8%	⚪
GPU 1% worst	21.2 ms	19.7 ms	1.6 ms	-7.9%	🔴
GPU 0.1% worst	26.8 ms	24.2 ms	2.6 ms	-10.9%	🔴

…ethrowing forever ## Problem One transient thumbnail failure poisons that item's thumbnail for the whole session: `ECSThumbnailProvider.GetAsync` rethrows `ThumbnailLoadFailedException` from cache on every subsequent request. The emote wheel's spinner sticks forever (unobserved `UniTaskVoid` chain), and backpack grids replay-log the exception per page render. Sentry family PAD/P9R/PD5 ≈ 260 events/week of pure cached replays (~396 events / ~33 users with the one-shot timeout signals PA1/P9Y included). Introduced by #8824. ## Root cause An initialized non-Succeeded `ThumbnailAssetResult` slot was treated as a permanent terminal state: the only producer of the sticky `Failed` state is the provider's own 30 s consumer-timeout catch, and there was no recovery path until restart. Separately, `EmotesWheelController.WaitForThumbnailAsync` had no catch, so the exception vanished into a `.Forget()` chain with the loading spinner still active. ## Fix 1. `ECSThumbnailProvider`: any initialized non-Succeeded slot (Failed as well as Cancelled) is now clear-and-retry — reset the slot and spawn a fresh promise. The timeout path still writes `Failed()` (it remains the release signal for concurrent waiters); it just stops being permanent. 2. `EmotesWheelController.WaitForThumbnailAsync`: backpack-style catch — OCE returns; other exceptions log via `ReportHub` (THUMBNAILS), fall back to the default thumbnail, and release the spinner (mirrors `BackpackEmoteGridController`). 3. Comment/doc updates stating the new per-attempt invariant (no behavior change). Contract change: `GetAsync` moves from "throws instantly forever after first failure" to "retries per explicit call". All 8 call sites are UI-render-bounded and concurrent waiters are deduped by the in-flight slot signal; worst case a persistently-failing item costs one 30 s promise per grid/wheel open. ## Test New EditMode `ECSThumbnailProviderShould`: `RetryAfterFailedSlotInsteadOfRethrowing` (pin: instant rethrow, 0 promises spawned), `MarkFailedOnTimeoutAndRetryOnNextCall` (pin: cached rethrow without spawning), `ReturnCachedSuccessWithoutSpawningPromise` (guards against an always-retry regression). ## Validation Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/3 as intended (retry test expected 1 promise got 0; timeout test expected 2 got 1; success guard passed) / GREEN PASS 3/3. Fixes #8902 Fixes #8891

…ethrowing forever ## Problem One transient thumbnail failure poisons that item's thumbnail for the whole session: `ECSThumbnailProvider.GetAsync` rethrows `ThumbnailLoadFailedException` from cache on every subsequent request. The emote wheel's spinner sticks forever (unobserved `UniTaskVoid` chain), and backpack grids replay-log the exception per page render. Sentry family PAD/P9R/PD5 ≈ 260 events/week of pure cached replays (~396 events / ~33 users with the one-shot timeout signals PA1/P9Y included). Introduced by #8824. ## Root cause An initialized non-Succeeded `ThumbnailAssetResult` slot was treated as a permanent terminal state: the only producer of the sticky `Failed` state is the provider's own 30 s consumer-timeout catch, and there was no recovery path until restart. Separately, `EmotesWheelController.WaitForThumbnailAsync` had no catch, so the exception vanished into a `.Forget()` chain with the loading spinner still active. ## Fix 1. `ECSThumbnailProvider`: any initialized non-Succeeded slot (Failed as well as Cancelled) is now clear-and-retry — reset the slot and spawn a fresh promise. The timeout path still writes `Failed()` (it remains the release signal for concurrent waiters); it just stops being permanent. 2. `EmotesWheelController.WaitForThumbnailAsync`: backpack-style catch — OCE returns; other exceptions log via `ReportHub` (THUMBNAILS), fall back to the default thumbnail, and release the spinner (mirrors `BackpackEmoteGridController`). 3. Comment/doc updates stating the new per-attempt invariant (no behavior change). Contract change: `GetAsync` moves from "throws instantly forever after first failure" to "retries per explicit call". All 8 call sites are UI-render-bounded and concurrent waiters are deduped by the in-flight slot signal; worst case a persistently-failing item costs one 30 s promise per grid/wheel open. ## Test New EditMode `ECSThumbnailProviderShould`: `RetryAfterFailedSlotInsteadOfRethrowing` (pin: instant rethrow, 0 promises spawned), `MarkFailedOnTimeoutAndRetryOnNextCall` (pin: cached rethrow without spawning), `ReturnCachedSuccessWithoutSpawningPromise` (guards against an always-retry regression). ## Validation Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/3 as intended (retry test expected 1 promise got 0; timeout test expected 2 got 1; success guard passed) / GREEN PASS 3/3. Fixes #8902 Fixes #8891 Includes inspection-warning cleanup in all touched files.

…ethrowing forever ## Problem One transient thumbnail failure poisons that item's thumbnail for the whole session: `ECSThumbnailProvider.GetAsync` rethrows `ThumbnailLoadFailedException` from cache on every subsequent request. The emote wheel's spinner sticks forever (unobserved `UniTaskVoid` chain), and backpack grids replay-log the exception per page render. Sentry family PAD/P9R/PD5 ≈ 260 events/week of pure cached replays (~396 events / ~33 users with the one-shot timeout signals PA1/P9Y included). Introduced by #8824. ## Root cause An initialized non-Succeeded `ThumbnailAssetResult` slot was treated as a permanent terminal state: the only producer of the sticky `Failed` state is the provider's own 30 s consumer-timeout catch, and there was no recovery path until restart. Separately, `EmotesWheelController.WaitForThumbnailAsync` had no catch, so the exception vanished into a `.Forget()` chain with the loading spinner still active. ## Fix 1. `ECSThumbnailProvider`: any initialized non-Succeeded slot is now clear-and-retry — reset the slot and spawn a fresh promise. The timeout path still writes `Failed()` (it remains the release signal for concurrent waiters); it just stops being permanent. 2. `EmotesWheelController.WaitForThumbnailAsync`: backpack-style catch — OCE returns; other exceptions log via `ReportHub` (THUMBNAILS), fall back to the default thumbnail, and release the spinner (mirrors `BackpackEmoteGridController`). 3. With the retry in place `WithFallback.Cancelled` lost its only in-lane reader, but the cancelled-vs-failed distinction is retained: the API pre-exists on dev and an in-flight sibling PR's disposal tests consume `CancelledResult()`; the resolver keeps stamping `Cancelled` for in-flight cancellation and `Failed` for terminal failures (see the Cross-PR note below). 4. Comment/doc updates stating the new per-attempt invariant (no behavior change). Contract change: `GetAsync` moves from "throws instantly forever after first failure" to "retries per explicit call". All 11 call sites are bounded per explicit call and concurrent waiters are deduped per attachment while an attempt is in flight (the signal is per-attachment, not per-attempt — a same-frame stale-entity overlap window exists and is tracked as a follow-up); worst case a persistently-failing item costs one 30 s promise per grid/wheel open. ## Test New EditMode `ECSThumbnailProviderShould`: `RetryAfterFailedSlotInsteadOfRethrowing` (pin: instant rethrow, 0 promises spawned), `MarkFailedOnTimeoutAndRetryOnNextCall` (pin: cached rethrow without spawning), `ReturnCachedSuccessWithoutSpawningPromise` (guards against an always-retry regression). ## Validation Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/3 as intended (retry test expected 1 promise got 0; timeout test expected 2 got 1; success guard passed) / GREEN PASS 3/3. Fixes #8902 Fixes #8891 ## Cross-PR note `WithFallback.Cancelled` / `CancelledResult()` are **retained** — an earlier revision of this branch deleted them (Fix item 3 above describes that revision and is superseded; `2b1374f8d3` reverted the deletion). The pair pre-exists this PR (present at the base commit, `StreamableLoadingResult.cs:58`), and while this branch no longer reads the `Cancelled` discriminator — `GetAsync` now treats every initialized non-Succeeded slot, cancelled or failed, as clear-and-retry — it is not dead API: the in-flight sibling PR `bugsweep/unload-thumbnail-nre-blocks-memory-release` consumes `WithFallback.CancelledResult()` in its thumbnail-disposal test (`StorageThumbnailDisposalShould.cs`), and the resolver's cancellation stamps remain the waiter-release signal via `IsInitialized`. Removing a pre-existing shared-struct member out from under a sibling in-flight PR is out of scope for this bugfix; if the discriminator is still unread once both PRs land, collapsing `Cancelled` into `Failed` can ship as its own cleanup. Includes inspection-warning cleanup in all touched files.

lorux0 added 2 commits May 19, 2026 15:44

fix thumbnail exception handling & improve cancelled operations

89e29cf

add missing meta

cc81be8

lorux0 requested review from a team as code owners May 19, 2026 18:59

github-actions Bot requested a review from Ludmilafantaniella May 19, 2026 18:59

balducciv approved these changes May 19, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

claude Bot reviewed May 20, 2026

View reviewed changes

Comment thread ...orer/Assets/DCL/AvatarRendering/Thumbnails/Systems/ResolveAvatarAttachmentThumbnailSystem.cs Outdated

claude Bot reviewed May 20, 2026

View reviewed changes

Comment thread ...orer/Assets/DCL/AvatarRendering/Thumbnails/Systems/ResolveAvatarAttachmentThumbnailSystem.cs

lorux0 and others added 2 commits May 21, 2026 09:50

fix timeout subsequent cancelled replacement

a2c6981

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Nicolas Lorusso <56365551+lorux0@users.noreply.github.com>

fix timeout subsequent cancellation replacement on ab thumbnail

680caf6

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Nicolas Lorusso <56365551+lorux0@users.noreply.github.com>

This comment has been minimized.

Sign in to view

claude Bot reviewed May 21, 2026

View reviewed changes

Comment thread ...orer/Assets/DCL/AvatarRendering/Thumbnails/Systems/ResolveAvatarAttachmentThumbnailSystem.cs Outdated

dont overwrite a sticky failed thumbnail result

cc7b254

lorux0 enabled auto-merge (squash) May 21, 2026 14:30

sandrade-dcl approved these changes May 28, 2026

View reviewed changes

lorux0 merged commit 5562c42 into dev May 28, 2026
17 of 19 checks passed

lorux0 deleted the fix/unloaded-backpack-thumbnails branch May 28, 2026 09:28

alejandro-jimenez-dcl mentioned this pull request Aug 19, 2026

fix: retry avatar thumbnail loads after a failed attempt instead of rethrowing forever #9790

Draft

Navigation Menu

Uh oh!

fix: backpack thumbnails exception handling & improvement on cancelled operations - #8824

fix: backpack thumbnails exception handling & improvement on cancelled operations#8824
lorux0 merged 5 commits into
devfrom
fix/unloaded-backpack-thumbnails

lorux0 commented May 19, 2026

Uh oh!

github-actions Bot commented May 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented May 19, 2026 •

edited

Loading

Uh oh!

balducciv commented May 19, 2026 •

edited

Loading

Uh oh!

lorux0 commented May 20, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

claude Bot commented May 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

claude Bot commented May 21, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

m3taphysics commented May 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

lorux0 commented May 19, 2026

Problem

Solution

Test plan

Uh oh!

github-actions Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review in Progress

Uh oh!

balducciv commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorux0 commented May 20, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

claude Bot commented May 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

claude Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-review Complete

Root Cause

Previous Blocking Issue — Fixed ✅

Non-Blocking Warning

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

m3taphysics commented May 21, 2026

Framework 13 i7

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented May 19, 2026 •

edited

Loading

claude Bot commented May 19, 2026 •

edited

Loading

balducciv commented May 19, 2026 •

edited

Loading

claude Bot commented May 21, 2026 •

edited

Loading