1678425 - Memory leak in GeckoEditable.mListener

Status: NEW

(GeckoView :: Extensions, defect, P2)

Description

[Christian Sadilek [:csadilek]]

We've been fixing memory leaks in Fenix and there's one known leak left which looks to be a GV issue we'd need help with.

We are calling releaseSession when the view is detached but somehow end up in this state: The GC root being in native code, and the view being referenced via GeckoEditable.mListener. Since the GC root isn't in A-C/Fenix we'd need some help understanding what could go wrong here :).

STR in Fenix debug:

• Open private tabs
• Tap on notification to close all private tabs
• Open a new private tab

LeakCanary:

┬───
│ GC Root: Global variable in native code
│
├─ org.mozilla.geckoview.GeckoEditable instance
│    Leaking: UNKNOWN
│    ↓ GeckoEditable.mListener
│                    ~~~~~~~~~
├─ org.mozilla.geckoview.GeckoInputConnection instance
│    Leaking: UNKNOWN
│    ↓ GeckoInputConnection.mTargetView
│                           ~~~~~~~~~~~
├─ mozilla.components.browser.engine.gecko.GeckoEngineView$geckoView$1 instance
│    Leaking: YES (View.mContext references a destroyed activity)
│    Anonymous subclass of mozilla.components.browser.engine.gecko.NestedGeckoView
│    mContext instance of org.mozilla.fenix.HomeActivity with mDestroyed = true
│    View#mParent is set
│    View#mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    ↓ GeckoEngineView$geckoView$1.$context
├─ org.mozilla.fenix.HomeActivity instance
│    Leaking: YES (GeckoEngineView$geckoView$1↑ is leaking and Activity#mDestroyed is true)
│    ↓ HomeActivity.mFragments
├─ androidx.fragment.app.FragmentController instance
│    Leaking: YES (HomeActivity↑ is leaking)
│    ↓ FragmentController.mHost
├─ androidx.fragment.app.FragmentActivity$HostCallbacks instance
│    Leaking: YES (HomeActivity↑ is leaking)
│    ↓ FragmentActivity$HostCallbacks.mFragmentManager
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: YES (HomeActivity↑ is leaking)
│    ↓ FragmentManagerImpl.mNonConfig
╰→ androidx.fragment.app.FragmentManagerViewModel instance
     Leaking: YES (ObjectWatcher was watching this because androidx.fragment.app.FragmentManagerViewModel received ViewModel#onCleared() callback)
     key = 14c16c55-9abe-433d-8578-ea1b78fc6a86
     watchDurationMillis = 59471
     retainedDurationMillis = 54469

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Is this still a problem for you, Christian? I cannot reproduce this on an up-to-date local Fenix x AC x GV build.

Comment 2

[Christian Sadilek [:csadilek]]

I can't reproduce this anymore on the latest builds. Any idea what could cause this though? Definitely unclear how this went away :). If not, I guess we can close and re-open if we see it again?

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

There were some bug fixes in the GeckoEditable area recently. My best guess is that we fixed it there.

Yeah, let's just reopen if necessary.

Comment 5

[[ex-Mozilla] Agi Sferro | :agi]

Gabe, could you provide the example app that you used to reproduce this in Bug 1719937?

Comment 6

[Gabe]

I'll try to make something I can distribute. Be aware that the Reference Browser (https://github.com/mozilla-mobile/reference-browser) has the same behavior/leak of the GeckoEditableChild although it also leaks the GeckoSession objects making it a poor diagnostic tool.

Comment 7

[Gabe]

Attachment: GeckoView_MemLeak_Test.zip

This is a sample app demonstrating the issue with memory leaking from GeckoSession objects.
This sample was tested with the latest stable GeckoView version (90.0.20210716144314) on an emulator running Android 7.1.

This sample opens and closes GeckoSession instances up to the amount specified in MAX_SESSIONS in a sequential manner.

1st test:
Set MAX_SESSIONS to 10. Allow all 10 to open/close (screen will be black). Use Android Studio Profiler to force multiple GCs (allocated counts will decrease).
Perform a heap dump and verify that no instances of the sample's BrowserView class remain (this proves that the sample is not leaking its own view).
Verify that 10 instances of GeckoSession remain (along with many other Gecko objects attached to GeckoSession).
This shows that Android is unable to GC the GeckoSession instances.

2nd test:
Set MAX_SESSIONS to 0. Allow GeckoSession instances to be opened/closed for 1 hour (or more). Use Android Studio Profiler during this time to track memory usage and allocations.
Verify that there is a constant increase in memory usage and allocation counts.
Eventually the app will crash due to lack of memory.

Comment 8

[Gabe]

Attachment: memory-20210719T191908.zip

Heap dump after running the sample for 1 hour.

Comment 9

[Gabe]

Attachment: Profiler_Memory.PNG

Android Studio Profiler screenshot of memory usage after 1 hour.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

De-assigning myself. Since we have a repro, I'm resetting priority.

Comment 11

[[:owlish] 🦉 PST]

still relevant

Comment 12

[Chris Peterson [:cpeterson]]

Assigning this bug to Makoto. He will decide whether we need to prioritize it for 2022 H2.