IssueTracker

Is this really a P1? This seems less priority than the perf work that I am doing but if it is a P1 I will block that and work on this.

I'm happy to move it to a p2, but it seems like a pretty big correctness issue that we should take a look at in the near future. I'm also happy to help look at this myself if you can give me some pointers on what the issue may be

This is not simple as and will change once Jim lands the threading of the composer.

Sure thing, thanks for the context!

For anyone running into this issue, in the meantime the simple workaround is just consuming the ambient inside the body of the function, you don't explicitly need to use the returned value anywhere.

Jim and I discussed this this morning and the reason it fails is because we skip the calls to composable methods at the call-site the call itself. We restart composition inside the callee. Kotlin lowers the adding of the default parameter into a synthetic method that calls the original method resulting in the ambient read sitting between skipping and restarting. Modifying the ambient restarts the caller's scope but that will then skip the call because the parameters haven't changed (from the callers perspective). If we invalidated the callee's scope composition would restart after inside the original method using the old value of the ambient because the synthetic that encodes the read is skipped.

After Jim's change, we already planned to move the skipping logic into the function being called. We also need to route the $static property (see Leland's ADS talk for a description of what $static is) or we will compare too many values and the slot table will grow (also described in Leland's talk). As part of that lowering we can take over the default value lowering from Kotlin and calculate both the $default and $static sets. This fixes this problem because this would cause the caller to be invalidated and re-execute the call, re-read the ambient, and then call the original function which will skip if the ambient is the same or execute if the ambient value is changed.

Next we plan to change how ambients work. The ambient values should be a map of ambient keys to State objects. Providing an ambient should create a new set with the provided value added. This would allow us to delete almost all of the of the special ambient logic in the composer which will also speed up both reading and updating ambients at the cost of additional overhead tracking the reads.

The combination of the above will cause this bug will go away. Fixing it before these changes would more difficult and would involve much of the work we are already planning.

However, we need a test case that ensures we indeed fix this case. Louis, it would be helpful if you would create a test case in our unit tests that reproduces this (FcsCodeGen would be a good place to add it)? We should add @Ignore to the new test (as it will fail) with a reference to this bug in the description.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ac4f6cda04a9607998e2bb8babba35c350a81d55
Author: Louis Pullen-Freilich <lpf@google.com>
Date:   Wed Oct 30 18:59:59 2019

    Test case for ensuring that ambient reads work inside default parameters
   
    This currently does not work, but this test case should be un-@Ignored
    once it is fixed.
   
    Removing the @Ignore and changing:
   
    `TextView(text = text, id = 42)`
   
    to
   
    `TextView(text = +ambient(TextAmbient), id = 42)`
   
    Makes the test pass.
   
    Bug: b/143464846
    Test: testAmbientConsumedFromDefaultParameter
    Change-Id: I8f8d069b39273dfaef40c6576b52ea426ef28b14

M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/FcsCodegenTests.kt

https://android-review.googlesource.com/1155144
https://goto.google.com/android-sha1/ac4f6cda04a9607998e2bb8babba35c350a81d55

Block

Assigning to me as a reminder that this can be marked as fixed once composer param work is done

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 797bd0cda4fd10a1d9d8b3b06b5954644a200334
Author: Leland Richardson <lelandr@google.com>
Date:   Thu May 07 15:16:51 2020

    Turn on Function-body-based code generation strategy
   
    Relnote: “
    Changes the code generation strategy of Compose’s compiler. Prior to the change, the compose compiler would transform calls to composable functions. With this change, we now transform the body of a composable function and leave the callsite unaltered (mostly).
   
    This means that most of the logic communicating with the compose runtime happens at the start of the function body, instead of at the callsite.
   
    This should be a source-compatible change for all usage of compose. Most users of compose should not have to update any code as a result of this change.
   
    In order to support this work, the JVM signature of all composable functions has changed. A Composable function accepting a single parameter is transformed into a function accepting 3 parameters, the additional parameters are the Composer, a ‘key’ integer. a bitmask integer used to propagate metadata through calls.
   
    Compose now also transforms default arguments to a composable function. It does this without introducing an additional synthetic default overload of the function itself, so this change will result in fewer functions being defined.
   
    Known intentional behavioral changes resulting from this:
    1. Some calls will skip where they wouldn’t have previously
    2. Composable expressions in default argument expressions are now correctly subscribed to and handled
   
    This work included some optimizations:
    1. The result of comparisons of parameters are propagated through the call graph to other composable functions. This will result in fewer comparisons at runtime, reduces the slot table size, as well as more skipping of composable functions that were previously not skipped
    2. Paremeters which are determined to be “static” at compile time are no longer compared or stored in the runtime. This reduces the number of comparisons and reduces slot table size.
    3. Control flow structure of the body of functions is used to minimize the number of groups that are generated. This reduces slot table size and results in less work for the runtime
    4. Unused dispatch and receiver parameters to functions are not included in determining skippability of the function if they are not used inside of the body of the function.
   
    Most breaking changes were for APIs that the compiler targets directly, and typical use of compose will not be affected:
    1. Composer::startExpr was removed
    2. Composer::endExpr was removed
    3. Composer::call was deprecated
    4. The non-varargs overloads of `key` have been removed. Use the `vararg` version going forward.
    5. The Pivotal annotation was deprecated. Use `key` as a replacement.
    6. ScopeUpdateScope::updateScope was changed to expect a Function3 instead of Function1
    7. restartableFunction and restartableFunctionN were updated to include additional compile time parameters
    “
   
    Bug: 144283149
    Bug: 144283416
    Bug: 144283245
    Bug: 150777987
    Bug: 143464846
   
    This effort has been a work in progress for some time, and much of the work has
    been done in the following previous commits:
   
    Make ComposableFunctionBodyTransformer work for any number of parameters
    Icc265a253c629f5533f1e68e5109db67234311f9
   
    Generate Movable groups with key and handle dynamic groups properly
    I5fe7799eb303dadbbdd47d71fe764767407c4ae2
   
    Function Body Skipping and Comparison Propagation
    I2ee971458ea466ba070dff25da5a4f60d42ff678
   
    Add Default Parameter IR Transform
    If530f61034b284bb3e1a8919e2b0be2511e2f243
   
    Handle Break/Continue in Control Flow Codegen
    I2c2c3261886ab8936baeb8a5b5e6b8f5e9e8d409
   
    Allow for early returns in control flow codegen
    If6446a90b7364a31ae67816595408c0aa971a39a
   
    Add ControlFlowTransformer WIP and tests
    I1f96ab3e11e3f26bf4f9e375e00420d25463510d
   
    Change-Id: I607560574d83b4b6c1e68ff72cc4124c5f8c2602

M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/AbstractIrTransformTest.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/AbstractLoweringTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/ComposeCallLoweringTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/ComposeMultiPlatformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/ComposerParamSignatureTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/ComposerParamTransformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/ControlFlowTransformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/DefaultParamTransformTests.kt
A       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/EmitTransformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/FcsCodegenTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/FcsModelCodeGenTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/FunctionBodySkippingTransformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/KtxCrossModuleTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/KtxModelCodeGenTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/KtxTransformationTest.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/RobolectricComposeTester.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/ComposeEmitResolver.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/ComposeFqNames.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/ComposeIrGenerationExtension.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/KtxNameConventions.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/AbstractComposeLowering.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposableCallTransformer.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposableFunctionBodyTransformer.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposableTypeRemapper.kt
D       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposeObservePatcher.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposerIntrinsicTransformer.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposerParamTransformer.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/IrSourcePrinter.kt
M       compose/compose-runtime/api/0.1.0-dev12.txt
M       compose/compose-runtime/api/current.txt
M       compose/compose-runtime/api/public_plus_experimental_0.1.0-dev12.txt
M       compose/compose-runtime/api/public_plus_experimental_current.txt
M       compose/compose-runtime/api/restricted_0.1.0-dev12.txt
M       compose/compose-runtime/api/restricted_current.txt
M       compose/compose-runtime/compose-runtime-benchmark/src/androidTest/java/androidx/compose/benchmark/siblings/SiblingManagement.kt
M       compose/compose-runtime/samples/src/main/java/androidx/compose/samples/PivotalSamples.kt
M       compose/compose-runtime/src/androidMain/kotlin/androidx/compose/KeySourceInfo.kt
M       compose/compose-runtime/src/commonMain/kotlin/androidx/compose/Composer.kt
M       compose/compose-runtime/src/commonMain/kotlin/androidx/compose/Key.kt
M       compose/compose-runtime/src/commonMain/kotlin/androidx/compose/Pivotal.kt
M       compose/compose-runtime/src/commonMain/kotlin/androidx/compose/SlotTable.kt
M       compose/compose-runtime/src/commonMain/kotlin/androidx/compose/internal/RestartableFunction.kt
M       compose/compose-runtime/src/jvmMain/kotlin/androidx/compose/internal/RestartableFunctionN.kt
M       ui/ui-core/src/androidTest/java/androidx/ui/core/AndroidPointerInputTest.kt
M       ui/ui-core/src/main/java/androidx/ui/core/ComposedModifier.kt
M       ui/ui-core/src/main/java/androidx/ui/core/focus/FocusModifierImpl.kt
M       ui/ui-core/src/main/java/androidx/ui/res/FontResources.kt
M       ui/ui-core/src/test/java/androidx/ui/core/ComposedModifierTest.kt
M       ui/ui-tooling/src/androidTest/java/androidx/ui/tooling/BoundsTest.kt
M       ui/ui-tooling/src/androidTest/java/androidx/ui/tooling/InspectableTests.kt
M       ui/ui-tooling/src/androidTest/java/androidx/ui/tooling/ModifierInfoTest.kt
M       ui/ui-tooling/src/main/java/androidx/ui/tooling/preview/PreviewUtils.kt

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

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 458aaff814870edec3837f3ebe343f197a198697
Author: Leland Richardson <lelandr@google.com>
Date:   Mon Jun 08 15:00:32 2020

    Prevent non-static default expressions from being treated as static
   
    Fixes: 143464846
    Change-Id: I26c8a7f54581c38aede45c5db2f2a2a4ee2e39df

M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/DefaultParamTransformTests.kt
M       compose/compose-compiler-hosted/integration-tests/src/test/java/androidx/compose/plugins/kotlin/FunctionBodySkippingTransformTests.kt
M       compose/compose-compiler-hosted/src/main/java/androidx/compose/plugins/kotlin/compiler/lower/ComposableFunctionBodyTransformer.kt

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

Im not sure but perhaps the problem is a composable can have lambda functions, and in some cases this could leak the activity if the expression mamipulates members data, because it holds a reference to it

See this link
https://stackoverflow.com/questions/42271208/kotlin-safe-lambdas-no-memory-leak

After updating librarie to  compose_version = '1.2.0'.  Activities are not leaked , but still have composable objects that are keeping in memory and confuse me

This is my sample: A Simple MainTimer1Activity and a couple of seconds go to MainTimer2Activity

private const val TAG = "MAIN_TIMER_1"
class MainTimer1Activity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContent {
            MyApplicationTheme {
                MainTimer1Screen()
            }
        }
    }
}

