strvcom / ios-chat-component

Chat modular library
MIT License
1 stars 1 forks source link

Crash on initialization #90

Open schwarja opened 3 years ago

schwarja commented 3 years ago

There is a random crash on chat component initialization

danpecher commented 3 years ago

Some screenshots of the crash for context: (It's not reproducible but it happened to me so I took screenshots 😄)

Screenshot 2021-05-11 at 17 23 54 Screenshot 2021-05-11 at 17 09 11

danpecher commented 3 years ago

@ondrejkorol might have some screenshots too, can we invite him to the repo? @schwarja

schwarja commented 3 years ago

@ondrejkorol used to have read access, I gave him maintenance rights now

ondrejkorol commented 3 years ago

Accessing dictionary cachedBeforeInitCalls on some specific occasion = 💣. It's our most frequent crash. Not that bad, in dozens of cases, but I'm afraid once we go live to the App Store, it could be much worse ... currently we have 100-200 active users, chat isn't used much and still it's happening! We need to solve this before launching the app.

Couldn't you help us with this, @cejanen, please? Any idea how we can fix it?

image

cejanen commented 3 years ago

@danpecher @ondrejkorol @schwarja Well I looked at it... The flow seems fine. At least the logic part -> task manager caches all tasks attributed after init to run them again after initialized is set.

All tasks on ChatCore are called with core queue so even issue between taskManager queue vs core queue I havent found.

Overall my suspicious is threading related to issue with background task or failed tasks, retrying loading network

Bc we are not able to simulate it now I suggest more investigation 1) hookup crashlytics logs to chat component logs so we know what was running with more details 2) test init of chat component on slow network / network with high latency so loading network will fall into retry of the task 3) on slow network (and confirm task is not finished) after init run any task (like load messages) and manually kill the app running that task. It should retry to run them all next time and although those are stored and run with afterInit again it can cause some issue as well 4) how to simulate potential similar issue like above but with background task Im not sure...

If any other help needed lmk

ondrejkorol commented 3 years ago

I investigated the issue again yesterday. From my findings - this crash is really caused because reading AND writing to cachedBeforeInitCalls occur at the same time on rare occasion. Even though All tasks on ChatCore are called within core queue, it can happen simultaneously and break the app.

I wasn't able to simulate the exact behaviour in our project. But I tried to play with the ChatApp example. Here I tried to mock the described case = reading the dictionary and writing to the dictionary from a different thread = I was getting one of the multiple different crashes every time I run the project but one of them was exactly our famous EXC_BAD_ACCESS.

DispatchQueue.global(qos: .background).async {
     for xxx in 0...15 {
          self.accessDictionary() // read, write
     }
}

I was following the approach from WWDC video about how to handle dictionary across background threads https://developer.apple.com/videos/play/wwdc2018/414/ (49:30) and so I prepared TaskCache object, where we use a dedicated sync queue for accessing the dictionary.

With this approach, there was no crash at all while mocking the access! I WANT to believe, this could solve our issue. 🤞🤞🤞 And if not, this update shouldn't make the codebase worse, right? Here's the PR: https://github.com/strvcom/ios-chat-component/pull/92

Thoughts? @cejanen @danpecher