firebase-ios-sdk
firebase-ios-sdk copied to clipboard
firebase
App crashed on removing firestore listener from event of DispatchSourceTimer
[REQUIRED] Step 1: Describe your environment
- Xcode version: 13.4.1
- Firebase SDK version: 9.1.0
- Installation method:
CocoaPods - Firebase Component: (Auth, Core, Database, Firestore, Messaging, Storage)
- Target platform(s):
iOS
[REQUIRED] Step 2: Describe the problem
Crashed: com.apple.root.default-qos.overcommit
0 libsystem_pthread.dylib 0x7744
Steps to reproduce:
We have implemented a repeating DispatchSourceTimer which is fire in every one minute to check data on firestore collection (by removing existing listener and adding new one.). You can check added code for better understanding.
Relevant Code:
class RepeatingTimer {
let timeInterval: TimeInterval
init(timeInterval: TimeInterval) {
self.timeInterval = timeInterval
}
private lazy var timer: DispatchSourceTimer = {
let t = DispatchSource.makeTimerSource()
t.schedule(deadline: .now() + self.timeInterval, repeating: self.timeInterval)
t.setEventHandler(handler: { [weak self] in
self?.eventHandler?()
})
return t
}()
var eventHandler: (() -> Void)?
private enum State {
case suspended
case resumed
}
private var state: State = .suspended
deinit {
timer.setEventHandler {}
timer.cancel()
/*
If the timer is suspended, calling cancel without resuming
triggers a crash. This is documented here https://forums.developer.apple.com/thread/15902
*/
resume()
eventHandler = nil
}
func resume() {
if state == .resumed {
return
}
state = .resumed
timer.resume()
}
func suspend() {
if state == .suspended {
return
}
state = .suspended
timer.suspend()
}
}
class UploadDownloadTimerManager: NSObject {
func startTimer(){
DispatchQueue.global(qos: .background).async {
self.timer = RepeatingTimer(timeInterval: TimeInterval(AppConstants.uploadTimerInterval))
self.timer?.eventHandler = {
DataDownloadManager.sharedManager().addDownloadListener() // Add download collection listener
}
self.timer?.resume()
}
}
}
class DataDownloadManager: NSObject {
var downloadListenerQuery:DbQuery? = nil
static var _sharedManager:DataDownloadManager? = nil
static func sharedManager() -> DataDownloadManager {
if _sharedManager == nil {
_sharedManager = DataDownloadManager()
}
return _sharedManager ?? DataDownloadManager();
}
func addDownloadListener(){
self.removeDownloadListener()
// code for reinitiate listener
}
func removeDownloadListener(){
downloadListenerQuery?.dispose()
}
}
class DbQuery: NSObject {
private var firebaseListener: ListenerRegistration?
init(listener: ListenerRegistration? = nil){
firebaseListener = listener
}
func dispose() {
firebaseListener?.remove()
firebaseListener = nil
}
}
I found a few problems with this issue:
- I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
- This issue does not seem to follow the issue template. Make sure you provide all the required information.
Hi @sspogra. This seems to be a duplicate of https://github.com/firebase/firebase-ios-sdk/issues/9302 which you logged earlier this year. Unfortunately, there are no additional insights into the root cause of this issue. Based on your description, however, it sounds like you are able to reproduce this crash. If that is the case, can you provide a reproducible app, in a GitHub repo that I could clone an reproduce myself? Without being able to reproduce I'm not sure how to investigate. Alternately, could you do a thread dump at the time that the crash occurs?
Also, the com.apple.root.default-qos.overcommit error generally means that the dispatch queue in question is overwhelmed with work. Is there anything your app is doing that may be flooding GCD with work, such as enqueuing a bunch of jobs that end up blocking and hogging the threads from GCD's thread pool?
Hi @dconeybe I am not able to reproduce it on local. Crashed only on live app.
@sspogra Is there a chance that your application is flooding GCD with blocking work?
Also, you mentioned in the original post that every minute your app "checks data on firestore collection (by removing existing listener and adding new one.)". This removing-of-and-re-adding-of the listener seems to be unnecessary. If you simply add a listener and leave it then it will get automatically notified when anything changes. If you do need to poll for some reason, then consider using DocumentReference.getDocument() or Query.getDocuments() to perform a one-time request.
Does any of this help your situation?
@sspogra Can you clarify what you mean by your last comment? Were you able to solve the problem by performing one-time "gets" instead of repeatedly adding a snapshot listener then removing it? Are you still experiencing this issue?
@dconeybe Again getting this type of crash on removing listener only once.
Crashed: com.apple.root.background-qos
0 libc++.1.dylib 0xe318 std::__1::__shared_weak_count::__release_weak() + 4
1 FirebaseFirestore 0x10d954 firebase::firestore::api::QueryListenerRegistration::Remove() + 39 (swap.h:39)
2 AppName 0x73160 DbQuery.dispose() + 4300616032 (
@sspogra I still don't have too much insight into this crash. It's quite puzzling. It looks like a use-after-free crash with the ListenerRegistration object. Maybe there is some race condition in your DbQuery class where two threads are calling dispose() concurrently. In that case, one thread could set firebaseListener to nil, causing it to be garbage collected, while the other thread calls its remove() method. If this is indeed the case, one thing you could try is to to avoid setting firebaseListener to nil. Actually, this makes me think that there is a concurrency bug in Firestore's ListenerRegistration class where it could crash if invoked concurrently. I'll put out a fix for that and see if it helps your situation.
I've merged https://github.com/firebase/firebase-ios-sdk/pull/10065 which may fix this crash for you. It will be included in the next release, which is planned for the last week of August 2022. I'll post back here once it's released.
9.5.0 was released yesterday, Aug 23, 2022, with the potential fix. Please test it out and report back.