@Composable
fun MainTimer1Screen() {
    Log.e(TAG, "MainTimer1Screen ...")

    val ctx = LocalContext.current
    LaunchedEffect(key1 = Unit, block = {
        try {
            startTimer1(5000L) { // start a timer for 5 secs
                Log.e(TAG, "Timer ended")
                toTimer2Screen(ctx)
            }
        } catch(ex: Exception) {
            Log.e(TAG,"timer cancelled")
        }
    })
}

suspend fun startTimer1(time: Long, onTimerEnd: () -> Unit) {
    delay(timeMillis = time)
    onTimerEnd()
}

fun toTimer2Screen(ctx: Context){
    val act = ctx.findActivity() as MainTimer1Activity

    val intent  = Intent(ctx, MainTimer2Activity::class.java)
    ctx.startActivity(intent)
    act.finish()
}


---------

private const val TAG = "MAIN_TIMER_2"
class MainTimer2Activity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContent {
            MyApplicationTheme {
                MainTimer2Screen()
            }
        }
    }
}

@Composable
fun MainTimer2Screen() {
    Log.e(TAG, "MainTimer2Screen ...")
    val ctx = LocalContext.current
  }

---------
The log ourput as espected
MainTimer1Screen ...
Timer ended
MainTimer2Screen ...


