android / play-billing-samples

Samples for Google Play In-app Billing
Apache License 2.0
2.38k stars 1.34k forks source link

BillingBroadcastReceiver Memory Leak #185

Open yusufceylan opened 5 years ago

yusufceylan commented 5 years ago

After buying a item successfully, I am closing the current activity which holds purching process. But Leak Canary catch a memory leak about BillingBroadcastReceiver. I init billing client OnCreate and release onDestroy.

Here is my init method

billingClient = BillingClient.newBuilder(this).setListener(this).build();
        billingClient.startConnection(new BillingClientStateListener() {
            @Override
            public void onBillingSetupFinished(int responseCode) {

                if (responseCode == BillingClient.BillingResponse.OK) {
                    // The billing client is ready. You can query purchases here.
                    loadProducts();
                } else {
                    // Error
                }

            }

            @Override
            public void onBillingServiceDisconnected() {
                // Try to restart the connection on the next request to
                Timber.d("Connection Error");
            }
        });

Here is my release Method

        if (billingClient != null && billingClient.isReady()) {
            billingClient.endConnection();
            billingClient = null;
        }

OnPurchaseUpdated I made a service call and close this activity based on service result.

    public void onPurchasesUpdated(int responseCode, @Nullable List<Purchase> purchases) {

        if (responseCode == BillingClient.BillingResponse.OK && purchases != null) { 
            for (Purchase purchase : purchases) {
                billingClient.consumeAsync(purchase.getPurchaseToken(), new ConsumeResponseListener() {
                    @Override
                    public void onConsumeResponse(int responseCode, String purchaseToken) {
                        if (responseCode == BillingClient.BillingResponse.OK && purchaseToken != null) {
                            Timber.d("onConsumeResponse --> %s", purchaseToken);
                            getViewModel().informPurchase(necessary data);
                        }
                    }
                });
            }
        } else if (responseCode == BillingClient.BillingResponse.USER_CANCELED) {
            // Handle an error caused by a user canceling the purchase flow.
            Timber.d("Billing Cancelled");

        } else {
            Timber.d("An Error Occured");
        }
    }

Here is the Leak Canary error

BroadcastError

CycleSkip commented 5 years ago

You don't need to check isReady() before calling endConnection(). Otherwise, the serviceConnection could still live (even if it is not ready) without being ended.

if (billingClient != null) {
    billingClient.endConnection();
    billingClient = null;
}
yusufceylan commented 5 years ago

