fix: keep checked-out GLTF clones alive when AssetPreLoadCache clears

[alejandro-jimenez-dcl]

Problem

Two coupled Sentry issues in the same sessions: UNITY-EXPLORER-PHE -
EcsSystemException [FinalizeGltfContainerLoadingSystem] wrapping an NRE on a destroyed
Root GameObject while consuming a successfully-resolved promise - and UNITY-EXPLORER-P8N -
"AssetPromise ... is already consumed" thrown every subsequent frame once the NRE aborts
FinalizeLoading between TryConsume and State = Finished (~48 events / 17 users past
week combined; scene content also silently vanishes for already-finished containers).

Root cause

Global-cache/per-scene-lifecycle mismatch → use-after-free. AssetPreLoadCache is a global
cache, but CleanUpAssetPreLoadSystem is registered per scene world: ANY scene teardown
calls Clear(), which disposed every live checked-out clone (GltfTemplate.Copies) -
destroying the Roots of clones other scenes had placed in-world or were still holding in
unconsumed promise results. The consume side then NREs on the dead Root (PHE), and the
abort leaves a consumed-promise/Loading component that throws "already consumed" forever
(P8N). Timeline fits: the clone-on-request mechanism landed in #9001 (2026-06-19); PHE
first appeared v0.158.

Fix (~30 LOC, two files)

1. AssetPreLoadCache.Clear() no longer disposes checked-out clones - clones are owned by
   the containers that checked them out; once the template entry is removed, their normal
   release path routes them into the shared pool, where LRU Unload disposes them (no
   leak). The now-dead Copies tracking is removed.
2. FinalizeGltfContainerLoadingSystem: enforce "consumed ⇒ terminal state" - after a
   successful consume, a destroyed Root finishes the component with FinishedWithError
   (the exact contract failed loads already use, loudly logged) instead of throwing, so the
   known destroyed-Root NRE can no longer arm the P8N cascade (an exception thrown for a
   different reason between consume and the terminal state would still abort mid-block).

Test

• FinalizeGltfContainerLoadingSystemShould.FinalizeWithErrorWhenAssetRootDestroyed -
  reproduces both Sentry issues deterministically at the pin (first update throws the NRE,
  second throws "already consumed").
• New AssetPreLoadCacheShould.KeepCheckedOutClonesAliveOnClear - a checked-out clone's
  Root must survive Clear(); template dereference still received.

Validation

Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/2 as intended (EcsSystemException
on the destroyed GameObject; clone Root destroyed by Clear) / GREEN PASS 2/2.

Fixes #9451
Fixes #9531
Related: #8866, #8291 (older auto-filed dupes of the same system exception), #9001
(context: the clone-on-request mechanism)

