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

Fix crash: firstObject is more secure because it returns nil #24

Merged
merged 1 commit into from Sep 1, 2017

Conversation

asam139
Copy link

@asam139 asam139 commented Sep 1, 2017

In some situations the framework crashes because the dispatch is asyncrhonous and it is accesing to the object with zero index, so to fix it it is better to use "firsObject" method because it returns nil and it does not crash.

@isadon isadon closed this Sep 1, 2017
@isadon isadon reopened this Sep 1, 2017
@isadon isadon merged commit 1376334 into isadon:3.0.0 Sep 1, 2017
@isadon
Copy link
Owner

isadon commented Sep 1, 2017

The pull request has not been merged, even though above it states it was..

Not sure why a crash is being generated when accessing a 0 index of an array. Above the 0 index access there are condition (if checks) that make sure the array has at least one object.. Can you give more details about the crash you are getting and why accessing via firstObject solves it?

@asam139
Copy link
Author

asam139 commented Sep 2, 2017

Here you has the explication about firstObject or objectAtIndex: Post

If you don't want to use firstObject you must do the check inside of the dispatch because it is asynchronous and it can crash.

screen shot 2017-09-02 at 14 13 02

Although for me, it is cleaner to use the "firstObject" method.

Here you can see the crash in Fabric of the app which I work.
screen shot 2017-09-02 at 13 57 10

Thanks you so much for your time!! :)

@isadon
Copy link
Owner

isadon commented Sep 6, 2017

The link you posted is no longer working. When I did read it though I don't believe it was sufficient to explain the reasoning why firstObject should be used and how it ties in to the need to use it in the async call. Can you provide steps to reproduce? How exactly is the crash being generated?

@asam139
Copy link
Author

asam139 commented Sep 6, 2017

I fixed the link if you want to read it. If I find the steps to reproduce it, I communicate you 👍

@isadon
Copy link
Owner

isadon commented Sep 7, 2017

As far as the link you posted I understand why a direct access to the array element index 0 results in a crash when the array is empty. firstObject returns nil and prevents a crash. But that wouldn't solve the true problem which is reproducing how the array becomes empty in the async call but not right before the async call in the "if" check. Definitely let me know if you can reproduce the issue.

@isadon
Copy link
Owner

isadon commented Sep 8, 2017

I believe the issue lies in that the dismissNotification method is not thread safe. If multiple messages are displayed and then auto dismissed the timing of the dismiss notifications may possibly lead to a crash. I haven't completely verified this yet but expect an update soon.

@isadon
Copy link
Owner

isadon commented Sep 12, 2017

Can you please try the develop branch and tell me if you still get the crash or can reproduce?

@asam139
Copy link
Author

asam139 commented Sep 12, 2017

ok, I am going to use develop branch and if I can reproduce the crash I will tell you :)

@isadon
Copy link
Owner

isadon commented Sep 12, 2017

also test the regular master branch.. there were some slight modifications there as well that "may" have solved the crash.

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.

None yet

2 participants