So if i look to profiler. and force GC the activitie MainTimer1Activity is removed but still have info about MainTimer1Screen Composable. i dont know if that belong to Composable tree taht never released

in first Capture you see  MainTimer1 and Maintimer2 Activities
in Second Capture   After force GC many time in 7 minutes.  Maintimer2Activity is released. but still there this objects:  

ComposableSingletons$Maintiner1Activitykt$lambda-1$1
ComposableSingletons$Maintiner1Activitykt$lambda-2$1

I think this composable are leaked, is strange or there is always info about compsables tree that  android never releases

Sory i try to said in Second Capture   After force GC many times in 7 minutes.  Maintimer1Activity is released. but still there this objects:  about  Activity1 composables. (inner instance that are not released)

This is a **major** issue, open for months and still not started. Can we have an update please? The reason this doesn't have enough upvotes is because unfortunately not a lot of devs investigate memory leaks.

It is mind blowing to see that nothing was done or even communicated for so long. Issue is incredibly major and blocks ALL products from migrating to compose framework

This issue is drastically underestimated, probably also due to not expressive title. It's not ComposeView problem. We don't even need any long running task.  It looks like Composer problem (maybe something deep in compose internals). In my opinion most of the Compose applications, which are not single-activity are impacted by this.

Any mutable state change during composition hiding is enough for replication. The impact is drastic: all subsequent compose activities are stucking in the memory, and just piling up.

