Fix cancellation swallowed as AssetRenderException in RenderableAsset render() - #907
Draft
A1shK wants to merge 3 commits into
Draft
Fix cancellation swallowed as AssetRenderException in RenderableAsset render()#907A1shK wants to merge 3 commits into
A1shK wants to merge 3 commits into
Conversation
A1shK
changed the title
A1shK
commented
Jul 20, 2026
| } | ||
| }.also { player.cacheAssetView(assetContext, it) } | ||
| } catch (exception: Throwable) { | ||
| if (exception is CancellationException) throw exception |
Contributor
|
/canary |
….render()
render()'s outer catch didn't special-case CancellationException the way
doRender()'s inner catch already does. When a parent rehydrates while an
in-flight inflateChild() render() call for a child asset is still suspended,
the parent's renewHydrationScope() cancellation gets wrapped into
AssetRenderException("Failed to render asset") instead of propagating as-is,
escaping inflateChild()'s unguarded launch and failing the whole flow.
Includes an isolated repro test demonstrating the race and confirming the fix.
RenderableAsset.render()'s rehydrate branch only called asyncHydrationTrackerPlugin.renderingComplete() from its try/catch (StaleViewException) bodies, not a finally. If rehydrate(cachedView) throws a CancellationException instead (e.g. a second rehydrate for the same asset id cancelling the hydrationScope this one is suspended in), renderingComplete() is skipped, permanently leaking that asset id in the tracker's pending set. onHydrationComplete then never fires again for that flow, hanging IdlingResourcePlugin's waitForIdleSync() until IdlingResourceTimeoutException. Moved renderingComplete() into a finally block, mirroring doRender()'s existing correct pattern. Adds a repro test using an independent sibling asset to isolate the leak (a later successful render for the SAME asset id would otherwise mask it, since pending is a plain Set<String> keyed by id).
A1shK
force-pushed
the
repro/cancellation-swallowed-in-render
branch
from
July 21, 2026 23:00
f2d03fd to
f1bd06f
Compare
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
2 participants
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.
AI generated summary
render()'s outer catch didn't special-case CancellationException the way doRender()'s inner catch already does. When a parent rehydrates while an in-flight inflateChild() render() call for a child asset is still suspended, the parent's renewHydrationScope() cancellation gets wrapped into AssetRenderException("Failed to render asset") instead of propagating as-is, escaping inflateChild()'s unguarded launch and failing the whole flow.
Includes an isolated repro test demonstrating the race and confirming the fix.
Human explanation
We are seeing a few flakes in our tests with errors saying " failed to render asset" and this is one theory that i want to test out. locally it has reduced some flakes but wanted to test it in a canary to actually confirm that.
Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ADoes your PR have any documentation updates?