Fix launchMolecule startup cancellation race by jmalpasuto · Pull Request #761 · cashapp/molecule

jmalpasuto · 2026-05-21T18:09:14Z

Fixes #760

Ensures launchMolecule completes its synchronous initial composition and produces one value even when its launch context is already cancelled. Cancellation still prevents launched effects and future recompositions.

Keeps composition and snapshot observer cleanup tied to recomposer completion, and cancels the recomposer when initial composition or emitter code fails so startup resources do not leak.

FletchMcKee · 2026-05-22T19:05:40Z

-  launch(finalContext, start = UNDISPATCHED) {
-    try {
-      recomposer.runRecomposeAndApplyChanges()
-    } finally {
-      composition.dispose()
-      snapshotHandle?.dispose()
-    }


It's been a while since I looked through the Recomposer code, but I think this try/finally block needs to wrap recomposer.runRecomposeAndApplyChanges(). With this change if something throws in the when (snapshotNotifier) block, the snapshotHandle would leak and the composition wouldn't be disposed.

I think all you need to do is add the finalContext.ensureActive().

While I dislike silent failure as a failure mode, checking isActive and returning early would be idiomatic with how other launch methods behave on cancelled scopes. That is what I would do.

Yep, looking at some of the tests I wonder if there’s another issue. I remember discovering this snapshotHandle leak when working on #596 as I would see the errorDelayed tests handles leaking into other tests. That’s why I added this check to any of the tests that threw an exception:

// Verify `runRecomposeAndApplyChanges` is no longer active. assertThat(job.isCompleted).isTrue()

But this didn’t work for the errorImmediately tests and I thought it wasn’t an issue because the exception was thrown in the initial composition so it never had a recomposition. For these tests I added job.cancelAndJoin() at the end and thought it was enough but I wonder if this is wrong.

If an exception occurs in the initial composition, I don’t know if anything ever cleans up the snapshotHandle and runRecomposeAndApplyChanges ends up hanging. Maybe something like this is also needed?

if (!finalContext.isActive) return try { composition.setContent { emitter(body()) } } catch (exception: Exception) { finalContext.cancel() throw exception }

There’s gotta be a cleaner way and I may be overthinking this, maybe the job completes on the next frame clock tick, but some food for thought.

Thanks for reviewing! I put up a follow up fix that hopefully addresses these issues... Let me know what you think!

jmalpasuto · 2026-05-26T16:07:19Z

  )

-  return flow!!
+  return flow ?: throw CancellationException("launchMolecule was cancelled before producing an initial value")


I wasn't sure what to return if the flow is null now... CancellationException seemed to make sense?

Hmm... What does stateIn do if you call it on a canceled scope? We should do whatever they do.

Good point... I'll take a peek

okay so looking at stateIn, the non-suspending initial value overload can always return MutableStateFlow(initialValue), even if the scope is cancelled.

the suspending overload (with no initial value) waits for the first value via a CompletableDeferred parented to the scope job, so a cancelled scope throws CancellationException.

since Molecule has no initial value to return, I think the StateFlow overload should probably match the suspending stateIn behavior by throwing the CancellationException?

The problem is that we aren't suspending, so we're not within a coroutine where that exception makes sense. Our function can be called synchronously on the main thread.

What if we wrap ourselves in NonCancelable and then cancel all of the children once we have our initial value?

The assertion here is that we can always complete the initial recomposition to produce a single value, and then only then do we prevent the recomposer and effects from running and producing subsequent values.

good call. updated! if this isn't exactly what you're thinking lmk and / or feel free to make changes as you see fit

simplified this a bit, should be good for another pass

jingibus · 2026-07-14T19:53:32Z

+      emitter(body())
+    }
+  } catch (throwable: Throwable) {
+    recomposer.cancel()


The recomposer, or the recomposerJob?

The recomposer. Canceling only the job could leave effects running whereas canceling the recomposer shuts down both.

jingibus · 2026-07-14T19:55:07Z

+  @Test fun cancelledContextComposesInitialValueBeforeStopping() = runTest {
+    for (mode in listOf(ContextClock, Immediate)) {
+      val job = Job()
+      val scope = CoroutineScope(coroutineContext + BroadcastFrameClock())


Breaks structured concurrency, which is bad — breaking SC can lead to unowned jobs, and concurrency leaks.

Instead, do something like:

(this + BroadcastFrameClock()).launchMolecule<Int>(mode, context = job)

If that hangs, well — good! More coverage from the test.

(This is contra the style used in other tests, I now realize, so - Jake will probably be fine with it)

Good call checking this. The scope keeps runTest’s existing job, so no new unowned job is created.

Ahh, yeah, I missed that. I just automatically scan for CoroutineScope as being problematic. You are right!

jingibus · 2026-07-14T19:55:56Z

+  @Test fun cancelledContextComposesInitialValueBeforeStopping() = runTest {
+    for (mode in listOf(ContextClock, Immediate)) {
+      val job = Job()
+      val scope = CoroutineScope(coroutineContext + BroadcastFrameClock())


Same feedback here.

jingibus · 2026-07-14T23:09:08Z

+      }
+      runCurrent()
+
+      assertThat(flow.value).isEqualTo(1)


Move this before runCurrent() - this should synchronously be the case immediately after launchMolecule.

jingibus · 2026-07-14T23:33:04Z

+  @Test fun cancelledContextComposesInitialValueBeforeStopping() = runTest {
+    for (mode in listOf(ContextClock, Immediate)) {
+      val job = Job()
+      val scope = CoroutineScope(coroutineContext + BroadcastFrameClock())


(This is contra the style used in other tests, I now realize, so - Jake will probably be fine with it)

FletchMcKee reviewed May 22, 2026

View reviewed changes

jmalpasuto commented May 26, 2026

View reviewed changes

jmalpasuto added 5 commits July 14, 2026 14:01

Fix launchMolecule startup cancellation race

6ec6833

Fix launchMolecule startup cancellation cleanup

54a47a8

Move isActive check earlier

3e82207

Allow initial Molecule value before startup cancellation

ed2e853

Simplify launchMolecule startup cleanup

d22e552

jmalpasuto force-pushed the jack--fix-molecule-startup-disposal-race branch from 5d7cab5 to d22e552 Compare July 14, 2026 18:03

jmalpasuto requested review from FletchMcKee, JakeWharton and jingibus July 14, 2026 18:10

jingibus reviewed Jul 14, 2026

View reviewed changes

jingibus approved these changes Jul 14, 2026

View reviewed changes

Assert initial Molecule value synchronously

e9e70b9

Uh oh!

Conversation

jmalpasuto commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FletchMcKee May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmalpasuto commented May 21, 2026 •

edited

Loading

FletchMcKee May 23, 2026 •

edited

Loading