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

Fixing AFImageWithDataAtScale race condition crash #2860

Merged
merged 5 commits into from Jul 27, 2015

Conversation

Projects
None yet
10 participants
@kcharwood
Copy link
Contributor

commented Jul 27, 2015

As discussed in #2815 and #2572, there is a crash related to a race condition with imageWithData: This PR takes in #2815 with a few minor modifications to hopefully squash this one.

@kcharwood kcharwood added this to the 2.6.0 milestone Jul 27, 2015

kcharwood added a commit that referenced this pull request Jul 27, 2015

Merge pull request #2860 from AFNetworking/AFImageWithDataAtScale_257…
…2_2815

Fixing AFImageWithDataAtScale race condition crash

@kcharwood kcharwood merged commit 7b84a72 into master Jul 27, 2015

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@egold

This comment has been minimized.

Copy link

commented Jul 29, 2015

Thanks for the fix. Would you consider releasing a 2.5.5 release w/ this fix instead of waiting for 2.6.0?

@alandeguz

This comment has been minimized.

Copy link

commented Aug 4, 2015

I'm with @egold ; a 2.5.5 release would be most welcome.

@tsabend

This comment has been minimized.

Copy link

commented Aug 5, 2015

👍 for patch release

@egold

This comment has been minimized.

Copy link

commented Aug 21, 2015

2.6.0 is out, with the fix!

@kcharwood kcharwood deleted the AFImageWithDataAtScale_2572_2815 branch Oct 2, 2015

@rnewman

This comment has been minimized.

Copy link

commented Nov 10, 2015

Has anyone filed a rdar for this behavior of UIImage? The workaround we're using in Firefox for iOS is to create an intermediate CIImage, rather than to serialize, but it'd be nice to kill the workaround one day.


@interface UIImage (AFNetworking)

+ (UIImage*) safeImageWithData:(NSData*)data;

This comment has been minimized.

Copy link
@pronebird

pronebird Nov 20, 2015

Contributor

is it really without a prefix here? I might have overlooked something but it seems that it is defined as af_safeImageWithData above.

This comment has been minimized.

Copy link
@0xced

0xced Nov 20, 2015

Collaborator

The method name in the header does not match the one in the implementation (af_safeImageWithData).

This comment has been minimized.

Copy link
@kcharwood

kcharwood Nov 20, 2015

Author Contributor

Ya we'll need to correct that, and probably come up with something to ensure anyone calling it outside of AF doesn't cause a conflict either...

This comment has been minimized.

Copy link
@benasher44

benasher44 Nov 20, 2015

@kcharwood has anyone experimented with CGImageSource as a way of getting from data to a CGImage? I can't find any documentation that explicitly says this way would be thread-safe, but then again I haven't found any that says +[UIImage imageWithData:] is either.

This comment has been minimized.

Copy link
@benasher44

benasher44 Nov 21, 2015

@kcharwood I was able spend some time comparing a method like this:

+ (UIImage *)imageFromImageSourceWithData:(NSData *)data {
  CGImageSourceRef imageSource = CGImageSourceCreateWithData((__bridge CFDataRef)data, NULL);
  CGImageRef imageRef = CGImageSourceCreateImageAtIndex(imageSource, 0, NULL);
  CFRelease(imageSource);
  UIImage *image = [UIImage imageWithCGImage:imageRef];
  CGImageRelease(imageRef);
  return image;
}

to just plain old [UIImage imageWithData:] So far, results show the above method is either as fast, faster, or sometimes slower than [UIImage imageWithData:]. Basically, they're roughly equivalent performance-wise, depending on how you run the test. I'm going to play with it some more on a real device next week. But in the mean time, do you have a way to reliably make [UIImage imageWithData:] crash? Even just a some example environment settings (iPhone 5, iOS 8.4, etc.) would be helpful. Thanks in advance!

This comment has been minimized.

Copy link
@cnoon

cnoon Nov 22, 2015

Member

I tried for quite some time in AlamofireImage to produce a crash for this scenario and was unsuccessful. There are several reports in the AFN issues that "claimed" they could make it crash in a test suite by iterating 1000's of times over image loading, but I couldn't reproduce the crash behavior.

This comment has been minimized.

Copy link
@kcharwood

kcharwood Nov 23, 2015

Author Contributor

@benasher44

This thread has multiple people taking a stab at it, along with people confirming their results. There is a dropbox link that is no longer valid :(

This comment has been minimized.

Copy link
@benasher44

benasher44 Nov 23, 2015

@kcharwood ah yes I see. Alright. I'll update if I find anything useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.