OneSignal-Android-SDK
OneSignal-Android-SDK copied to clipboard
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:
- add onesignal to your project
- open a dialog fragment (I could repro with a bottom sheet fragment)
- 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
andsetInFocusDisplaying(OneSignal.OSInFocusDisplayOption.Notification)
- A service that extends
NotificationExtenderService
which reacts to the intent filtercom.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.