Skip to content

Fix crash in Glide Compose integration during fast scroll (#5739) - #5744

Open
karandhaodiyal28-hash wants to merge 1 commit into
bumptech:move_logic_into_glide_nodefrom
karandhaodiyal28-hash:fix/glide-compose-detached-crash-5739
Open

Fix crash in Glide Compose integration during fast scroll (#5739)#5744
karandhaodiyal28-hash wants to merge 1 commit into
bumptech:move_logic_into_glide_nodefrom
karandhaodiyal28-hash:fix/glide-compose-detached-crash-5739

Conversation

@karandhaodiyal28-hash

Copy link
Copy Markdown

Fixes #5739

During fast scrolling of a \GlideImage\ inside a \LazyColumn/\LazyRow, the app would crash with:

\
java.lang.IllegalStateException: This node does not have an owner.
at androidx.compose.ui.Modifier.Node.getCoroutineScope(Modifier.kt:188)
at com.bumptech.glide.integration.compose.GlideNode.launchRequest.invoke(GlideModifier.kt:405)
\\

Root cause: the existing \isAttached\ guard at the call site (in \onEndApplyChanges) checks the flag synchronously, but the \sideEffect\ that actually calls \coroutineScope\ is invoked later by the Compose runtime. A fast scroll can detach the \GlideNode\ between those two points, so the \coroutineScope\ property getter then throws.

Fix: re-check \isAttached\ inside the \sideEffect\ block, before touching \coroutineScope. If the node is no longer attached, skip the launch — the image is not on screen, so there is no request to issue.

Adds a regression test that mounts 20 \GlideImage\s in a \LazyRow\ and rapidly scrolls back and forth through all indices, then verifies that no exception is thrown and the surviving items still render correctly.

)

During fast scrolling of a GlideImage inside a LazyColumn/LazyRow, the
app would crash with:

  java.lang.IllegalStateException: This node does not have an owner.
      at androidx.compose.ui.Modifier.Node.getCoroutineScope(Modifier.kt:188)
      at com.bumptech.glide.integration.compose.GlideNode.launchRequest\.invoke(GlideModifier.kt:405)

Root cause: the existing isAttached guard at the call site (in
onEndApplyChanges) checks the flag synchronously, but the sideEffect
that actually calls coroutineScope is invoked later by the Compose
runtime. A fast scroll can detach the GlideNode between those two
points, so the coroutineScope property getter then throws.

Fix: re-check isAttached inside the sideEffect block, before touching
coroutineScope. If the node is no longer attached, skip the launch —
the image is not on screen, so there is no request to issue.

Adds a regression test that mounts 20 GlideImages in a LazyRow and
rapidly scrolls back and forth through all indices, then verifies
that no exception is thrown and the surviving items still render
correctly.

Fixes bumptech#5739
@karandhaodiyal28-hash
karandhaodiyal28-hash changed the base branch from master to move_logic_into_glide_node August 16, 2026 14:47
@karandhaodiyal28-hash

Copy link
Copy Markdown
Author

Hi! Quick note on the CI status for transparency — the two failing checks are pre-existing on the base branch and unrelated to the 2-file diff in this PR.

Failing checks

1. \zizmor-output\ (zizmor/unpinned-uses)

Failing on 7 occurrences across 3 workflow files that exist on the \move_logic_into_glide_node\ base:

  • .github/workflows/build.yml\ (lines 12, 13, 17)
  • .github/workflows/gradle-wrapper-validation.yml\ (lines 9, 10)
  • .github/workflows/renovate-config-validation.yml\ (lines 8, 9)

None of these are touched by this PR (diff is +48/-0 across 2 files in \integration/compose/). The base branch's HEAD (\582077df4) has no check runs of its own, but the workflow files it carries are exactly the ones zizmor is flagging on this PR.

Happy to send a follow-up PR that pins the action references to SHAs, or you can address them separately — just let me know.

2. \import/copybara\ (status context)

Errored with "An error happened while migrating the change". Google-internal import infrastructure, not under this PR's control.

What's actually going well

  • \cla/google\ ✅
  • \check-changes\ ✅
  • \zizmor-scan, \zizmor-config, \zizmor-upload\ ✅
  • PR is \MERGEABLE\ per branch protection state
  • The actual \Android CI\ (build.yml) hasn't started yet on this commit — would appreciate confirmation on whether it should trigger automatically

The actual change in this PR

Just two files in \integration/compose/:

  • \GlideModifier.kt\ (+7 lines): re-check \isAttached\ inside the \sideEffect\ block in \launchRequest. Fixes the \IllegalStateException\ from the issue's stack trace.
  • \GlideImageTest.kt\ (+41 lines): \glideImage_rapidLazyRowScroll_doesNotThrowDetachedScopeException\ regression test.

Let me know if you'd like the workflow pinning in a separate PR, or anything else. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in the lazy list

1 participant