Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Message memory leaks #932

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

pyricau
Copy link
Member

@pyricau pyricau commented Mar 17, 2015

Prior to Android L, the VM caches stack local variables in blocking loops. This can lead to memory leaks of messages for threads that become inactive but aren't shutdown. This change ensure that we never leak for too long and always replace the leaking reference.

These memory leaks tend to be problematic when using Picasso in an app that also uses Dialogs, because Dialogs obtain template Message instance that are never recycled. These messages are listeners and often hold references to the activity. When the dialog is GCed, the messages should be GCed too but they aren't because of the thread stack local message leak.

screen shot 2015-03-16 at 6 22 52 pm

screen shot 2015-03-16 at 6 23 04 pm

screen shot 2015-03-16 at 6 23 11 pm

Prior to Android L, the VM caches stack local variables in blocking loops. This can lead to memory leaks of messages for threads that become inactive but aren't shutdown. This change ensure that we never leak for too long and always replace the leaking reference.

These memory leaks tend to be problematic when using Picasso in an app that also uses Dialogs, because Dialogs obtain template Message instance that are never recycled. These messages are listeners and often hold references to the activity. When the dialog is GCed, the messages should be GCed too but they aren't because of the thread stack local message leak.
@dnkoutso dnkoutso added this to the Picasso 2.5.1 milestone Mar 17, 2015
JakeWharton added a commit that referenced this pull request Mar 19, 2015
@JakeWharton JakeWharton merged commit 6204195 into square:master Mar 19, 2015
@rahulrj
Copy link

rahulrj commented Feb 9, 2017

I have few questions on this @pyricau . I have also asked these questions on your blog here

  1. For this leak why is a BlockingQueue required? Is this because if its not a blocking operation, then the Thread also will be garbage collected?
  2. Also here Fixing Message memory leaks #932, you wrote in the description “….memory leaks of messages for threads that become inactive but aren’t shutdown”. But if a thread is inactive, then wont it be Gced along with the message?
  3. Also its written that “….Prior to ART, a thread waiting on a blocking queue will leak the last dequeued object as a stack local reference.” But the following code also exists in the latest version of Android in Looper class.
for (;;) {
    Message msg = queue.next(); // might block
    if (msg == null) {
        return;
    }
    msg.target.dispatchMessage(msg);
    msg.recycleUnchecked();
}

So shouldn’t the leak be reproducible on latest version of Android also?

@seekforwings
Copy link

seekforwings commented Sep 3, 2019

@rahulrj I think you should have the 'Debuggable in 'Project Structure' -> 'Build Types' being set to true for the debug mode. Change it to false and retry on the art device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants