JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.24k stars 1.11k forks source link

`BasicAlertDialog` Crash iOS with OOM (Memory Leak) #4852

Open chrisjenx opened 1 month ago

chrisjenx commented 1 month ago

Describe the bug Since 1.6.10x, m3 BasicAlertDialogs with lots of content will recompose and leak memory causing out of memory crashes on iOS.

iOS will throw didReceiveMemoryWarning then crash:

Screenshot 2024-05-22 at 5 38 33 PM

More diagnostics to follow once got a sample project for you.

Affected platforms

Versions

To Reproduce

More to follow, but full screen dialogs will recompose unconditionally and crash (will try and create a reproduction project for you)

Steps to reproduce the behavior:

  1. Open Dialog with scrollable content
  2. Scroll Content very slowly, app will crash after it runs out of memory drawing

Expected behavior Don't OOM and crash on iOS

Screenshots Screenshot 2024-05-22 at 5 38 33 PM

Additional context I thought it was related to Kotlin 2.x but downgraded and still an issue, downgraded CMP to 1.6.1 and now not crashing.

Link to trace from Profling iOS App, looks like a Skia Canvas draw leak in 1.6.10. If you stop moving it will dealloc the objects, but scrolling slowly will not release quick enough.

https://drive.google.com/drive/folders/1UUPOyAWaeg7uTKaXc5EMCp-03jSy4hrk?usp=sharing

mazunin-v-jb commented 1 month ago

Hello! I'll notify the team about that, however, a small reproducible example would really help. Could you please attach it once you've made it to your first message?

chrisjenx commented 1 month ago

Yeah we're working on deploying a hot fix to users once that's done I'll create a sample for y'all

chrisjenx commented 1 month ago

Added Trace, easy to reproduce, any dialog with scrollable content, scroll SLOWLY and you'll eventually run out of memory

mazunin-v-jb commented 1 month ago

Unfortunately, I wasn't able to successfully reproduce this crash. Memory usage on a small project from template was about 70 mb and didn't increase no matter how I scrolled. Could you please make a reproducible example?

chrisjenx commented 1 month ago

Unfortunately, I wasn't able to successfully reproduce this crash. Memory usage on a small project from template was about 70 mb and didn't increase no matter how I scrolled. Could you please make a reproducible example?

Dang, OK, will try and create a sample project. Sorry about that

chrisjenx commented 1 month ago

Unfortunately, I wasn't able to successfully reproduce this crash. Memory usage on a small project from template was about 70 mb and didn't increase no matter how I scrolled. Could you please make a reproducible example?

Oh just checking this ONLY happens on a physical device, simulators are fine (faster/more ram from host machine)

chrisjenx commented 1 month ago

@mazunin-v-jb As requested: DialogCrash.zip

Launch app on physical iPhone, open dialog, scroll slowly (will be jerky vs previous versions) and then will suddenly crash.

I should have noted it was the BasicAlertDialogs from the m3 lib

chrisjenx commented 1 month ago

Nvm mind, I tested on the foundation Dialog and the same thing, guessing it's an underlying skia issue from what I can tell.

mazunin-v-jb commented 1 month ago

@chrisjenx many thanks! I haven't reproduced the crash, but the memory warning was received many times, and memory consumption is very weird. We'll look at it

chrisjenx commented 1 month ago

Oh good, maybe a placebo, but 1.6.10 doesn't feel as smooth as 1.6.0 potentially related if it's over drawing, leaking, etc

chrisjenx commented 3 weeks ago

@mazunin-v-jb I will give 1.6.11 ago, I'm wondering if this was the cause of the OOM https://github.com/JetBrains/compose-multiplatform-core/pull/1355

chrisjenx commented 3 weeks ago

Tested 1.6.11, still crashing for me. Looks like it's something else :(

(It's actually worse after some more testing)