Skip to content

Fix SDWebImageDownloadOperation imageData multi-thread issue. #2011

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

Merged

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Aug 30, 2017

Force acess to imageData on the same delegate queue to solve non thread safe issue. And ensure image process and completion block with non-mutable data

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues: ...

Pull Request Description

This PR is about #1998

The NSMutableData imageData can be set to nil during -(void)reset call and some call in URLSessionDelegate. So there may be an data race that the reset queue and delegate queue is not equal, and this can cause multi-thread problem.

We can make all the access(getter and setter) to imageData into an single queue to ensure this. But since all the acess to imageData is under NSURLSessionDelegate call, so we just need to change that self.imageData = nil in to the same delegate queue. So I get the queue from the session and put that nil process into it.

And moreover. The imageData is mutable but the completion block is an immutable NSData. We should always use copy to get the immutable instance to avoid modification while process.(Since NSData supports Copy on Write and many cases this would not cause any performance issue)

@dreampiggy dreampiggy force-pushed the Fix_downloadOperation_imagedata_crash branch from 52c1548 to b0f44a6 Compare August 31, 2017 10:06
…on thread-safe issue. Also, ensure imageData in completion block is immutable to avoid accident modification from the other queue
@dreampiggy dreampiggy force-pushed the Fix_downloadOperation_imagedata_crash branch from b0f44a6 to 9021e5b Compare August 31, 2017 13:05
@dreampiggy dreampiggy added this to the 4.1.1 milestone Sep 9, 2017
@dreampiggy dreampiggy mentioned this pull request Sep 19, 2017
3 tasks
@dreampiggy dreampiggy merged commit cd11580 into SDWebImage:master Sep 28, 2017
@bpoplauschi
Copy link
Member

Looks good @dreampiggy, thanks.

@dreampiggy dreampiggy deleted the Fix_downloadOperation_imagedata_crash branch October 6, 2017 19:21
@dreampiggy dreampiggy mentioned this pull request Jan 28, 2018
@dreampiggy dreampiggy mentioned this pull request Oct 29, 2018
8 tasks
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.

2 participants