[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	25056	0	13
PlayMode	✅ Passed	236	0	37

[decentraland-bot]

PR Review: fix: keep checked-out GLTF clones alive when AssetPreLoadCache clears

---

STEP 1 — Context & Scope

Docs loaded: CLAUDE.md, docs/README.md
Subsystem: Asset loading pipeline — AssetPreLoadCache (global cache), GltfContainerAssetsCache (LRU pool), FinalizeGltfContainerLoadingSystem (promise consumer)

Changed files (9):

• AssetPreLoadCache.cs — core fix: remove clone tracking/disposal from cache
• GltfContainerAssetsCache.cs — simplify dereference path
• FinalizeGltfContainerLoadingSystem.cs — destroyed-Root guard
• FinalizeGltfContainerLoadingSystemShould.cs — new test + inspection cleanup
• AssetPreLoadCacheShould.cs — new test file
• Meta/asmref files for the new test folder

Surrounding files read in full context:

• CleanUpAssetPreLoadSystem.cs — the caller of Clear() (registered per scene world via IFinalizeWorldSystem)
• CleanUpGltfContainerSystem.cs — entity cleanup calls Dereference
• ResetGltfContainerSystem.cs — component reset calls Dereference
• IGltfContainerAssetsCache.cs — interface contract
• GltfContainerAsset.cs — Dispose() implementation (deref + destroy Root)
• Utils.cs — TryDuplicateGltfAssetFromTemplate (clones acquire their own ref on AssetData)
• StaticContainer.cs — global cache construction
• AssetPreLoadPlugin.cs — plugin registration

---

STEP 2 — Root-cause check

Problem: AssetPreLoadCache is global (created once in StaticContainer), but CleanUpAssetPreLoadSystem is registered per scene world. Any scene teardown calls Clear(), which disposed every live checked-out clone via the Copies list — destroying Roots of clones owned by other live scenes.

Does the diff fix the cause or a symptom? The diff fixes the cause:

1. Primary fix (AssetPreLoadCache.Clear): removes Copies tracking entirely and stops disposing checked-out clones. Clones are now exclusively owned by the containers that checked them out, released through Dereference.
2. Resilience fix (FinalizeGltfContainerLoadingSystem): adds a destroyed-Root guard after promise consumption. This handles the broader class of destroyed-Root scenarios (cache Unload/Remove, etc.) and is correctly scoped — it only fires after the promise is consumed and ensures the component reaches a terminal state.

The primary fix is not a null-check workaround or exception swallow — it corrects the ownership model. The resilience fix is belt-and-suspenders for a real edge case (the comment accurately describes the scenario).

PASS ✅

---

STEP 3 — Design & integration

No new long-lived units introduced. The diff removes code (the Copies list, ReleaseGltfInstance method) and adds a guard in an existing system.

Owner search — clone lifecycle:

Lifecycle moment	Owner	File
Clone creation	AssetPreLoadCache.TryGetGltfInstance → called from GltfContainerAssetsCache.TryGet	AssetPreLoadCache.cs:49, GltfContainerAssetsCache.cs:67
Clone usage	FinalizeGltfContainerLoadingSystem (consumes promise, parents Root)	FinalizeGltfContainerLoadingSystem.cs:74
Clone release on entity destroy	CleanUpGltfContainerSystem.DestroyGLTFContainer → cache.Dereference(key, asset)	CleanUpGltfContainerSystem.cs:75
Clone release on component reset	ResetGltfContainerSystem.TryReleaseAsset → cache.Dereference(key, asset)	ResetGltfContainerSystem.cs:69
Clone disposal in Dereference	GltfContainerAssetsCache.Dereference: if template still cached → asset.Dispose() directly; if not → pool for LRU eviction	GltfContainerAssetsCache.cs:99-104
Template release on cache clear	AssetPreLoadCache.Clear → gltfCache.Dereference(key, template, handleAssetLoad: false)	AssetPreLoadCache.cs:112

Post-Clear flow verified: After Clear() removes the template entry, ContainsGltf(key) returns false. When a container later calls Dereference, the clone enters the shared LRU pool (not disposed immediately). The pool's Unload eventually disposes it. No leak.

handleAssetLoad: false in Clear() is correct — it prevents the circular check against ContainsGltf during clearing.

Teardown trace: No new subscriptions, callbacks, or event hookups are introduced. The removed Copies list was the only clone-tracking mechanism, and it is fully excised.

PASS ✅

---

STEP 4 — Member audit

Member	Change	Consumers	Assessment
ReleaseGltfInstance (public)	Removed	Was called only by GltfContainerAssetsCache.Dereference (1 caller)	✅ Safe removal — sole caller now calls asset.Dispose() directly
GltfTemplate.Copies (private)	Removed	Was used only within AssetPreLoadCache (add in TryGetGltfInstance, remove in ReleaseGltfInstance, iterate in Clear)	✅ Clean removal, no orphan refs (rg confirms zero remaining .Copies references)
TryAdd<T> null check	Added guard	Existing callers unchanged	✅ Defensive hardening — T is unconstrained generic, null is legitimate
TryGet<T> [MaybeNullWhen(false)]	Annotation added	Existing callers unchanged	✅ Correct nullability improvement

No new public members. PASS ✅

---

STEP 5 — Line-level review

Pass A — Blocking-issue categories:

No P0 or P1 issues found.

1. Code quality (CLAUDE.md standards): The new ReportHub.LogError at line 92 uses string interpolation ($"..."). This allocates, but it is inside a consumed-promise guard that fires at most once per entity lifecycle — not a hot path. Acceptable.

2. Pattern matching improvement: The diff replaces repeated result.Asset! null-forgiving operators with result.Asset is not { } asset pattern matching. This eliminates 8 ! operators with no justifying comment (CLAUDE.md §Nullable). Net improvement.

3. Null guard in Dereference: The existing Root == null guard at GltfContainerAssetsCache.cs:96 correctly handles destroyed-Root clones that arrive at Dereference — they are silently dropped (no crash, no re-pooling). The new asset.Dispose() at line 102 only runs when Root != null (the guard above returns early otherwise). No double-free risk.

Pass B — Design, encapsulation & resource smells:

4. No construction/DI issues. No new classes, no new dependencies.
5. Naming: All existing — no new types or members to name.
6. Comments: The three new block comments accurately describe what the annotated code does/guarantees (ownership contract, cache-clear rationale, destroyed-Root recovery). They do not narrate external behavior.

Security review: No security issues found. The error log exposes asset name and hash — these are public content identifiers, not sensitive data.

---

STEP 6 — Complexity

COMPLEX — Touches the asset loading pipeline, object pooling, cache management, and resource cleanup paths. Changes cross-cutting ownership semantics between a global cache and per-scene lifecycle systems.

---

STEP 7 — QA assessment

QA_REQUIRED: YES — Modifies runtime GLTF container loading and cache management. Affects what users see (scene content can silently vanish without this fix). The primary scenario to verify: multi-scene navigation where one scene tears down while another has active GLTF containers.

---

STEP 8 — Non-blocking warnings

None. Main scene is not modified.

---

STEP 9 — Verdict

REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies cross-cutting ownership semantics between global AssetPreLoadCache, per-scene cleanup systems, and the GltfContainerAssetsCache LRU pool
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 #9799, run #32262032543

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)	2201 (×3)
CPU average	38.6 ms (33.4–38.8)	40.7 ms (40.5–40.9)	2.1 ms	⚪ within noise
CPU 1% worst	322.6 ms (57.0–343.5)	426.6 ms (412.7–486.8)	104.0 ms	⚪ within noise
CPU 0.1% worst	344.1 ms (341.1–360.3)	445.5 ms (433.3–505.5)	101.4 ms	🔴 29% slower
GPU average	9.5 ms (9.2–9.6)	9.4 ms (9.4–9.5)	-0.1 ms	⚪ within noise
GPU 1% worst	35.6 ms (23.5–37.7)	46.2 ms (44.4–52.5)	10.6 ms	⚪ within noise
GPU 0.1% worst	44.4 ms (39.8–45.0)	55.5 ms (50.3–60.6)	11.1 ms	🔴 25% slower
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)	4124 (×3)
CPU average	21.8 ms (21.8–22.9)	21.7 ms (21.7–22.1)	-0.1 ms	⚪ within noise
CPU 1% worst	215.9 ms (215.7–217.7)	229.4 ms (97.7–233.8)	13.5 ms	⚪ within noise
CPU 0.1% worst	226.3 ms (222.9–228.8)	239.0 ms (233.8–243.8)	12.7 ms	🔴 6% slower
GPU average	2.5 ms (2.0–3.2)	2.9 ms (1.0–6.0)	0.4 ms	⚪ within noise
GPU 1% worst	34.3 ms (34.2–36.2)	34.1 ms (33.8–36.0)	-0.2 ms	⚪ within noise
GPU 0.1% worst	36.3 ms (34.8–37.5)	35.4 ms (35.1–37.1)	-0.9 ms	⚪ within noise
Exceptions per run	0	0	0	⚪ none new

[alejandro-jimenez-dcl]

Approved