fix: evict corrupt Unity AB cache entries and retry the download once

[alejandro-jimenez-dcl]

Problem

Unity native error Unable to open archive file: <LocalLow cache>/<name>/<hash>/__data
whenever a corrupted entry in Unity's built-in AssetBundle cache is mounted. The affected
bundle (scene, wearable, LOD) then fails permanently: the corrupt entry is never evicted,
so every retry and every future session hits the same broken file; the only recovery today
is the user manually deleting the cache folder. Sentry UNITY-EXPLORER-KFX (ongoing,
8 ev/wk, 353 total); same class on macOS (#8704, #8643).

Root cause

The load pipeline has no recovery path for a cache-served corrupt archive. A cache hit
completes the web request successfully; the null bundle from the failed native mount lands
in a branch that throws without retrying - and even a retry would re-read the same corrupt
file, since a cache hit never reaches the network. Nothing in the runtime ever calls
Caching.ClearCachedVersion (the only Caching call at the pin is an Editor-only menu).
The invariant "a cache entry is either loadable or absent" is maintained nowhere.

Fix (~50 LOC, single locus)

• New internal CorruptAbCacheEvictor: CacheNameFromUrl (Unity keys cache entries by the
  URL's file name) + TryEvict(url, cacheHash) via Caching.ClearCachedVersion.
• LoadAssetBundleSystem.FlowInternalAsync: in the null-bundle branch, when the intention
  carries a cacheHash - switch to main thread, evict, and re-issue the identical request
  exactly once (now a cache miss → real download that re-populates the entry). If still
  null, the existing exception fires. LoadGlobalAssetBundleSystem inherits the flow;
  embedded/StreamingAssets loads never set cacheHash and are untouched.

Worst case for a non-cache null bundle (CDN serves a garbage 200): one extra download
attempt, then the exact pre-fix failure. Out of scope: StreamingAssets/aa install
corruption (#8312/#8131/#7789) - no cache to evict.

Test

New EditMode CorruptAbCacheRecoveryShould:

• RecoverFromCorruptCachedArchive - seeds the real Editor cache with the repo's test
  bundle, corrupts __data in place, drives the real flow (environment preconditions
  guarded by Assume.That).
• RetryExactlyOnceAfterEvictingWhenCachedRequestYieldsNullBundle - deterministic stub
  repro: first request returns a null bundle, second a real one; pin throws after exactly
  one request, fix makes exactly two and succeeds.

Validation

Windows Unity 6000.4 EditMode lane at the pin: RED FAIL as intended on the stub test (the
exact "Asset Bundle is null: Unable to open archive file" signature; the Editor-Caching
test proved non-discriminating under batchmode on this lane - the stub test is the
load-bearing repro) / GREEN PASS 2/2.

Fixes #8023
Related: #8704, #8643 (same defect class on macOS), #9610 (custom IDiskCache hardening -
different cache layer)

Includes inspection-warning cleanup in all touched files.

Fixes #8023

[github-actions]

🚦 CI Status

Build skipped — no changes detected under Explorer/.

No C# files changed — lint ratchet skipped.

All Unity tests passed ✅

TESTS SUITE	Result	Passed	Failed	Skipped
EditMode	✅ Passed	25061	0	13
PlayMode	✅ Passed	236	0	37

[decentraland-bot]

PR Review — #9791 fix: evict corrupt Unity AB cache entries and retry the download once

STEP 2 — Root-cause check: PASS ✅

The PR correctly identifies that Unity's built-in Caching never self-evicts corrupt entries. A corrupt cached archive completes its web request (cache hit, no network) but yields a null bundle from the native mount — and the load pipeline had no recovery path. The fix targets the root cause by adding eviction (Caching.ClearCachedVersion) + a single retry at the exact point of failure in FlowInternalAsync, not a symptom workaround.

STEP 3 — Design & integration: PASS ✅

Owner search for CorruptAbCacheEvictor:

CorruptAbCacheEvictor is an internal static class with no state, no lifecycle, no collections. It contains:

• CacheNameFromUrl — URL string parsing (extracting the file-name component as Unity's cache key)
• TryEvict — delegates to Caching.ClearCachedVersion

This is not a system, manager, or lifecycle owner — it is a pure utility that encapsulates a domain-specific URL-to-cache-key mapping and a Unity API call. There is no lifecycle to manage, no persistent state, no per-frame reconciliation.

The recovery logic lives inside LoadAssetBundleSystem.FlowInternalAsync — the exact system that owns the load flow and already handles the null-bundle case. LoadGlobalAssetBundleSystem inherits this flow via class inheritance. The logic is in the right place.

Files searched: PrepareAssetBundleLoadingParametersSystemBase.cs (sets cacheHash from manifest), LoadGlobalAssetBundleSystem.cs (inherits flow), GetAssetBundleIntention.cs (struct definition, cacheHash field), AssetBundleLoadingResult.cs (result type), GetAssetBundleArguments.cs (args struct).

Teardown / consumption trace: No subscriptions, events, callbacks, or persistent state added. The retry is a one-shot inline operation within the existing async flow. No leak concerns.

STEP 4 — Member audit: PASS ✅

Member	Consumers	Assessment
CacheNameFromUrl(URLAddress)	1 (TryEvict)	Distinct operation (URL parsing with query-string stripping + last-segment extraction) warranting named extraction for discoverability and testability. Not a single-use wrapper — it encapsulates a non-obvious Unity caching invariant.
TryEvict(URLAddress, Hash128)	1 (FlowInternalAsync)	Clean delegation to Caching.ClearCachedVersion with the extracted cache name. Correctly documented as main-thread-only.

STEP 5 — Line-level review

Pass A — Blocking issues: None found.

Checked against all 11 blocking-issue categories:

1. ✅ No code quality violations — naming follows camelCase for locals/params, PascalCase for types/methods
2. ✅ No bugs — CacheNameFromUrl handles URLs with/without query strings correctly; LastIndexOf('/', end - 1) is safe for all valid AB URLs (always contain at least one /); Substring bounds are correct
3. ✅ No security vulnerabilities — cache eviction is a local operation on the user's machine, URLs come from internal pipeline (not user input)
4. ✅ No performance issues — eviction + retry is bounded to exactly one attempt, only on cache-eligible loads with null bundles; SwitchToMainThread is required for Unity Caching API
5. ✅ Error handling preserved — if retry also yields null, the existing NullReferenceException throw fires with the retry's error message (more useful than the original's)
6. ✅ Logic is clear and well-commented
7. ✅ No resource/subscription leaks — no new subscriptions, events, or persistent handles
8. ✅ No unconsumed infrastructure
9. ✅ No detached async for essential work — retry is awaited inline
10. ✅ Nullability contracts correct — metadataJson change from string? to string with ?? string.Empty is safe (string.IsNullOrEmpty handles both); pattern matching in WaitForDependencyAsync is a correctness improvement (catches Succeeded == true with null Asset)
11. ✅ No false-intent conditions

Pass B — Design, encapsulation & resource smells: None found.

• Construction: No new constructors or DI changes
• Naming: CorruptAbCacheEvictor accurately describes its responsibility; method names are clear
• Encapsulation: Behavior lives with its data (URL parsing in the evictor, retry logic in the load system)
• Constants: No magic values introduced
• Resource lifecycle: No GPU textures, pools, or caches added

Inspection-warning cleanup changes are all correct:

• Removed unused DCL.Optimization.PerformanceBudgeting import
• Removed unused reportCategory parameter from LoadAllAssetsAsync (confirmed unused in method body)
• Renamed metadataJSON → metadataJson (camelCase for locals)
• Renamed parentEntityID → parentEntityId (camelCase for params)
• Pattern matching improvement in WaitForDependencyAsync — strictly safer than the original

Security review: No security issues found. No secrets, credentials, user input handling, auth changes, or injection vectors. Cache eviction is a bounded local operation.

Test quality: Two well-structured tests covering both integration (real Unity Caching with seeded+corrupted archive) and unit (NSubstitute stub verifying exactly-once retry semantics). Proper Assume.That for environment preconditions, LogAssert.ignoreFailingMessages for expected native errors, and thorough teardown (UnloadAllAssetBundles, ClearAllCachedVersions, system/world disposal).

STEP 6 — Complexity: COMPLEX

Modifies the asset bundle loading pipeline's async flow (recovery path in FlowInternalAsync) and touches Caching API interactions.

STEP 7 — QA: YES

Runtime code is modified (LoadAssetBundleSystem.cs); affects asset loading behavior that is visible to users when bundles fail to load from corrupt cache entries.

STEP 8 — Non-blocking warnings: None

Main.unity not modified.

STEP 9 — Verdict

REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies the asset bundle loading pipeline's async recovery flow and Unity Caching API interactions
QA_REQUIRED: YES

---

Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub

[github-actions]

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

[decentraland-bot]

PR #9791, run #32259874293

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

How to read this table

• Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
• Δ is Change minus Baseline (a negative Δ means Change is faster).
• 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
• ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
• Exceptions per run — the average number of exceptions in a run's log; more than the baseline is flagged 🔴 even when frame times look fine. The Exception breakdown under each table groups them by the explorer's report category and exception type (as totals across the runs).
• A run that logged unusually many exceptions (at least 10 and 5× the median of its build's runs — e.g. a service was down during it) is excluded from all numbers and called out under the table.

Intel Core i5

Metric	Baseline	Change	Δ	Result
Samples	2313 (×3)	2369 (×3)
CPU average	38.6 ms (33.4–38.8)	37.7 ms (37.4–37.7)	-0.9 ms	⚪ within noise
CPU 1% worst	322.6 ms (57.0–343.5)	297.1 ms (285.9–304.5)	-25.5 ms	⚪ within noise
CPU 0.1% worst	344.1 ms (341.1–360.3)	314.7 ms (312.8–331.9)	-29.4 ms	🟢 9% faster
GPU average	9.5 ms (9.2–9.6)	9.4 ms (9.3–9.5)	-0.1 ms	⚪ within noise
GPU 1% worst	35.6 ms (23.5–37.7)	31.5 ms (30.2–32.9)	-4.0 ms	⚪ within noise
GPU 0.1% worst	44.4 ms (39.8–45.0)	38.2 ms (36.6–41.9)	-6.2 ms	🟢 14% faster
Exceptions per run	66	66	0	⚪ none new

Exception breakdown

Exception	Baseline (3 runs)	Change (3 runs)
[UI] DllNotFoundException	192	192
[ENGINE] NullReferenceException	3	3
[ENGINE] ObjectDisposedException	3	3

Apple M1

Metric	Baseline	Change	Δ	Result
Samples	4105 (×3)	3958 (×3)
CPU average	21.8 ms (21.8–22.9)	22.6 ms (22.3–22.9)	0.7 ms	⚪ within noise
CPU 1% worst	215.9 ms (215.7–217.7)	233.5 ms (231.0–234.5)	17.7 ms	🔴 8% slower
CPU 0.1% worst	226.3 ms (222.9–228.8)	238.5 ms (233.2–239.2)	12.2 ms	🔴 5% slower
GPU average	2.5 ms (2.0–3.2)	3.2 ms (2.4–8.1)	0.7 ms	⚪ within noise
GPU 1% worst	34.3 ms (34.2–36.2)	35.3 ms (35.1–36.0)	0.9 ms	⚪ within noise
GPU 0.1% worst	36.3 ms (34.8–37.5)	36.4 ms (36.2–37.2)	0.1 ms	⚪ within noise
Exceptions per run	0	0	0	⚪ none new

[alejandro-jimenez-dcl]

Approved

[alejandro-jimenez-dcl]

Superseded by #9828, which compounds this fix together with the rest of the bugsweep batch. Closing in favor of that combined PR.