ASMultiplexImageNode holds reference of _downloader

[garrettmoon]

From @samw723 on June 23, 2015 10:38

I found if set self to ASMultiplexImageNode like [[ASMultiplexImageNode alloc] initWithCache:nil downloader:self] , the downloader holds reference while deinit

id _downloader; // seems not __weak

Could someone modify to use __weak on _downloader?

Copied from original issue: facebookarchive/AsyncDisplayKit#502

[garrettmoon]

From @appleguy on June 23, 2015 17:39

@samw723 this is an interesting case to consider. However, I think it could be important that the reference is retained. What if a user creates an ASBasicImageDownloader and passes it in as the downloader, but then releases the object? How would it stay alive for the node's usage?

[garrettmoon]

From @samw723 on June 24, 2015 3:15

@appleguy I understand and can resolve it by using another ASImageDownloaderProtocol implementation rather than using class itself.

I found this problem when I try to create a ASCellNode which contains multiplex image display, and the ASImageDownloaderProtocol implemented on cell node, then cell node cannot be released due to strong reference loop.

It makes developers confuse since your sample project "Multiplex" using _imageNode = [[ASMultiplexImageNode alloc] initWithCache:nil downloader:self];

and this situation will happen on Objective-C ARC mode or in Swift.

[garrettmoon]

From @appleguy on July 5, 2015 19:17

@samw723 looking at this again now. Thanks for raising this issue to my attention; I am equally concerned about the ease of use of the framework for developers who are not as sophisticated as yourself, and may not realize the existence of this issue and the need for a "workaround".

Do you have any specific suggestions how we could resolve this and achieve correctness in both cases? E.g., there are certainly cases where it retaining the downloader is required (I would say, this is probably most cases, particularly because ASBasicImageDownloader is provided for the "out of the box" usage) — but then there is this case where a retain cycle can be set up if you implement the downloader protocol on the object that owns the image node. I'm not entirely sure there is a clean solution to this, but hopefully I'm missing something.