Memory leak when dismissing a bottom sheet

Description:

When a DialogFragment gets dismissed, leak canary reports a memory leak caused by OneSignal. Note that I have no reference to the onesignal SDK in that dialog fragment.

Environment

Version 3.14.1 (gradle)

Steps to Reproduce Issue:

1. add onesignal to your project
2. open a dialog fragment (I could repro with a bottom sheet fragment)
3. dismiss it

Anything else:

The SDK is initialised with those lines:

OneSignal.startInit(this).autoPromptLocation(false)
        .setNotificationOpenedHandler(this).init()
OneSignal.setInFocusDisplaying(OneSignal.OSInFocusDisplayOption.Notification)

Memory leak log:

┬───
│ GC Root: Java local variable
│
├─ com.onesignal.UserStateSynchronizer$NetworkHandlerThread thread
│    Leaking: UNKNOWN
│    Thread name: 'OSH_NetworkHandlerThread'
│    ↓ UserStateSynchronizer$NetworkHandlerThread.<Java Local>
│                                                 ~~~~~~~~~~~~
├─ android.os.Message instance
│    Leaking: UNKNOWN
│    ↓ Message.obj
│              ~~~
├─ androidx.fragment.app.DialogFragment$2 instance
│    Leaking: UNKNOWN
│    Anonymous class implementing android.content.DialogInterface$OnCancelListener
│    ↓ DialogFragment$2.this$0
│                       ~~~~~~
╰→ com.example.MyBottomSheetFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.example.MyBottomSheetFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     key = 378d77ff-e2a3-4019-99ed-d6f2eda05699
​     watchDurationMillis = 7654
​     retainedDurationMillis = 2653

How to solve it according to Leak canary:

@romainpiel com.onesignal.UserStateSynchronizer$NetworkHandlerThread is designed to live the life time with the app process since it is a HandlerThread. I can accept update Runnables at anytime.

├─ com.onesignal.UserStateSynchronizer$NetworkHandlerThread thread
│    Leaking: UNKNOWN
│    Thread name: 'OSH_NetworkHandlerThread'
│    ↓ UserStateSynchronizer$NetworkHandlerThread.<Java Local>
│                                                 ~~~~~~~~~~~~

For the fragment, I don't see any references to OneSignal here. Can you provide more detail on how it is linked to OneSignal?

╰→ com.example.MyBottomSheetFragment instance
​     Leaking: YES (ObjectWatcher was watching this because com.example.MyBottomSheetFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     key = 378d77ff-e2a3-4019-99ed-d6f2eda05699
​     watchDurationMillis = 7654
​     retainedDurationMillis = 2653

That's the weird thing, there is no reference to One signal in that bottom sheet. Our codebase has only references to one signal in two places:

• The application object: startInit and setInFocusDisplaying(OneSignal.OSInFocusDisplayOption.Notification)
• A service that extends NotificationExtenderService which reacts to the intent filter com.onesignal.NotificationExtender

@romainpiel I see, thanks for confirming.

I just read through Leak Canary's Fixing a memory leak documentation which cleared up some wrong assumptions I was making.

This leak looks like it could be from not handling onFragmentDestroyed in this method. https://github.com/OneSignal/OneSignal-Android-SDK/blob/3.15.1/OneSignalSDK/onesignal/src/main/java/com/onesignal/OSSystemConditionController.java#L50-L78

Looking at the Android Fragment lifecycle it seems onDestroy fires before onDetech so Leak Canary is detecting this as a leak since the lifecycle callback is not cleaned up by then. https://developer.android.com/guide/components/fragments#Creating So this may just be a case of a delayed clean up instead of a leak. We will investigate further to confirm.