Release unused assets from the cloneable asset cache

[toaster0123]

Problem

After an application loads models or materials and stops using them, the cloneable asset cache can keep the original assets alive indefinitely. Even loading another asset, which normally triggers cleanup, does not release those unused entries.

The cache creates a phantom reference to learn when an asset's shared key has been collected, but it does not keep that reference alive. A ReferenceQueue does not retain the references registered with it, so the cleanup notification is lost.

Fix

Keep the existing cleanup reference in its cache entry. When a notification arrives, remove only that entry, so a delayed notification cannot evict a newer asset loaded with an equal key.

The queued reference still does not own the asset. Explicit cache clearing and deletion can therefore release originals without waiting for another cache operation.

Validation

• Core test suite: 487 tests, zero failures, one existing skip
• Four deterministic regression tests cover collection, replacement, deletion followed by reload, and clearing followed by reload. These do not depend on GC timing
• Core main/test Checkstyle tasks pass, with existing warnings outside the changed files
• A separate real-GC smoke test retained all 128 unused originals on the baseline; explicit clear released them. The fix released all 128 during normal cache cleanup in five repeated runs
• Separate queued-reference smoke tests verify that clear/delete release 64 originals even when notifications arrive before or after removal, and that 64 replacement entries survive stale notifications

Validation is limited to the core module. There are no public API or serialized-format changes.

[jaime-jmebot]

The fix correctly retains the KeyRef phantom reference in the cache entry so its ReferenceQueue notification isn't lost. Good catch on the phantom-ref-not-retained issue.

One thing to double-check: a PhantomReference only gets enqueued after its referent (the AssetKey) is collected, but the referent here is the shared/cloned AssetKey. In practice clone keys are usually long-lived (owned by AssetManager), so cleanup notifications may fire rarely. Do you have a test or comment noting that this only releases the original asset memory (the heavy geometry/material data) while the AssetKey object itself may still be strongly held elsewhere? That distinction is worth a brief code comment so future readers understand what actually gets released.

[jaime-jmebot]

The four regression tests look solid — asserting that entries actually disappear from smartCache is the right level, and it keeps them deterministic without relying on GC timing.

One small robustness thought: reaching into the private smartCache field via reflection couples these tests tightly to the field name. If it's ever renamed the tests will fail with a confusing NoSuchFieldException rather than a clear message. Consider a short comment above getEntry noting the reflection is intentional, so the next person doesn't try to "clean it up" by exposing a getter.

[jaime-jmebot]

Nice find — the phantom reference was being registered with the ReferenceQueue but never retained anywhere, so the enqueue notification was simply dropped and the cleanup path never fired. Retaining it inside the AssetRef entry is the minimal, targeted fix.

A couple of small thoughts, left inline:

• Identity check on removal — comparing the notified KeyRef against the stored one before evicting is exactly the right call. Without it, a late notification for an old asset could silently evict a freshly loaded one sharing the same cloned key. Good catch.
• No ownership leak — KeyRef only keeps weak references to its keys, so holding it in the entry doesn't keep the original asset or its key alive. That preserves the whole point of a weak cache.

Just one clarification worth documenting in the code: the phantom reference fires when the shared AssetKey is collected, not the asset itself. Since clone keys are usually held by the AssetManager, cleanup may run rarely in practice — the memory actually reclaimed is the original asset payload. A short comment spelling that out would save the next reader some head-scratching.

The four deterministic tests cover collection, replacement, delete-then-reload, and clear-then-reload without depending on GC timing — that's a solid regression net, and they check internal state directly rather than relying on flaky WeakReference assertions. Approved from my side.

[toaster0123]

Added the ownership/cleanup explanation and intentional-reflection comments in 28ea926.

The tracked referent is the original AssetKey shared by user-visible clones. Keeping that original key alive delays cleanup by design. KeyRef strongly retains only the separate cloned lookup key, not the original key or asset; after the original key is collected, a later addToCache() drains the queue and releases the cached original asset. The comment now makes that distinction explicit.

The test helpers also explain why reflection is intentional: it keeps inspection out of the public API and lets the tests enqueue the tracked reference without relying on GC timing. All four regression tests and both core Checkstyle tasks pass again. This follow-up changes comments only.