Verify block is still alive before calling it in task callbacks #7054
Conversation
| if (callback) { | ||
| callback(metadata, self.error); | ||
| } | ||
| self->_fetcherCompletion = nil; |
There was a problem hiding this comment.
Mostly curious, why the retain cycle is needed here? A comment on it would be useful to avoid such questions in the future.
There was a problem hiding this comment.
Maybe to avoid multiple calls? I'm not sure, but either way, it seems outside the scope of this PR.
There was a problem hiding this comment.
sure! You just removed the warning suppression so I thought you know why it's actually needed :)
There was a problem hiding this comment.
I removed the warning suppression because the warning suppression isn't needed :). Warnings don't get generated.
|
|
||
| [fetcher beginFetchWithCompletionHandler:^(NSData *_Nullable data, NSError *_Nullable error) { | ||
| weakSelf.fetcherCompletion(data, error); | ||
| if (weakSelf.fetcherCompletion != nil) { |
There was a problem hiding this comment.
This is unsafe--the object can be deallocated while the method is in progress. Calling a method does not retain self. You need to assign from weakSelf to a (new) strong reference and then check if that strong reference is nil.
|
Thanks @wilhuff |
Fix #7051
General case follow up to #3786