1978626 - [composable toolbar] Memory leak in tab counter

Status: RESOLVED FIXED

(Firefox for Android :: Toolbar, defect, P2)

Description

[Petru-Mugurel Lingurar [:petru]]

Seen in a recent try run while the navbar was being enabled even without the composable toolbar and it would setup the default composable toolbar configuration.

┬───
│ GC Root: System class
│
├─ android.view.inputmethod.InputMethodManager class
│ Leaking: NO (InputMethodManager↓ is not leaking and a class is never leaking)
│ ↓ static InputMethodManager.sInstance
├─ android.view.inputmethod.InputMethodManager instance
│ Leaking: NO (ConstraintLayout↓ is not leaking and InputMethodManager is a singleton)
│ ↓ InputMethodManager.mNextServedView
├─ androidx.constraintlayout.widget.ConstraintLayout instance
│ Leaking: NO (HomeActivity↓ is not leaking and View attached)
│ View is part of a window view hierarchy
│ View.mAttachInfo is not null (view attached)
│ View.mWindowAttachCount = 1
│ mContext instance of org.mozilla.fenix.HomeActivity with mDestroyed = false
│ ↓ View.mContext
├─ org.mozilla.fenix.HomeActivity instance
│ Leaking: NO (NavHostFragment↓ is not leaking and Activity#mDestroyed is false)
│ mApplication instance of org.mozilla.fenix.DebugFenixApplication
│ mBase instance of androidx.appcompat.view.ContextThemeWrapper
│ ↓ HomeActivity.navHost$delegate
├─ kotlin.SynchronizedLazyImpl instance
│ Leaking: NO (NavHostFragment↓ is not leaking)
│ ↓ SynchronizedLazyImpl._value
├─ androidx.navigation.fragment.NavHostFragment instance
│ Leaking: NO (Fragment.mLifecycleRegistry.state is RESUMED)
│ ↓ Fragment.mChildFragmentManager
│ ~~~~~~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│ Leaking: UNKNOWN
│ Retaining 1.9 kB in 66 objects
│ ↓ FragmentManager.mNonConfig
│ ~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerViewModel instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23736 objects
│ ↓ FragmentManagerViewModel.mViewModelStores
│ ~~~~~~~~~~~~~~~~
├─ java.util.HashMap instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23723 objects
│ ↓ HashMap["390fdd30-354e-44ae-a82f-2c3711cba03e"]
│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ androidx.lifecycle.ViewModelStore instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23686 objects
│ ↓ ViewModelStore.map
│ ~~~
├─ java.util.LinkedHashMap instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23685 objects
│ ↓ LinkedHashMap["mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore"]
│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ org.mozilla.fenix.components.StoreProvider instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23653 objects
│ ↓ StoreProvider.store
│ ~~~~~
├─ mozilla.components.compose.browser.toolbar.store.BrowserToolbarStore instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23642 objects
│ ↓ Store.currentState
│ ~~~~~~~~~~~~
├─ mozilla.components.compose.browser.toolbar.store.BrowserToolbarState instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23612 objects
│ ↓ BrowserToolbarState.displayState
│ ~~~~~~~~~~~~
├─ mozilla.components.compose.browser.toolbar.store.DisplayState instance
│ Leaking: UNKNOWN
│ Retaining 1.6 MB in 23610 objects
│ ↓ DisplayState.browserActionsEnd
│ ~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│ Leaking: UNKNOWN
│ Retaining 1.5 MB in 23470 objects
│ ↓ ArrayList[0]
│ ~~~
├─ mozilla.components.compose.browser.toolbar.concept.Action$TabCounterAction instance
│ Leaking: UNKNOWN
│ Retaining 1.5 MB in 23467 objects
│ ↓ Action$TabCounterAction.onLongClick
│ ~~~~~~~~~~~
├─ mozilla.components.compose.browser.toolbar.store.BrowserToolbarInteraction$CombinedEventAndMenu instance
│ Leaking: UNKNOWN
│ Retaining 1.5 MB in 23465 objects
│ ↓ BrowserToolbarInteraction$CombinedEventAndMenu.menu
│ ~~~~
├─ org.mozilla.fenix.home.toolbar.BrowserToolbarMiddleware$$ExternalSyntheticLambda5 instance
│ Leaking: UNKNOWN
│ Retaining 1.5 MB in 23464 objects
│ ↓ BrowserToolbarMiddleware$$ExternalSyntheticLambda5.f$0
│ ~~~
├─ org.mozilla.fenix.home.toolbar.HomeToolbarEnvironment instance
│ Leaking: UNKNOWN
│ Retaining 1.5 MB in 23463 objects
│ context instance of org.mozilla.fenix.HomeActivity with mDestroyed = true
│ ↓ HomeToolbarEnvironment.context
│ ~~~~~~~
╰→ org.mozilla.fenix.HomeActivity instance
​ Leaking: YES (ObjectWatcher was watching this because org.mozilla.fenix.HomeActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
​ Retaining 148.9 kB in 2681 objects
​ key = f81d055f-a655-4e66-b505-6646c2852751
​ watchDurationMillis = 20373
​ retainedDurationMillis = 15371
​ mApplication instance of org.mozilla.fenix.DebugFenixApplication
​ mBase instance of androidx.appcompat.view.ContextThemeWrapper

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1978626 - Prevent memory leak report for the composable toolbar r=#android-reviewers

Different way of writing the same code prevents the memory leak report
showing that that is a false positive.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

After investigating the reported issue and a potential fix I think the report is a false positive.

Here is a try which reverts the recent "fix" to get back the code that was reported as causing memory leaks. And we indeed see the same reported leaks as before.
Here is a followup try which rewrites the same functionality but does not in itself modify anything that would cause or avoid the reported memory leak - if that would be real. After this change we can see there are no reported memory leaks anymore.

To me this shows the original report is a false positive.

@Segun I'm curious if you agree with my conclusion and if you know of a way to avoid such issues in the future.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/894023ce7ae0
https://hg.mozilla.org/integration/autoland/rev/4d344378eec2
Prevent memory leak report for the composable toolbar r=android-reviewers,Roger

Comment 4

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4d344378eec2