decentraland / decentraland/unity-explorer
[TECH DEBT] StreamableLoading | Refcount-0 window lets cache eviction destroy assets handed to in-flight loads
- Dominant language
- C#
- Stars
- 23
- Forks
- 17
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 101
Description
### ๐ **Priority Level:**
Medium
### ๐ **Area/Component:**
`ECS.StreamableLoading` โ `LoadSystemBase.CacheableFlowAsync` + `RefCountStreamableCacheBase` (affects every registered ref-counted cache: textures, GLTFs, asset bundles, audio clipsโฆ)
### ๐ **Description:**
There is a window between an asset entering a memory cache and its first consumer taking a reference, during which the asset is a legitimate eviction target even though a load flow is about to hand it to a consumer. An eviction landing inside that window destroys the underlying Unity object, and the in-flight flow then returns a destroyed asset (Unity fake-null) to its consumer.
### ๐ **Current State:**
In `LoadSystemBase.CacheableFlowAsync`:
1. A successful download is added to the memory cache via `PutAsync` with **refcount 0**.
2. Only on a later system tick does `ApplyLoadedResult` run `cache.AddReference` for the consumer.
Between (1) and (2), `CanBeDisposed()` is true. Worse, `StreamableRefCountData.LastUsedFrame` is only stamped by `AddReference`/`Dereference`, so a fresh un-referenced entry has `LastUsedFrame = 0` and `RefCountStreamableCacheBase.Unload` sorts it as the **oldest** entry โ freshly downloaded assets are the *first* eviction candidates during budgeted memory-pressure unloads.
This surfaced as a hard crash in local scene development (path-only hashes), where `ECSReloadScene` runs an unbudgeted `UnloadCache(budgeted: false)` on every reload and reaps every refcount-0 entry, corpsing textures for GLTF imports in flight across the reload (`GltFastDownloadProviderBase.RequestTextureAsync` โ `TextureData.EnsureTexture2D` โ NRE). That specific vector is mitigated by keeping path-only LSD scenes on `NoCache`, but the underlying window still exists everywhere the budgeted cleaner runs โ production included; it is just rare there.
### ๐ก **Proposed Solution:**
Options, roughly in order of preference:
1. Hand the consumer a pre-acquired reference: `AddReference` inside `CacheableFlowAsync` before publishing the result, and have `ApplyLoadedResult` adopt (not re-add) it, dereferencing on abandonment โ closes the window structurally.
2. At minimum, stamp `LastUsedFrame` in `RefCountStreamableCacheBase.Add` so fresh entries are not the top eviction candidates (narrows but does not close the window for unbudgeted drains).
### โ๏ธ **Impact Assessment:**
#### Performance Impact:
None expected; refcount bookkeeping only.
#### Maintainability Impact:
Clarifies asset ownership during the load flow; removes a class of hard-to-reproduce fake-null crashes.
#### Risk of Refactoring:
`LoadSystemBase` is shared by all streamable loaders, so an ownership change touches every asset type. Needs care around abandonment paths (`DisposeAbandonedResult`) and `OngoingRequests` dedup to avoid leaks or double-dereference.
### ๐ **Effort Estimate:**
M
### ๐ **Dependencies:**
Branch `feat/lsd-content-versioned-reload` contains the LSD-specific mitigation (path-only scenes use `NoCache`; `TextureData.EnsureTexture2D` fake-null log removed).
### ๐ฅ๏ธ **Additional Notes:**
Repro evidence: LSD reload of a path-only (no-mtime dev server) scene while Genesis Plaza GLTFs were still streaming โ `NullReferenceException` in `UnityEngine.Object.GetName` via `TextureData.EnsureTexture2D` ~6s after the reload drain.
Contributor guide
Assessment
This issue has not been assessed yet.