google-pay / compose-pay-button

An Android library that provides a Jetpack Compose wrapper on top of the Google Pay Button API.
Apache License 2.0
29 stars 2 forks source link

Memory leak in `PayButton` #2

Closed tillh-stripe closed 3 months ago

tillh-stripe commented 1 year ago

Hi there 👋

We’re seeing a memory leak seemingly originating from PayButton where the current activity’s context is being held on to.

Here’s a repository with a sample project to reproduce the issue. Simply open the details activity by pressing Open details and then close it again. The LeakCanary notification will show shortly after.

dmengelt commented 1 year ago

thanks @tillh-stripe for reporting this and even create a reproducer. we will look right into it!

dmengelt commented 1 year ago

@tillh-stripe we fixed the memory leak. Starting with Google Play Services > 23.39 users should no longer see the memory leak. Could you try to update (if not automatically updated already) your device or emulator and check if the memory leak is gone for you?

tillh-stripe commented 1 year ago

Seems resolved. Thanks for your help 🙏

cmargonis commented 6 months ago

Hello @dmengelt we're encountering possibly the same memory leak (using Google Play Services 24.15.18).

I cloned @tillh-stripe 's repository, updated the dependencies/tools to the latest versions and the memory leak exists.

dmengelt commented 6 months ago

@cmargonis sorry for the late reply. are you still able to reproduce this?

cmargonis commented 6 months ago

Hello @dmengelt , yeap, using the original repository provided above (after having updated every dependency to the latest stable versions). Shall I try again (as the last time I checked was over 1.5 week ago)?

samer-stripe commented 5 months ago

Still happening for us as well. Leak trace looks very general but replacing PayButton with a general compose Button fixed the leak so it looks like it is still coming from PayButton

leakcanary.NoLeakAssertionFailedError: Application memory leaks were detected:
====================================
HEAP ANALYSIS RESULT
====================================
1 APPLICATION LEAKS
References underlined with "~~~" are likely causes.
Learn more at https://squ.re/leaks.
2229536 bytes retained by leaking objects
Signature: db50881b4ef26977ce94c6b892ac07b329c1a085
┬───
│ GC Root: System class
│
├─ android.view.LayoutInflater class
│    Leaking: NO (AppCompatTextView↓ is not leaking and a class is never leaking)
│    ↓ static LayoutInflater.sConstructorMap
├─ java.util.HashMap instance
│    Leaking: NO (AppCompatTextView↓ is not leaking)
│    ↓ HashMap["android.support.v7.widget.AppCompatTextView"]
├─ java.lang.reflect.Constructor instance
│    Leaking: NO (AppCompatTextView↓ is not leaking)
│    ↓ Executable.declaringClass
├─ android.support.v7.widget.AppCompatTextView class
│    Leaking: NO (DelegateLastClassLoader↓ is not leaking and a class is never leaking)
│    ↓ static AppCompatTextView.$class$classLoader
├─ dalvik.system.DelegateLastClassLoader instance
│    Leaking: NO (bdn↓ is not leaking and A ClassLoader is never leaking)
│    ↓ ClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│    Leaking: NO (bdn↓ is not leaking)
│    ↓ Object[534]
├─ m.bdn class
│    Leaking: NO (a class is never leaking)
│    ↓ static bdn.a
│                 ~
├─ m.hg instance
│    Leaking: UNKNOWN
│    Retaining 2.5 MB in 57876 objects
│    ↓ hg.a
│         ~
├─ m.hm instance
│    Leaking: UNKNOWN
│    Retaining 2.5 MB in 57874 objects
│    ↓ hm.a
│         ~
├─ java.util.LinkedHashMap instance
│    Leaking: UNKNOWN
│    Retaining 2.5 MB in 57873 objects
│    ↓ LinkedHashMap["6c40af7e-209e-4fcd-8333-123539826d35"]
│                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ m.bdl instance
│    Leaking: UNKNOWN
│    Retaining 24.4 kB in 256 objects
│    c instance of com.stripe.android.paymentsheet.example.ExampleApplication
│    ↓ bdl.f
│          ~
├─ m.bdp instance
│    Leaking: UNKNOWN
│    Retaining 22.9 kB in 231 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    a instance of com.stripe.android.paymentsheet.PaymentSheetActivity with mDestroyed = true
│    mContext instance of android.view.ContextThemeWrapper
│    ↓ bdp.a
│          ~
╰→ com.stripe.android.paymentsheet.PaymentSheetActivity instance
     Leaking: YES (ObjectWatcher was watching this because com.stripe.android.paymentsheet.PaymentSheetActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
     Retaining 2.2 MB in 56804 objects
     key = 5edd3157-3e63-47fd-a18a-4e50c0614bc4
     watchDurationMillis = 5636
     retainedDurationMillis = 635
     mApplication instance of com.stripe.android.paymentsheet.example.ExampleApplication
     mBase instance of androidx.appcompat.view.ContextThemeWrapper
====================================
0 LIBRARY LEAKS
A Library Leak is a leak caused by a known bug in 3rd party code that you do not have control over.
See https://square.github.io/leakcanary/fundamentals-how-leakcanary-works/#4-categorizing-leaks
====================================
0 UNREACHABLE OBJECTS
An unreachable object is still in memory but LeakCanary could not find a strong reference path
from GC roots.
====================================
METADATA
dmengelt commented 5 months ago

@cmargonis / @samer-stripe yes. we can reproduce the issue as well. most likely it happens because the fix was rolled out behind an experiment flag and for some users this flag has a stale value. We will cleanup the experiment and this should resolve the issue. Sorry for the inconvenience.

dmengelt commented 5 months ago

We confirmed that the leak happens because of a stale experiment value. I cleaned up the experiment and submitted the change. It will be released with one of the upcoming Play Services versions.

The leak only happens for some users the first time the App is started. The second time the updated flags are loaded and the leak goes away.

@samer-stripe I will let you know when you can re-try running your Google Pay tests.

samer-stripe commented 5 months ago

Perfect, thank you so much @dmengelt

dmengelt commented 3 months ago

@samer-stripe / @cmargonis could you check this again? Make sure you are running a Google Play Services version equal or higher as 24.26

Google Play Services version: Settings -> Apps -> "See all XXX Apps" > Click the search icon top right > Search for "Google Play Services"

samer-stripe commented 3 months ago

Looks like the leak is fixed, thank you so much! @dmengelt