BoxWithConstraints/SubcomposeLayout behaviour flaw that may lead to unexpected crashes [254645321]

Obsolete

Bug

Status Update

No update yet.

Description

ol...@gmail.com

created issue #1

Oct 20, 2022 04:22PM

Jetpack Compose version: 1.2.1 (also tested on 1.3.0-rc01)

Jetpack Compose component(s) used: foundation, material

Here is the most simplified version of the code that led me to mysterious crashes when using ModalBottomSheetLayout:

Column {
    var value by remember { mutableStateOf(true) }
    Button(
        onClick = {
            value = false
        }
    ) {
        Text("Change state")
    }

    if (value) {
        BoxWithConstraints() {
            assert(value)
        }
    }
}

When the button is clicked the code crashes, although you would not expect this just looking at the code. You would expect BoxWithConstraints to be removed from composition as soon as the value becomes false.

And if you replace BoxWithConstraints with regular Box it would work without assertion crash however.

If I understand correctly, the SubcomposeLayout causes its content to be recomposed without regards to the outer check. Which may be fine if you know about this behaviour and expect it. However in case of higher level composables such as ModalBottomSheetLayout it hides the underlying behaviour which may lead to similar unexpected crashes.

I don't know but to me it seems like a flaw. Please correct me if I'm just not doing something correctly.

Comments

ma...@google.com <ma...@google.com> Oct 20, 2022 04:55PM

Reassigned to an...@google.com.

an...@google.com <an...@google.com> Feb 1, 2023 06:13PM

Reassigned to je...@google.com.

da...@gmail.com <da...@gmail.com> #2Feb 16, 2023 10:57PM

deleted

ch...@google.com <ch...@google.com> #3Apr 4, 2023 11:36PM

Reassigned to ch...@google.com.

Project: platform/frameworks/support
Branch: androidx-main

commit 3a4bb5cc9921539344aea335266a6b068fb69bbb
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 15:00:51 2024

Rename `androidTest` to `androidInstrumentedTest` on `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I42ea11960d7dd83224d8ae8cb256c616735200d1

M savedstate/savedstate/src/androidInstrumentedTest/AndroidManifest.xml
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/ErrorInStaticBlock.android.kt
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/SavedStateRegistryTest.android.kt
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/ViewTreeSavedStateRegistryOwnerTest.android.kt

https://android-review.googlesource.com/3067205

ae...@gmail.com <ae...@gmail.com> #4May 27, 2023 10:20PM

Project: platform/frameworks/support
Branch: androidx-main

commit ccf0ec0a1d84f18fb366140dca86b9ba80b0fcbd
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 16:44:04 2024

Use `kmpDocs` on `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I288623ec97038c5641194828684ad05ba448afff

M docs-tip-of-tree/build.gradle

https://android-review.googlesource.com/3066769

ol...@gmail.com <ol...@gmail.com> #5May 28, 2023 10:25AM

Project: platform/frameworks/support
Branch: androidx-main

commit 5d58fbb28a0613242ca71ac5ec0c30f6f807f909
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 11:15:03 2024

Add `KMP` as a build type on `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I689e0d87ec326c606e3b3b70e6dd2d3323ac3411

M settings.gradle

https://android-review.googlesource.com/3067202

ae...@gmail.com <ae...@gmail.com> #6May 28, 2023 07:54PM

Project: platform/frameworks/support
Branch: androidx-main

commit 435d4ec5635e98b17ee0cbee4022037ff43abe7e
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 12:26:01 2024

Rename `main` to `androidMain` on `savedstate`

Test: N/A
Bug: 334076622
Change-Id: Iec0c7b6d12abe1be6c50a05cda9467430c5c1ccc

M savedstate/savedstate/src/androidMain/AndroidManifest.xml
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/Recreator.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryController.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryOwner.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/View.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/ViewTreeSavedStateRegistryOwner.android.kt
M savedstate/savedstate/src/androidMain/res/values/ids.xml

https://android-review.googlesource.com/3067204

an...@gmail.com <an...@gmail.com> #7May 29, 2023 03:18PM

Project: platform/frameworks/support
Branch: androidx-main

commit 1ceef555faa4f23da4d84d10710635a70477b778
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 14:59:42 2024

Convert `ErrorInStaticBlock` to Kotlin

Test: N/A
Bug: 334076622
Change-Id: I0486d8d7c62ea76f277a2df2b3a47bd7fde7d838

M savedstate/savedstate/src/androidTest/java/androidx/savedstate/ErrorInStaticBlock.kt

https://android-review.googlesource.com/3066904

jo...@gmail.com <jo...@gmail.com> #8May 31, 2023 07:00AM

Project: platform/frameworks/support
Branch: androidx-main

commit 488858fc6b2a8df6b4738a5edc991c2d57bb496e
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 12:20:18 2024

Configure multiplatform plugin on `savedstate`

Test: N/A
Bug: 334076622
Change-Id: If360f8f2595d211b541830da906cb3f8078b74ab

M savedstate/savedstate/build.gradle

https://android-review.googlesource.com/3067203

ae...@gmail.com <ae...@gmail.com> #9May 31, 2023 09:41AM

Project: platform/frameworks/support
Branch: androidx-main

commit 05f0fb1eca3aba60adf6229d42e2c09452a59a96
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 14:59:42 2024

Convert `ErrorInStaticBlock` to Kotlin in `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I070f642916bf2da4660efddf9d6b9e86d659bc52

M savedstate/savedstate/src/androidTest/java/androidx/savedstate/ErrorInStaticBlock.kt

https://android-review.googlesource.com/3071743

ol...@gmail.com <ol...@gmail.com> #10May 31, 2023 09:55AM

Project: platform/frameworks/support
Branch: androidx-main

commit d865a3700636d1536e8d0027fa6dbb2178c6f40a
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue Apr 30 11:15:03 2024

Add `KMP` as a build type in `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I3013b6b330ad16a1385c32c83928daa1d94ad959

M settings.gradle

https://android-review.googlesource.com/3070219

ae...@gmail.com <ae...@gmail.com> #11May 31, 2023 01:49PM

Project: platform/frameworks/support
Branch: androidx-main

commit 27b0ad3a572311c2663150708b4553e51ef3213a
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri May 03 15:26:51 2024

Rename `main` to `androidMain` in `savedstate`

Test: N/A
Bug: 334076622
Change-Id: Ib57446be87d946a0205f5b6f41f9bdb84a6b8d4d

M savedstate/savedstate/src/androidMain/AndroidManifest.xml
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/Recreator.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryController.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryOwner.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/View.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/ViewTreeSavedStateRegistryOwner.android.kt
M savedstate/savedstate/src/androidMain/res/values/ids.xml

https://android-review.googlesource.com/3071742

as...@gmail.com <as...@gmail.com> #12Jun 2, 2023 10:48PM

Project: platform/frameworks/support
Branch: androidx-main

commit 036f30202ee51da9fdd60951826bda440779dc81
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri May 03 15:25:56 2024

Configure multiplatform plugin in `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I48757dd18550ee4db98e11134cae75702b1d6f67

M docs-tip-of-tree/build.gradle
M savedstate/savedstate/build.gradle

https://android-review.googlesource.com/3070221

ch...@google.com <ch...@google.com> #13Jun 5, 2023 06:09PM

Project: platform/frameworks/support
Branch: androidx-main

commit 507de1178e29efb9e4fbfab001feacf6e1fa6711
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri May 03 15:27:19 2024

Rename `androidTest` to `androidInstrumentedTest` in `savedstate`

Test: N/A
Bug: 334076622
Change-Id: I39d9c83299ccabbc9f902d89d2ceaca0d76d28bb

M savedstate/savedstate/src/androidInstrumentedTest/AndroidManifest.xml
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/ErrorInStaticBlock.android.kt
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/SavedStateRegistryTest.android.kt
M savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/ViewTreeSavedStateRegistryOwnerTest.android.kt

https://android-review.googlesource.com/3071744

ap...@google.com <ap...@google.com> #14Jun 14, 2023 01:50AM

Project: platform/frameworks/support
Branch: androidx-main

commit f802d46e4221eb1a1833c4b0099891a5930d1ea6
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue May 07 14:55:20 2024

