This repository has been archived by the owner on Jan 17, 2023. It is now read-only.
Fix for issue #2572 Crash on AFImageWithDataAtScale when loading image #2815
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#2572 - UIImage initWithData is not thread safe. In order to avoid crash we need to serialise call
|
I ran into this issue as well and tried using the changes from your pull request to fix. Please note that your +[ImageLockInitializer initialize] method never gets called. You need to use the class somewhere for it's initialize method to be called. |
|
You are right. Added UIImage category with safeWithData method. This should do the trick |
|
Running into this one pretty frequently. Is this something that will be merged and released in the near future? Thanks. |
|
Anyone have any ideas how we could create a test that causes this to crash in the current version of the library? |
|
@kcharwood Probably iterating for example 10000 times to create 10000 threads all calling the method AFImageWithDataAtScale may do the trick... although being a threading issue it might sometimes not crash... |
|
I'm experimenting with a test like this right now: - (void)testCrash {
// This is an example of a functional test case.
NSString *bundlePath = [[NSBundle bundleForClass:[AFImageResponseSerializerTests class]] resourcePath];
NSString *imagePath = [bundlePath stringByAppendingPathComponent:@"huge_image.jpg"];
NSData *data = [NSData dataWithContentsOfFile:imagePath];
NSDictionary *headers = @{@"Content-Length":@"8621961",@"Content-Type":@"image/jpeg"};
NSHTTPURLResponse *response = [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"http://test.com"] statusCode:200 HTTPVersion:@"1.1" headerFields:headers];
for (int i = 0; i < 100000; i++) {
XCTestExpectation *expectation = [self expectationWithDescription:@"wait"];
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
AFImageResponseSerializer *serializer = [AFImageResponseSerializer serializer];
XCTAssert([serializer responseObjectForResponse:response data:data error:nil]);
[expectation fulfill];
});
}
[self waitForExpectationsWithTimeout:30.0 handler:nil];
} |
|
@paulosotu Thanks for tackling this. I made some minor changes here. If these looks good, I'll get this merged in! Let me know your thoughts. 🍻 |
|
@kcharwood Glad to help :) Seems fine to me, so go for it. |
|
Merged as part of #2860 🍻 all around. |
nicklockwood
added a commit
to facebook/react-native
that referenced
this pull request
Nov 20, 2015
Summary: public I had previously assumed (based on past experience and common wisdom) that `[UIImage imageWithData:]` was safe to call concurrently and/or off the main thread, but it seems that may not be the case (see AFNetworking/AFNetworking#2815). This diff replaces `[UIImage imageWithData:]` with ImageIO-based decoding wherever possible, and ensures that it is called on the main thread wherever that's not possible/convenient. I've also serialized access to the `NSURLCache` inside `RCTImageLoader`, which was causing a separate-but-similar crash when loading images. Reviewed By: fkgozali Differential Revision: D2678369 fb-gh-sync-id: 74d033dafcf6c412556e4c96f5ac5d3432298b18
sunnylqm
pushed a commit
to sunnylqm/react-native
that referenced
this pull request
Dec 2, 2015
Summary: public I had previously assumed (based on past experience and common wisdom) that `[UIImage imageWithData:]` was safe to call concurrently and/or off the main thread, but it seems that may not be the case (see AFNetworking/AFNetworking#2815). This diff replaces `[UIImage imageWithData:]` with ImageIO-based decoding wherever possible, and ensures that it is called on the main thread wherever that's not possible/convenient. I've also serialized access to the `NSURLCache` inside `RCTImageLoader`, which was causing a separate-but-similar crash when loading images. Reviewed By: fkgozali Differential Revision: D2678369 fb-gh-sync-id: 74d033dafcf6c412556e4c96f5ac5d3432298b18
Crash--
pushed a commit
to Crash--/react-native
that referenced
this pull request
Dec 24, 2015
Summary: public I had previously assumed (based on past experience and common wisdom) that `[UIImage imageWithData:]` was safe to call concurrently and/or off the main thread, but it seems that may not be the case (see AFNetworking/AFNetworking#2815). This diff replaces `[UIImage imageWithData:]` with ImageIO-based decoding wherever possible, and ensures that it is called on the main thread wherever that's not possible/convenient. I've also serialized access to the `NSURLCache` inside `RCTImageLoader`, which was causing a separate-but-similar crash when loading images. Reviewed By: fkgozali Differential Revision: D2678369 fb-gh-sync-id: 74d033dafcf6c412556e4c96f5ac5d3432298b18
aleclarson
pushed a commit
to aleclarson/react-native
that referenced
this pull request
Sep 5, 2016
Summary: public I had previously assumed (based on past experience and common wisdom) that `[UIImage imageWithData:]` was safe to call concurrently and/or off the main thread, but it seems that may not be the case (see AFNetworking/AFNetworking#2815). This diff replaces `[UIImage imageWithData:]` with ImageIO-based decoding wherever possible, and ensures that it is called on the main thread wherever that's not possible/convenient. I've also serialized access to the `NSURLCache` inside `RCTImageLoader`, which was causing a separate-but-similar crash when loading images. Reviewed By: fkgozali Differential Revision: D2678369 fb-gh-sync-id: 74d033dafcf6c412556e4c96f5ac5d3432298b18
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#2572 - UIImage initWithData is not thread safe. In order to avoid
crash we need to serialise call.