Leakcanary: Another leak in EditText

[Guneetgstar]

As discussed in here, this is another leak of EditText(or any other subclass of it) that does not get destroyed every time it gets focused and retains the mContext until the focus gets shifted to another EditText. Please note that mContext is destroyed as soon as the focus gets shifted to another EditText and again a new mContext is retained.
Let me know if more info needed.

LeakTrace information

Leak trace generated on API 23 LG Nexus 5 although when tested on higher versions the leak is gone.
Steps to reproduce: Simple just use an EditText anywhere inside activity and make sure it's focused and after Activity.onDestroy() you will get the leak.

ApplicationLeak(className=com.colevit.furmate.module.container.ContainerActivity, leakTrace=
┬
├─ java.lang.Thread
│    Leaking: UNKNOWN
│    Thread name: 'Studio:InputCon'
│    GC Root: Java local variable
│    ↓ thread Thread.<Java Local>
│                    ~~~~~~~~~~~~
├─ com.android.tools.profiler.support.event.InputConnectionWrapper
│    Leaking: UNKNOWN
│    ↓ InputConnectionWrapper.mTarget
│                             ~~~~~~~
├─ com.android.internal.widget.EditableInputConnection
│    Leaking: UNKNOWN
│    ↓ EditableInputConnection.mTargetView
│                              ~~~~~~~~~~~
├─ androidx.appcompat.widget.AppCompatEditText
│    Leaking: YES (View.mContext references a destroyed activity)
│    mContext instance of android.view.ContextThemeWrapper, wrapping activity com.colevit.furmate.module.container.ContainerActivity with mDestroyed = true
│    View#mParent is set
│    View#mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 1
│    ↓ AppCompatEditText.mContext
├─ android.view.ContextThemeWrapper
│    Leaking: YES (AppCompatEditText↑ is leaking and ContextThemeWrapper wraps an Activity with Activity.mDestroyed true)
│    ↓ ContextThemeWrapper.mBase
╰→ com.colevit.furmate.module.container.ContainerActivity
​     Leaking: YES (ContextThemeWrapper↑ is leaking and Activity#mDestroyed is true and ObjectWatcher was watching this)
​     key = 618d2116-4c2d-4118-ab3a-a9b5d6f8ae68
​     watchDurationMillis = 7833
​     retainedDurationMillis = 2833
, retainedHeapByteSize=7447)

[github-actions[bot]]

🙏Thank you for opening an issue! LeakCanary is maintained by volunteers from the community. Please be kind and remember that LeakCanary isn't anyone's main job 😘.

[consp1racy]

com.android.tools.profiler.support.event.InputConnectionWrapper

So it's only happening when the debugger/profiler/whatever is attached and doesn't happen i production, right?

[Guneetgstar]

LeakCanary as of v2.0+ can only be used as a debugImplementation in gradle and is not installed in release artifacts. Thats why you may not see these logs in production but that doesn't mean the leak is gone, if this is what you mean.

com.android.tools.profiler.support.event.InputConnectionWrapper

So it's only happening when the debugger/profiler/whatever is attached and doesn't happen i production, right?

[consp1racy]

LeakCanary as of v2.0+ can only be used as a debugImplementation in gradle

Recently a popular app decided implementation is fine and included LeakCanary in production. So no, LeakCanary can be included wherever you want and nothing is going to stop you.

My point is that

Thread name: 'Studio:InputCon'
com.android.tools.profiler.support.event.InputConnectionWrapper

sounds like it's connected to Android Studio. That wouldn't be a concern in production.

This definitely is a library/Android SDK leak, I'm just pitching information for the leak description.

[Guneetgstar]

@consp1racy Then I would say I must test its release version as well before commenting on that.
In the mean time I want to mention a similar leak I found while I was recreating this leak. I hope that both the leaks are same but differs in traces due to some reason, what's remarkable is that _the other one is already an identified leak by LeakCanary._ Check this out.

[consp1racy]

That's a totally different leak, just follow the trace. C'mon, if LeakCanary says it's a different leak then it is a different leak. Have some trust, it's the tool's job, it's been doing that for years.

[Guneetgstar]

That's a totally different leak, just follow the trace. C'mon, if LeakCanary says it's a different leak then it is a different leak. Have some trust, it's the tool's job, it's been doing that for years.

Yeah, obviously I trust LeakCanary that's why I use it. But what makes me doubtful is that how could a leak can disappear in a similar implementation with same config. If you see, these two leaks are different but the one disappears in the other implementation.

[Guneetgstar]

Anyways @consp1racy this issue doesn't seem to comply with what you said and I neither find anything good with your statement in the official LeakCanary documentation as well.

LeakCanary as of v2.0+ can only be used as a debugImplementation in gradle

Recently a popular app decided implementation is fine and included LeakCanary in production. So no, LeakCanary _can_ be included wherever you want and nothing is going to stop you.

[Guneetgstar]

@consp1racy Then I would say I must test its release version as well before commenting on that.

And not just the concerned one but as per my test LeakCanary is not able to trace any single leak in my release builds! This includes the one I mentioned earlier.

[pyricau]

A few notes:

• LeakCanary disables object watching in release builds, though you can reenable that. So in release builds it won't detect anything, and soon we'll make it crash as a default.
• When the debugger is attahed, LeakCanary won't dump heap. So we know that wasnt the case here.
• LeakCanary displays just one path, there could be another path from a different root. However thread locals have power priority so its likely this leak is indeed caused by the profiler. We'd need a hprof file reproducing this to confirm.
• While you def won't have the profiler in release builds and therefore this leak is debug only, might still be nice to identify the root cause and get it fixed so that we get less noise in leak reports.

[Guneetgstar]

Thanks @pyricau for your points they are always helpful.

• LeakCanary displays just one path, there could be another path from a different root. However thread locals have power priority so its likely this leak is indeed caused by the profiler. We'd need a hprof file reproducing this to confirm.

I can share the hprof if that could help.

• While you def won't have the profiler in release builds and therefore this leak is debug only, might still be nice to identify the root cause and get it fixed so that we get less noise in leak reports.

And Yup, I tested the release build with AppWatcher enabled and the leak was gone. I did it as I thought if there is another path for the leak I can see with profiler disabled but that was not the case.

The last thing I am still wondering is that how is it possible for a library leak to go away in one project and present in the other?(I tested this with same configs and same device in release build and the library leak is present as expected)

[pyricau]

This isn't the same leak. Here the problem most likely comes from com.android.tools.profiler.support.event.InputConnectionWrapper, sounds like maybe you used the profiler?

[Guneetgstar]

This isn't the same leak. Here the problem most likely comes from com.android.tools.profiler.support.event.InputConnectionWrapper, sounds like maybe you used the profiler?

I think you got me wrong. Let me clarify.

• There is just one leak in this activity.
• This is a completely different 'expected-library leak' due to profiler.
• There has to be another library leak with trace similar to this as well hence taking the total to 2 leaks in this activity (because configs for both the tests are same).

Tell me if I am wrong?

[pyricau]

Sorry, I'm confused, I don't really follow. The leak trace you shared at the top is related to the profiler. Are you saying its not?

[Guneetgstar]

No, you are absolutely right!
I am saying that in addition to this one there must be 1 more leak, how could the other go away after all thats a library leak? Is that possible? I am just asking if you don't mind!

[pyricau]

I'm really sorry but I'm completely lost. You initially shared a single leak, which was classified as Application Leak. I don't understand which one is "the other one", how it's a library leak and what it has to do with the profiler leak.

[Guneetgstar]

Just look at the trace here and compare it with the above one. Even though these two are completely different, you will find few points common:

• Both has the same device configs.
• Both has a leaking AppCompatEditText.
• (Explicitly) Both the leak traces are from the same build variant i.e. debug.

While the leak trace here is an application leak and only exists in non prod, the other (I just referred above) is a library leak.
My question is why both the leaks can't be found at the same leak trace?
I am pointing this out because reproducing these two leaks has the same steps.

[pyricau]

Thanks! I understand now.

A few things to consider:

• A leak is typically a bug in code, related to lifecycle. Somewhere a reference to something isn't cleared when it should be cleared.
• The bug is rarely directly in the class that leaks. E.g. here AppCompatEditText happens to leak in both cases yet there's probably nothing wrong with the code in AppCompatEditText.
• At runtime there can be many bugs / leaks at once. The object graph is complex, when an instance is leaking, there are usually many different path from GC Roots to the leaking instance. A single bug / leak can still be expressed as a thousand varying paths. There's no easy way to distinguish a case of two bugs vs one bug causing a single instance leak. So LeakCanary surfaces just one path (the leak trace), and amongst all the possible paths it picks the best / shortest.
• The idea is that if there's only one "bug", then it will surface in that shortest path anyway, and so shortest means less information to look at. If there are two bugs, well too bad, let's fix the first one and then the leak will surface again later.

Here the leaktraces are fairly different so this is probably two distinct bugs (or it could be some more general issue with InputMethodManager).

[Guneetgstar]

Ok, thanks for all of your points.

Your first point hit me so well that I just tested my app one more time exactly the same way I tested this and I found that the other(referred) leak is still there and I was not able to see it earlier was all because of the lifecycle of the application as you mentioned as _this one only occurs when you exit your application, not necessarily when Activity.onDestroy is called_.

Conclusion: Do not suppose that a leak is gone if it's not found. There could be another way/path to that.

[Guneetgstar]

This definitely is a library/Android SDK leak, I'm just pitching information for the leak description.

For the description I would add that InputConnectionWrapper holds EditableInputConnection.mTarget and hence the mContext only until a new view asks for keyboard input

[dmytroKarataiev]

I've created a repo where you can easily reproduce this leak on Samsung devices: https://github.com/dmytroKarataiev/TextInputLayout-MemoryLeak