IssueTracker

Jetpack Compose version: BOM 2024.10.01

Jetpack Compose component used: runtime (SnapshotStateList, derivedStateOf)

Problem

This is probably WAI, but it's a sharp edge that I wonder if there's some way to smooth off.

This code looks reasonable:

val list = mutableStateListOf(…)
val listMax by derivedStateOf { list.max() }

This is bad though: If listMax is read in the global snapshot, it can throw a ConcurrentModificationException. list.max() iterates over the list, reading the snapshot state object on every iteration. Any changes made to the size of the list during this time, or in a snapshot that is applied during this time, will cause a ConcurrentModificationException to be thrown.

The way to fix this code is to ensure that listMax is only read in a snapshot (i.e. not the global one). Notably, it does not work to create a snapshot inside the derivedStateOf block as that throws an ISE about reading a state object from a snapshot that has not been applied.

This does make sense, but I think the derivedStateOf can give a false sense of security and so it's a very easy footgun to trigger.

Potential solutions

Option 1: Isolate automatically

One possible way to shave off this sharp edge might be to ensure that derivedStateOf always runs its calculation block in an isolating snapshot. derivedStateOf avoids making such a snapshot whenever it's already in a snapshot, but maybe it could explicitly always run the calculation in a snapshot when being read from the global snapshot?

Option 2: Lint

SnapshotStateList operations done in a derivedStateOf could throw a lint warning. I think list operations are one common case where developers might not realize they need to ensure are run in an isolating snapshot, and developers may not realize that derivedStateOf won't necessarily run in an isolated snapshot, and this could at least warn them of that common case.

Full repro

@Test fun concurrentModification() {
  // Start with a large list to give the max function something to iterate over.
  val list = SnapshotStateList<Int>().apply { repeat(1000) { add(0) } }
  val max by derivedStateOf {
    list.max()
  }
  var exception: Throwable? = null

  val mutator = thread(name = "mutator") {
    var add = false
    while (true) {
      Snapshot.withMutableSnapshot {
        // This must be a structural change (i.e. change the list size),
        // can't just write to an offset.
        if (add) {
          list.add(1)
        } else {
          list.removeLast()
        }
        add = !add
      }
    }
  }

  val reader = thread(name = "reader") {
    while (exception == null) {
      try {
        println("max=$max")
      } catch (e: Throwable) {
        exception = e
      }
    }
  }

  while (mutator.isAlive && reader.isAlive) {
    Thread.sleep(500)
  }
  mutator.interrupt()
  reader.interrupt()

  exception?.let { throw it }
}