@CycleSkip Still same error happens :(

isaidamier commented 5 years ago

Might you be able to follow the TrivialDriveKotlin sample so as to decouple your billing implementation from your activity lifecycle?

leonrenner commented 4 years ago

@yusufceylan I am having the same error today. Did you find a solution?

yusufceylan commented 4 years ago

@leonrenner unfortunately not :(

ogrenichv commented 4 years ago

Having the same problem. I was able to reproduce it by changing locale in phone i.e. start application, go to phone settings, change locale, open application from recent apps. It recreates activity but it seems that old activity is leaking.

atulgpt commented 4 years ago

Is there any update for this? It is also causing memory leak in my case

HRankit commented 4 years ago

Same problem with me too. Can reproduce it by toggling between dark mode and light mode.

jcarter326613 commented 4 years ago

Try changing BillingClient.newBuilder(this) to BillingClient.newBuilder(this.applicationContext)

CycleSkip commented 4 years ago

Try changing BillingClient.newBuilder(this) to BillingClient.newBuilder(this.applicationContext)

Why would this help?

dp-singh commented 3 years ago

This is still happening, linking the Stack Overflow issue. https://stackoverflow.com/questions/65180072/android-billing-client-causes-memory-leak

haris15 commented 3 years ago

@yusufceylan have you found any solution ? if yes then help me

Qing451800 commented 3 years ago

I believe we found the issue: PBL is holding an activity context and didn't free it. The issue has been fixed. It shall be rolled out with the next PBL release.

On Thu, Jan 7, 2021 at 2:29 AM haris15 notifications@github.com wrote:

@yusufceylan https://github.com/yusufceylan have you found any solution ? if yes then help me

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/android/play-billing-samples/issues/185#issuecomment-756030039, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOOVKZHP373H36O6C7ZAC2LSYWEJ7ANCNFSM4G6YKTAQ .

SMehranB commented 3 years ago

Almost February, 2021 and this problem still exists. When is the new version going to be released? @Qing451800

mariusmorabosch commented 3 years ago

Hey! This still seems to be an issue for me even on 4.0.0 - are you completely sure this has been fixed? I see other reports of it not being fixed for 3.0.3 either (https://stackoverflow.com/questions/67025367/google-play-billing-library-3-0-3-causing-memory-leak)

EmilAlipiev commented 2 years ago

it is definetly not fixed. it is still happening with 4.0.0

atulgpt commented 2 years ago

@Qing451800 It's been almost one year. Did you able to release the new library with the memory fix in that?

ytheekshana commented 2 years ago

I also have the same issue.

aloz77 commented 2 years ago

With billingClient 4.1.0 still have a similar leak. Looks like billingClient.endConnection() doesn't destroy some Handler with a reference to some listener

    ┬───
    │ GC Root: System class
    │
    ├─ android.app.ActivityThread class
    │    Leaking: NO (MessageQueue↓ is not leaking and a class is never leaking)
    │    ↓ static ActivityThread.sMainThreadHandler
    ├─ android.app.ActivityThread$H instance
    │    Leaking: NO (MessageQueue↓ is not leaking)
    │    ↓ Handler.mQueue
    ├─ android.os.MessageQueue instance
    │    Leaking: NO (MessageQueue#mQuitting is false)
    │    HandlerThread: "main"
    │    ↓ MessageQueue[2]
    │                  ~~~
    ├─ android.os.Message instance
    │    Leaking: UNKNOWN
    │    Retaining 120 B in 4 objects
    │    Message.what = 0
    │    Message.when = 23510943 (16342 ms after heap dump)
    │    Message.obj = null
    │    Message.callback = instance @317222688 of com.android.billingclient.api.zzz
    │    Message.target = instance @317222768 of android.os.Handler
    │    ↓ Message.callback
    │              ~~~~~~~~
    ├─ com.android.billingclient.api.zzz instance
    │    Leaking: UNKNOWN
    │    Retaining 56 B in 3 objects
    │    ↓ zzz.zzb
    │          ~~~
    ├─ com.android.billingclient.api.zzy instance
    │    Leaking: UNKNOWN
    │    Retaining 12 B in 1 objects
    │    ↓ zzy.zza
    │          ~~~
    ├─ com.my.app.OrderFragment$1 instance
    │    Leaking: UNKNOWN
    │    Retaining 12 B in 1 objects
    │    Anonymous class implementing com.android.billingclient.api.SkuDetailsResponseListener
    │    ↓ OrderFragment$1.this$0
    │                      ~~~~~~
    ╰→ com.my.app.OrderFragment instance
    ​     Leaking: YES (ObjectWatcher was watching this because com.my.app.OrderFragment received Fragment#onDestroy()
    ​     callback and Fragment#mFragmentManager is null)
    ​     Retaining 2,5 MB in 14148 objects
    ​     key = f04ce3ba-e9b2-470d-bfca-ad05e6f57e98
    ​     watchDurationMillis = 7928
    ​     retainedDurationMillis = 2928
    ​     mainActivity instance of com.my.app.MainActivity with mDestroyed = true
    ====================================
    0 LIBRARY LEAKS
j4zib commented 2 years ago

Any updates on this issue?

aapunain commented 1 year ago

I used below code and leak removed:

private static class BillingClientCallbacksWrapper implements BillingClientStateListener, PurchasesUpdatedListener {
    @Nullable
    private BillingClientStateListener billingClientStateListener;
    @Nullable
    PurchasesUpdatedListener purchasesUpdatedListener;

    public BillingClientCallbacksWrapper(@Nullable BillingClientStateListener billingClientStateListener,
                                         @Nullable PurchasesUpdatedListener purchasesUpdatedListener) {
        this.billingClientStateListener = billingClientStateListener;
        this.purchasesUpdatedListener = purchasesUpdatedListener;
    }

    public void endConnection() {
        billingClientStateListener = null;
        purchasesUpdatedListener = null;
    }

    @Override
    public void onBillingServiceDisconnected() {
        if (billingClientStateListener != null) {
            billingClientStateListener.onBillingServiceDisconnected();
        }
    }

    @Override
    public void onBillingSetupFinished(@NonNull BillingResult billingResult) {
        if (billingClientStateListener != null) {
            billingClientStateListener.onBillingSetupFinished(billingResult);
        }
    }

    @Override
    public void onPurchasesUpdated(@NonNull BillingResult billingResult, @Nullable List<Purchase> list) {
        if (purchasesUpdatedListener != null) {
            purchasesUpdatedListener.onPurchasesUpdated(billingResult, list);
        }
    }
}

use BillingClientCallbacksWrapper at the place of BillingClientStateListener and PurchasesUpdatedListener 
and call BillingClientCallbacksWrapper#endConnection at the time of BillingClient#endConnection.

This Wrapper will prevent leak of your whole Activity/Broadcast..

aloz77 commented 1 year ago

@aapunain: It doesn't fix the issue for me with billing:5.0.0. Not sure how nulling the references TO the listeners should fix the issue if the handlers IN the listeners are holding reference TO the context and are not cancelled/destroyed properly.

aapunain commented 1 year ago

@aapunain: It doesn't fix the issue for me with billing:5.0.0. Not sure how nulling the references TO the listeners should fix the issue if the handlers IN the listeners are holding reference TO the context and are not cancelled/destroyed properly.

1. if you are passing context then pass applicationContext

2. as I saw your comment in your case com.android.billingclient.api.SkuDetailsResponseListener is leaking so use wrapper around that also.

3. "Not sure how nulling the references TO the listeners should fix the issue" --> by nulling it will just leak the wrapper object not the whole activity/fragment/broadcast (so leak will have very minimal impact).

marticztn commented 10 months ago

It's Oct 13, 2023 and the issue is still there

chaoshengyuan commented 1 month ago

It's July 11, 2024 and the issue is still there

chaoshengyuan commented 1 month ago

┬─── │ GC Root: System class │ ├─ android.app.ActivityThread class │ Leaking: NO (MessageQueue↓ is not leaking and a class is never leaking) │ ↓ static ActivityThread.sMainThreadHandler ├─ android.app.ActivityThread$H instance │ Leaking: NO (MessageQueue↓ is not leaking) │ ↓ Handler.mQueue ├─ android.os.MessageQueue instance │ Leaking: NO (MessageQueue#mQuitting is false) │ HandlerThread: "main" │ ↓ MessageQueue[16] │ ~~~~ ├─ android.os.Message instance │ Leaking: UNKNOWN │ Retaining 3.2 MB in 12225 objects │ Message.what = 0 │ Message.when = 103777760 (19551 ms after heap dump) │ Message.obj = null │ Message.callback = instance @335324064 of com.android.billingclient.api.zzw │ Message.target = instance @335324000 of android.os.Handler │ ↓ Message.callback │ ~~~~ ├─ com.android.billingclient.api.zzw instance │ Leaking: UNKNOWN │ Retaining 3.2 MB in 12223 objects │ ↓ zzw.zzb │ ~~~ ├─ com.android.billingclient.api.zzl instance │ Leaking: UNKNOWN │ Retaining 3.2 MB in 12221 objects │ ↓ zzl.zzb │ ~~~ ├─ com.topstories.android.app.pay.GooglePay$$ExternalSyntheticLambda3 instance │ Leaking: UNKNOWN │ Retaining 3.2 MB in 12220 objects │ f$1 instance of com.topstories.android.app.person.premium.PremiumActivity │ with mDestroyed = true │ ↓ GooglePay$$ExternalSyntheticLambda3.f$1 │ ~~~ ╰→ com.topstories.android.app.person.premium.PremiumActivity instance ​ Leaking: YES (ObjectWatcher was watching this because com.topstories. ​ android.app.person.premium.PremiumActivity received Activity#onDestroy() ​ callback and Activity#mDestroyed is true) ​ Retaining 3.2 MB in 12219 objects ​ key = 8901f54e-9d00-4d3b-8992-9491aa505c2c ​ watchDurationMillis = 5594 ​ retainedDurationMillis = 593 ​ mApplication instance of com.topstories.android.app.MyApp ​ mBase instance of androidx.appcompat.view.ContextThemeWrapper