PINAnimatedImageView optimization

[chadpod]

I've got a PINAnimatedImageView in a collection view cell. The image and animatedImage are being nil'd out on prepareForReuse. Subsequently when setting the image on the reused cell, I think the clearing of the animatedImage is causing some issues. Should clearing the animatedImage when setting the image be conditional on the _animatedImage even existing?

Currently:

- (void)setImage:(PINImage *)image
{
    PINAssertMain();
    if (image) {
        self.animatedImage = nil;
    }
    
    super.image = image;
}

Proposed:

- (void)setImage:(PINImage *)image
{
    PINAssertMain();
    if (image && _animatedImage) {
        self.animatedImage = nil;
    }
    
    super.image = image;
}

This change cleared up my issue of setImageFromUrl: not rendering the returned image.

[garrettmoon]

@chadpod the proposed solution isn't thread safe.

Would you mind posting an example project which demonstrates the issue?

[chadpod]

I'll try to pull an example together when I get a break in our release schedule. Hopefully I can remember how to recreate it.

Probably a dumb question on my part, but I don't see how the above isn't thread safe? Can you explain further? Thanks.

[garrettmoon]

The above isn't thread safe because you're accessing _animatedImage without holding the lock. Setting self.animatedImage holds the lock. With your code the main thread could check if _animatedImage isn't nil (which could be in the process of being set on another thread) and then be set to nil before the self.animatedImage = nil code is run.

In practice this wouldn't cause an issue because you're setting self.animatedImage to nil in a thread safe way and the worst that could happen is it's set to nil on another thread, but it's much harder to reason about this than to make sure we hold the lock the entire time we're checking and setting _animatedImage.