launchMolecule can call setContent on a disposed Composition during startup cancellation

[jmalpasuto]

Description

launchMolecule can throw IllegalStateException("The composition is disposed") when the supplied coroutine context is cancelled during startup.

The current implementation creates a Recomposer and Composition, launches recomposer.runRecomposeAndApplyChanges() with CoroutineStart.UNDISPATCHED, disposes the composition in that coroutine's finally, and only then calls composition.setContent { ... }.

If the recomposer coroutine completes due to cancellation before setContent runs, setContent is invoked on an already-disposed composition.

I originally saw this in production Android crash reports with stacks like:

java.lang.IllegalStateException: The composition is disposed
  at androidx.compose.runtime.CompositionImpl.ensureRunning(Composition.kt:1470)
  at androidx.compose.runtime.CompositionImpl.setContent(Composition.kt:634)
  at app.cash.molecule.MoleculeKt.launchMolecule(Molecule.kt:259)
  at app.cash.molecule.MoleculeKt.launchMolecule$default(Molecule.kt:217)
  at app.cash.molecule.MoleculeKt$immediateClockFlow$1$1$1.invokeSuspend(molecule.kt:85)

Repro

This test demonstrates the failure on current trunk and also matches the same ordering in 2.2.0:

@Test fun cancelledContextClockLaunch() = runTest {
  val job = Job()
  val clock = BroadcastFrameClock()
  val scope = CoroutineScope(coroutineContext + clock)

  job.cancel()

  assertFailure {
    scope.launchMolecule(ContextClock, emitter = { fail("emitter should not be called") }, context = job) {
      fail("body should not be composed")
    }
  }.hasMessage("The composition is disposed")
}

Expected behavior

Launching with an already-cancelled context, or one that is cancelled during startup, should fail with normal coroutine cancellation semantics (CancellationException) and should not attempt Composition.setContent after disposal.

Possible fix direction

A small fix might be to check finalContext.ensureActive() before creating/launching the composition, and/or reorder startup so setContent happens before the recomposer coroutine can dispose the composition. If reordering, startup failures before the recomposer coroutine is launched should still dispose the composition and snapshot observer before rethrowing.

[jingibus]

Nice find!

Looking at the code, I think that just doing an ensureActive should be sufficient. All of this stuff is happening in nonsuspending land, so — that'll do it, I imagine.