Fix SOG parsers throwing when a load completes after app teardown - #9232
Merged
Conversation
Nothing cancels an in-flight request, so a SOG load callback can run after AppBase#destroy. Teardown drops the asset registry before it marks the graphics device destroyed, so a callback can arrive while app.assets is null and the device still looks alive. _shouldAbort dereferenced app.assets before reaching its guarded device check, throwing a TypeError instead of aborting the load. Check the device first and optional-chain the registry, so the order teardown happens in no longer matters.
Build size reportThis PR changes the size of the minified bundles.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Loading a streamed SOG scene and destroying the app while requests are still in flight throws out of the parser instead of aborting the load cleanly:
Nothing cancels an in-flight request —
ResourceLoader#destroyonly drops its bookkeeping andHttphas no abort — so any of these callbacks can run afterAppBase#destroy. Teardown happens in a fixed order: the asset registry is dropped (this.assets = null) before the graphics device is marked destroyed. That leaves a window whereapp.assetsis null whileapp.graphicsDevice._destroyedis still false._shouldAbortdereferenced the registry in its first condition, before reaching the device check that is already written defensively:Checking the device first is not sufficient on its own, since the registry is nulled earlier than the device is flagged, so the registry access is optional-chained too. The result no longer depends on the order teardown happens in.
Most likely to be hit by streamed LOD scenes, which keep many chunk requests in flight continuously, so almost any teardown — navigation, iframe reload, HMR — lands a callback afterwards.
Changes:
SogParser#_shouldAbortandSogBundleParser#_shouldAbort(both@private) check the graphics device before the asset registry, and tolerate the registry already being gone