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

WeakReference on Target #83

Closed
mbaroukh opened this issue Jun 13, 2013 · 10 comments
Closed

WeakReference on Target #83

mbaroukh opened this issue Jun 13, 2013 · 10 comments

Comments

@mbaroukh
Copy link

Hi.

I had a strange-not-always-appearing bug.
Finally, I found it was because of using WeakReference on
public void into(Target target) {
makeTargetRequest(target, false);
}

I was using this method with an inner class. Something like
.load(xx).into(new Target() {
});

And as Picasso doesn't keep references on it, it was sometimes garbage collected before the image is downloaded.

Having a weakreference on an ImageView make sense but on a Target, I'm not sure.
It needs more work from the developper to keep a strong reference somewhere else to a target object he doesn't really care about.
I'm not afraid Piasso keeps a strong reference on this object until the request is done and the callback called.

Mike

@JakeWharton
Copy link
Member

Use fetch(), it keeps a hard reference. The next version of Picasso will likely force Target on an object and provide a different strong-referenced callback.

@jerrellmardis
Copy link

I just ran into this issue as well. Providing a strong reference to the Target fixed the problem for me.

@jonson
Copy link

jonson commented Aug 14, 2013

This one just got me too (using the latest code from master). I'm loading larger images and using Picasso's resize(), to scale them down. I think this is resulting in an immediate GC by the vm, clearing the weak ref to my Target.

I'm not sure the proper usage of fetch() in this case, I want to get a callback when it's complete. I've resorted to maintaining a strong reference to the Target object myself, but it feels dirty.

@JakeWharton
Copy link
Member

master has a callback version of into.

@jerrellmardis
Copy link

Unless it's changed recently, I don't think the loaded Bitmap is passed as
parameter with the new Callbacks.
On Aug 13, 2013 7:35 PM, "Jake Wharton" notifications@github.com wrote:

master has a callback version of into.


Reply to this email directly or view it on GitHubhttps://github.com//issues/83#issuecomment-22607963
.

@JakeWharton
Copy link
Member

That's by design. It's to take action when an image is loaded (e.g., animate the view) not touch the Bitmap. Implement Target on a view if you need the Bitmap for some reason.

@dnkoutso
Copy link
Collaborator

It's not "dirty" at all. Target is meant to be used for custom views that deal with bitmaps. You should use it and keep a reference to your target to prevent gc.

Keeping the reference around will also allow you to call cancelRequest(target), which you should always invoke.

If Picasso maintained a strong reference to your target it would eventually leak your context until the download was complete.

fetch() in 2.0 no longer requires a reference and keeps no reference. Fetch does what it says and helps you warm up your disk/memory cache without a target.

@jerrellmardis
Copy link

Oh nice. When should you call cancelRequest(Target)?
On Aug 14, 2013 4:07 AM, "Dimitris" notifications@github.com wrote:

It's not "dirty" at all. Target is meant to be used for custom views that
deal with bitmaps. You should use it and keep a reference to your target to
prevent gc.

Keeping the reference around will also allow you to call
cancelRequest(target), which you should always invoke.

If Picasso maintained a strong reference to your target it would
eventually leak your context until the download was complete.

fetch() in 2.0 no longer requires a reference and keeps no reference.
Fetch does what it says and helps you warm up your disk/memory cache
without a target.


Reply to this email directly or view it on GitHubhttps://github.com//issues/83#issuecomment-22623147
.

@SimonVT
Copy link
Contributor

SimonVT commented Aug 14, 2013

You should call cancelRequest when a Target (e.g. a custom View) requests a new image. This will let picasso cancel an old request if it's not yet executed.

@JakeWharton
Copy link
Member

But only if you are not in a list/grid adapter! Requesting an image for the same imageview/target (e.g., in an adapter getView) will do this automatically. You should only need to cancel (and you don't actually need to) if you're making requests and then leaving the screen.

justinmuller referenced this issue in atermenji/android Nov 8, 2013
pedrovgs added a commit to pedrovgs/Nox that referenced this issue May 6, 2015
…k reference of the targets used to get the bitmaps. With this approach our new targets instances was being garbage collected. We've reduced the number of allocations and the number of targets created using an array of NoxItemCatalogImageLoaderListener and using a WeakHashMap to keep the target instances. THIS BUG WAS VERY INTERESTING TO FIX square/picasso#83
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

No branches or pull requests

6 participants