Introduce `SavedState` to abstract the data structure used to save and restore data in native platforms when using KMP.

* An opaque `SavedState` type that typealiases to a native type based on the platform.
* A `Reader` and `Writer` abstraction to modify the `SavedState` within Kotlin's common source sets. The abstraction utilizes a `value class` and all methods are `inline fun` to guarantee no overhead when using these methods to modify an `Bundle` on Android.

RelNote: "Introduce `SavedState` to abstract the data structure used to save and restore data in native platforms when using KMP. Includes a reader and writer used to mutate the `SavedState`."
Test: N/A
Bug: 334076622
Change-Id: I185754bb2e7e04c32ccc89209a4d4e765334e547

M savedstate/savedstate/api/current.txt
M savedstate/savedstate/api/restricted_current.txt
M savedstate/savedstate/build.gradle
A savedstate/savedstate/src/androidInstrumentedTest/kotlin/androidx/savedstate/RobolectricTest.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/Recreator.android.kt
A savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedState.android.kt
A savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryController.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryOwner.android.kt
A savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateWriter.android.kt
A savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/ParcelableSavedStateTest.android.kt
A savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/RobolectricTest.android.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedState.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateWriter.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/internal/SavedStateUtils.kt
A savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/RobolectricTest.kt
A savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateTest.kt
A savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedState.nonAndroid.kt
A savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
A savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateWriter.nonAndroid.kt
A savedstate/savedstate/src/nonAndroidTest/kotlin/androidx/savedstate/RobolectricTest.nonAndroid.kt

https://android-review.googlesource.com/3176247

ch...@google.com <ch...@google.com> #15Jun 16, 2023 05:13PM

Project: platform/frameworks/support
Branch: androidx-main

commit 8b2da06337524429901a0c8c3db28d2eb1b529f0
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Sep 13 11:26:35 2024

Add additional KMP targets to SavedState

RelNote: "SavedState, SavedStateWriter and SavedStateReader are now KMP compatible."
RelNote: "SavedState module now supports the following targets: android, desktop, ios, linux and mac."
Test: SavedStateRegistryTest
Bug: 334076622
Change-Id: I26305abd31dcbbf1c686e0bcfa9871b23cdd8545

A savedstate/savedstate/bcv/native/current.txt
M savedstate/savedstate/build.gradle
M savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt
M savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateWriter.kt
M savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
M savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateWriter.nonAndroid.kt

https://android-review.googlesource.com/3266971

ri...@ffw.com <ri...@ffw.com> #16Jun 28, 2023 01:54PM

Project: platform/frameworks/support
Branch: androidx-main

commit 904217890feef81667eb1ae57c2fd762ea9e24ee
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Sep 13 18:29:51 2024

Replace `SafeIterableMap` with `synchronized` + `mutableMap`

* `SafeIterableMap` class wasn't converted to KMP. Instead of converting, and to prevent `ConcurrentModificationException` during iteration, we replaced it with a standard `mutableMap` wrapped in a `synchronized` block. This ensures thread safety and allows for safe iteration.

Test: SavedStateRegistryTest
Bug: 334076622
Change-Id: Ie5ea66fa06fa6d2cd5e7abccc569db9a14908654

M savedstate/savedstate/build.gradle
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/internal/SynchronizedObject.kt
A savedstate/savedstate/src/darwinMain/kotlin/androidx/savedstate/internal/SynchronizedObject.darwin.kt
A savedstate/savedstate/src/jvmMain/kotlin/androidx/savedstate/internal/SynchronizedObject.jvm.kt
A savedstate/savedstate/src/linuxMain/kotlin/androidx/savedstate/internal/SynchronizedObject.linux.kt
A savedstate/savedstate/src/nativeMain/kotlin/androidx/savedstate/internal/SynchronizedObject.native.kt

https://android-review.googlesource.com/3267277

Message last modified on Jun 28, 2023 02:15PM

ch...@google.com <ch...@google.com> #17Jun 28, 2023 04:23PM

Project: platform/frameworks/support
Branch: androidx-main

commit 33d5114f54ef6377d5d47176581551453825134e
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Sep 13 22:33:32 2024