still not fixed?

Perhaps. is not compose problem, and the problem is with lambda  instances  because that are  the objects retained in the profiler- Perhasps lambda notation leaks memory. But im not no sure. , or is a combination of compose with lambda notation. The problem still exists . With this issue i prefer use old way xml layouts,  and avoid to migrate to compose.


See $lambda-1$1 in the profiler

ComposableSingletons$Maintiner1Activitykt$lambda-1$1
ComposableSingletons$Maintiner1Activitykt$lambda-2$1

I made a minimal reproduction of #23. https://github.com/zmunm/compose-leak-sample

The coil is holding the drawable as state and this overlapped and caused the oom.
I made a PR to address this. https://github.com/coil-kt/coil/pull/1675#issuecomment-1484923260

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

commit 66fef38b1d11e0c48b11137e6c3d007909f4a2d1
Author: Chuck Jazdzewski <chuckj@google.com>
Date:   Wed Mar 29 13:51:01 2023

    ON_STOP should pause the frame clock broadcasts instead of composition
   
    When an Android window receives an ON_STOP noficiation it would pause
    the pausable clock it created for its recomposer. This has the effect
    of pausing all compositions and causes the recomposer to collect change
    nofications from the snapshot sustem in an unbounded collection.
   
    With this change, windows that receive ON_STOP will now only pause the
    frame clock broadcasts which has the effect of blocking withFrameNanos
    that is used by animations. This means animations stop advancing but
    composition is allowed to continue allowing the recomposer to drain the
    notifications from the snapshot system.
   
    Relnote: """The recomposer created for an Android window will now
    only block calls to `withFrameNanos` instead of all composition when it
    receives an ON_STOP notification. This means windows associated with
    stopped activites will continue to recompose for data changes but the
    animations, or any other caller of withFrameNanos, will block."""
   
    Fixes: 240975572
    Test: ./gradlew :compose:r:r:tDUT
    Change-Id: Id9e7fe262710544a48c2e4fc5fcbf1d27bfaa1ba

M       compose/runtime/runtime/api/current.txt
M       compose/runtime/runtime/api/public_plus_experimental_current.txt
M       compose/runtime/runtime/api/restricted_current.txt
M       compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
M       compose/runtime/runtime/src/commonTest/kotlin/androidx/compose/runtime/RecomposerTests.kt
M       compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/WindowRecomposer.android.kt

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

Does anyone have an estimation when the fix will be available in production?
This leak is happening for us on every screen migrated to Compose and would like to check any alpha version for the fix.

I already tested with

androidx.compose.runtime:runtime:1.5.0-alpha03
androidx.compose.ui:ui:1.5.0-alpha03

But the problems still exists

ComposableSingletons$Activitykt$lambda-1$1. never are released . The Activity is released. but the composables are always keep it in memory, so its seems to be a kind of memory leak

As a conclusion: ComposableSingletons are part of composable and will never release during all tha app. So its part of the framework itself

Is it still not resolved?

It is resolved here: https://android-developers.googleblog.com/2023/08/whats-new-in-jetpack-compose-august-23-release.html

I have upgraded to the version mentioned in the article but the issue still persists. Could someone confirm if the issue is fixed.

But  you have to be very carefully with Singleton in Android. As you remark singleton are allocated once , but it not leaved untill app leaves, so supose you have old navigation with many activities , this ComposableSingleton   are allocated and never released untill you leave the app. I think if its part of composable framework this is dangerous  because they are keeping memory releated to an activity that is eventually finished

With latest version 'androidx.activity:activity-compose:1.8.2'  the leacked acivity has gonne, but still have instance of  

ComposableSingletons$Activitykt$lambda-1$1. that keep in memory

I think is part of the composable framework