Release the application references on destroy - #9190
Conversation
The module scoped application reference in app-base.js, exported as pc.app, was assigned in the constructor and on every frame update, but never cleared by AppBase#destroy - unlike the equivalent globals (getApplication, script.app and AppBase._applications). A destroyed application was therefore kept alive indefinitely, along with everything still referenced by it, such as its asset registry and all of its assets, unless a new application happened to replace it. ResourceLoader#destroy now clears its application reference as well, so that a handler or loader which outlives the application does not keep it alive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Build size reportThis PR changes the size of the minified bundles.
|
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
Automated PR review by Codex (GPT-5).
Reviewed single- and multi-application teardown behavior, live ES-module binding semantics for pc.app, ResourceLoader request/cache behavior after destroy, retained-loader references, API compatibility, and performance impact. No blocking issues found.
The identity guard correctly preserves a newer active application when an older one is destroyed, while releasing the destroyed loader reference. The focused Application suite passes (3/3), an additional two-application teardown probe passes, and all repository CI checks are green. Because this PR is authored by the account posting the review, GitHub does not permit a formal self-approval; this COMMENTED review records the clean automated result.
1 participant
Found while investigating #3886, and the dominant reason a destroyed application leaks.
app-base.jshas a module scopedappreference, exported publicly aspc.app. It is assigned in theAppBaseconstructor and again on every frame update, butAppBase#destroynever clears it - unlike the equivalent globals right next to it (setApplication(null),script.app = null,AppBase._applications[canvasId] = null).A destroyed application is therefore retained forever, unless a new application happens to overwrite the reference. Everything still referenced by the destroyed instance goes with it - most notably
assets(the fullAssetRegistry, everyAsset, and theirdataandfile.contentspayloads), plusxr, the default layers, the frame graph and the boundtickclosure.Verified with a
WeakRefprobe on the null device: before this change, a destroyed application is uncollectable even when the test holds no reference to it at all, and becomes collectable only once a second application is created; after it, the destroyed application and its registry are collected right away.Changes
AppBase#destroyclears the module scoped reference, guarded byapp === thisso that destroying an old application does not clear the pointer to a newer one (mirroring the existinggetApplication() === thischeck).ResourceLoader#destroyclears its_appreference, so a handler or loader which outlives the application does not keep it alive. Both of its use sites are safe:loadearly-returns once the handlers have been cleared, and the variant lookup inAsset#filealready falls back togetApplication().pc.app,getApplication()andloader._appare all cleared by destroy.Related but independent: #9189 clears the
AssettoAssetRegistryback-reference, which is the other half of #3886.🤖 Generated with Claude Code