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

Add KVO to NSURLSessionTask may cause crash. #390

Closed
zjmdp opened this issue Jul 18, 2017 · 17 comments · Fixed by #410
Closed

Add KVO to NSURLSessionTask may cause crash. #390

zjmdp opened this issue Jul 18, 2017 · 17 comments · Fixed by #410

Comments

@zjmdp
Copy link

zjmdp commented Jul 18, 2017

The crashes are here: http://crashes.to/s/45830cf8ec3

I found the same issue discussed about AFNetworking. https://github.com/AFNetworking/AFNetworking/issues/1477

I searched all the code in my project that the category NSURLSessionTask+Timing with the observer on the property of NSURLSessionTask may cause the crash.

@garrettmoon
Copy link
Collaborator

@zjmdp well that's just… terrible :( How frequently are you seeing this crash?

@zjmdp
Copy link
Author

zjmdp commented Jul 24, 2017

Not very frequent, but it's the most frequently crash for my app.
wx20170724-103409 2x

@garrettmoon
Copy link
Collaborator

Ok, it'll probably be a bit before I can look at this, but if you decide to jump in before then, happy to review a pull request!

@zjmdp
Copy link
Author

zjmdp commented Aug 11, 2017

I just come back to beta.9 for work round. Maybe I need sometime to learn the code before attempting to fix it.

@xilin
Copy link

xilin commented Sep 7, 2017

@garrettmoon Same problem happens on my app, the top 1 crash.
After reading on the AF issue 1477 and 2638, it seems that in the end they decided to use method swizzle instead of KVO (PR 2702) to fix it.
I'll try to create a pull request to fix it.

@workhardupc
Copy link

@zjmdp, We use 3.0.0-beta.11 has this problem, Does 3.0.0-beta.9 has this problem?

@garrettmoon
Copy link
Collaborator

garrettmoon commented Sep 19, 2017

@xilin Thank you for your heroic efforts attempting to fix this! It's really too bad this bug exists. Does anyone have confirmation it still exists in iOS 11?

@hovox
Copy link
Contributor

hovox commented Sep 20, 2017

It only happens on iOS 10.x, no iOS 9 or iOS 11.

@workhardupc
Copy link

@hovox same as us

@garrettmoon
Copy link
Collaborator

Aweeeeesome! I think I may suggest we wait this one out then 😬

@luohui8891
Copy link

Got same issue on iOS 11, see detail here

@workhardupc
Copy link

@garrettmoon Is there any way to use Texture, but not use PINRemoteImage?

@garrettmoon
Copy link
Collaborator

@workhardupc yes, I wrote the integration into Texture to be entirely pluggable.

@garrettmoon
Copy link
Collaborator

Hopefully the PR I put up addresses this. It's a pretty significant change but removes the need for KVO with the limitation that iOS 9 users and below don't get the benefits of knowing time to first byte.

@luohui8891
Copy link

@garrettmoon which PR? Would you mind to provide the URL?

@xilin
Copy link

xilin commented Sep 22, 2017

@luohui8891 See PR#410

@luohui8891
Copy link

@xilin got it, thanks.

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 a pull request may close this issue.

6 participants