removing the event listener causes crashes on iOS 13

e-mission / e-mission-transition-notify

Generate local notifications for various transitions of the finite state machine

BSD 3-Clause "New" or "Revised" License

0 stars 2 forks source link

removing the event listener causes crashes on iOS 13 #27

Closed shankari closed 4 years ago

shankari commented 4 years ago

Not sure on older versions. This has happened many times in testing by @tzachari and others. It happens when you call removeEventListener when there is already an event listener registered.

Concretely, at this line

        NSMutableArray* currList = configWrapper[listName];
        NSUInteger existingIndex = [self findIndex:localNotifyConfig fromList:currList];
        if (existingIndex != -1) { // There is an existing entry for this ID

existingIndex   NSUInteger  9223372036854775807

so then when we call

            [currList removeObjectAtIndex:existingIndex];

There is a crash.

shankari commented 4 years ago

ok, so there were two interrelated errors here.

First, the parsing process from JSON string -> NSDictionary was returning an NSNumber instead of a long. We were comparing the incoming and stored ids with a simple ==, which failed. We need to use isEqualToNumber instead.
And then, when we didn't have a match, we expected to get -1, but we actually ended up getting NSNotFound instead. NSNotFound is the giant number above, so when we tried to delete the entry at that index, we crashed.

The fixes are pretty self-explanatory.

shankari commented 4 years ago

Testing done (on iOS 13):

add event with id 737678
delete event with id 737678 <- event is removed, no crash
delete event with id 737678 again <- event does not exist, no crash
re-add event with id 737678
delete event with id 737679 <- event is not removed, no crash

shankari commented 4 years ago

Also confirmed that add + remove does not crash on android.