Use unique Lifecycle dependency version in SavedState

* The project included, by mistake, two different lifecycle artifacts: one in the commonMain source set and the other in the androidMain source set.

Test: SavedStateRegistryTest
Bug: 334076622
Change-Id: I33cb7caeda5a5b3b0ffd4bd96985ea6e60d46f93

M savedstate/savedstate/build.gradle

https://android-review.googlesource.com/3267333

ae...@gmail.com <ae...@gmail.com> #18Jun 28, 2023 09:38PM

Project: platform/frameworks/support
Branch: androidx-main

commit c5c7f78edc095ca6f789b455c6cb6eb61fd89905
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Mon Sep 16 16:32:56 2024

Disable experimental Android Lint K2 UAST

* The `useK2Uast` flag in Android Lint is causing `IllegalArgumentExceptionWithAttachments` errors when analyzing `SavedState` classes that have been converted to KMP.
* Instead of suppressing these warnings, the Android Lint team has recommended disabling the experimental K2 UAST integration entirely. This change disables the `useK2Uast` flag and adds a comment referencing the related bug (

b/345531033

) for future consideration.

Test: N/A
Bug: 334076622
Change-Id: Iba2ab0f64f7a5418e78f017a949de47d244cb161

M savedstate/savedstate/build.gradle

https://android-review.googlesource.com/3266975

ch...@google.com <ch...@google.com> #19Jun 28, 2023 09:55PM

Project: platform/frameworks/support
Branch: androidx-main

commit 1cc768070d5a75812e335581109a42ad9a7b9427
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Sep 13 20:29:18 2024

Extract common logic to `SavedStataRegistryImpl`

* By extracting a shared `SavedStateRegistryImpl`, that enable us to migrate `SavedStateRegistry` and `Controller` to `commonMain` with different `expect` and `actual` per platform, reducing duplication and the CL diff.

Test: SavedStateRegistryTest
Bug: 334076622
Change-Id: Id064dec964ee278438909f6099a16b158d9b769f

M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryController.android.kt
A savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/internal/SavedStateRegistryImpl.kt

https://android-review.googlesource.com/3267278

ch...@google.com <ch...@google.com> #20Jun 28, 2023 10:03PM

Project: platform/frameworks/support
Branch: androidx-main

commit 79a1767e2fe38493991d583c84b02c4524fc8e31
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Sep 13 20:34:36 2024

Move `SavedStateRegistry` and `Controller` to `commonMain`

RelNote: "SavedStateRegistry and SavedStateRegistryController are now KMP compatible."
Test: SavedStateRegistryTest
Bug: 334076622
Change-Id: Id7bb8064cc8bf19faade95636dd1436943056ce6

M savedstate/savedstate/bcv/native/current.txt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistry.android.kt
M savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateRegistryController.android.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateRegistry.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateRegistryController.kt
M savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateRegistryOwner.kt
M savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/internal/SavedStateRegistryImpl.kt
A savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateRegistry.nonAndroid.kt
A savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateRegistryController.nonAndroid.kt

https://android-review.googlesource.com/3267279

an...@google.com <an...@google.com> #21Jun 29, 2023 06:15PM

Project: platform/frameworks/support
Branch: androidx-main

commit 3a82f00a1255b352441328eb85c936e33ea56843
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Thu Sep 19 11:59:40 2024

Add `android` suffix to all files from `androidMain` in `viewmodel-savedstate`

Test: N/A
Bug: 334076622
Change-Id: Ia5911896f02a81c45c6e79b9a33da44bc937e417

M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/AbstractSavedStateViewModelFactory.android.kt
M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/LegacySavedStateHandleController.android.kt
M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/SavedStateHandle.android.kt
M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/SavedStateHandleController.android.kt
M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/SavedStateHandleSupport.android.kt
M lifecycle/lifecycle-viewmodel-savedstate/src/androidMain/kotlin/androidx/lifecycle/SavedStateViewModelFactory.android.kt

https://android-review.googlesource.com/3272055

al...@google.com <al...@google.com> Apr 17, 2024 04:02PM

Status: Won't Fix (Obsolete)