-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Crash in - -[FIRAMonitor saveMonitoringDataToUserDefaultsOnMonitorQueue] #431
Comments
@nitesh-homage are you able to reproduce this bug, even inconsistently? If so, can you try reproing the issue with NSZombies/tsan enabled? If not, and this bug reappears, please add any additional stack traces to this bug--the more information we have, the better. |
I've seen similar crashes, most in iOS 11. The top of the crash stack is:
I have no idea why this happens, or how to reproduce the crash. But I think it is an Apple bug, because it access UI elements in background thread. Let me know if you have any thoughts. |
Are you listening to any NSUserDefaults changes? It looks like you are updating the UI on a background queue of Firebase Analytics when it updates the NSUserDefaults. |
Ah yeah I see. Notifications are not guaranteed to be delivered on the main queue--if you're doing UI work in a notification handler, remember to dispatch to the main queue. Looks like this isn't caused by the app developer listening to the NSUserDefaults change notification though (context). |
@baolocdo I didn't listen to NSUserDefaults changes. Line 17 is a call to -[NSUserDefaults setObject:forKey:], but line 16 is a call to -[NSNotificationCenter postNotificationName:object:userInfo:]. I don't know how it happens. |
@morganchen12 The problem is that I call -[NSUserDefaults setObject:forKey:] in line 17 (missing in the call stack). I don't know why notifications appear in the call stack. |
iOS allows listening to NSUserDefaults changes by adding an observer to it: https://developer.apple.com/documentation/foundation/nsuserdefaultsdidchangenotification . When the SDK updates NSUserDefaults, the system notifies its observers automatically. That's why you can see on the line: |
@baolocdo So it seems a system bug. Are you able to file a bug to Apple? |
Let me file a bug to Apple and see whether they can fix that. Thanks for reporting. |
@baolocdo Did you hear anything from Apple? Can you please add me to cc line (if possible)? |
No, I haven't received anything from them :( |
Today Apple closed the bug I filed as duplicate with 35323490. I couldn't access to the original bug report though.
|
Thank you @baolocdo ! I'll file a bug too, since Apple uses duplicates as a measure of priority. |
Anything update ? |
None yet. Since it's a bug in UIKit, the fix will come through an update to iOS, and will only resolve the issue for users on that version of iOS or later. Apple may not tell us when they release the fix. |
Are there any workarounds for this, while waiting for Apple? This is now our top crasher. |
Unfortunately there isn't. The Firebase team can work around this by forcing the userdefaults change on the main queue, so the listener in UIKit no longer crashes (probably). @baolocdo is currently looking into that--it's likely the action we'll go with at the cost of a little cruft in Firebase core. |
@morganchen12, @baolocdo - Thank you. Sounds like a reasonable trade-off. |
Is there any update on this? |
None yet :( |
What about now? We have same crash track with @castleche ... from iOS-11.0 to 11.2.5... awful... |
We has same crash track with @castleche and same function crash get diferrent trace.
|
Same issue. Getting things like:
And
Both have in common that "FIRAIsValidConditionalUserProperty" line. |
Tracking internally at b/64945474 |
Hey all, We have a pending fix for this bug. It'll be undergoing some review before it's released, but we hope to have this resolved soon. |
nice thanks for the notification |
@morganchen12 |
Not yet. The fix brings a lot of complexity, so it's likely to be in review for a while. b/64945474 is the internal bug number. |
@morganchen12 Thank you for letting me know! |
We have a beta SDK available that addresses cases where NSUserDefaults are being written off the main thread and hopefully this bug. If you're interested in trying it out, please shoot me an email: katowulf at google. |
@katowulf Hi, can you please confirm is this issue is fixed and included in latest sdk (4.13.0)? Thanks. |
@levantAJ updated firebase to 4.13.0, still getting saveMonitoringDataToUserDefaultsOnMonitorQueue crashes, all in iOS 11 come on guys, months later, this is grounds for removing firebase from my app, this is Google??? |
As @katowulf stated above that we have a potential fix but we need some help in determine whether it is safe to release. We are actively working on the bug, we understand that it is frustrating to wait and we would like to fix it as soon as possible. Please contact @katowulf if you can help us try out the fix. Thanks. |
yo guys any update on this one? |
Still crashing for us... we're using Firebase 5.0.1. Did the last version finally fix it? (the iOS changelog doesn't mention it) Any update??? @katowulf @morganchen12 @Narayane Thanks! |
Yes I'm not sure what this is about, I'm not doing anything "special" with Firebase Analytics with my iOS app and this is one of the biggest crashes, so I have to believe it's happening to everyone...for months. I contacted @katowulf for the potential fix to download, he told me something like "looks like the fix has already been verified"? |
Can you upgrade to the latest Analytics SDK, and add the flag |
Hi, thanks. We'll try this in our next release and get back to you asap for feedback. @rdamiano we have encountered this issue for more than one year, but it's quite minor as we have only a few user sessions crashing over thousand at each release. |
@baolocdo Hi, your fix seems to work, thanks |
Apple asked to verify the fix by using the iOS 12 beta seed. Since I can't reproduce the issue, can any of you confirm the fix (without Analytics' flag)? Thanks |
Hey all, given the response to the flagged fix we're going to enable the flag by default in the next update of Firebase. I'll close this issue for now, it'll go out with the M30 milestone in the next Analytics release. If it regresses after that, please comment here and I'll re-open the issue. Thanks all! |
Hi is the flag added in info.plist to the 5.4.1 version? Thanks! |
The latest version has the Info.plist flag. Please update to the latest version. The flag will be on by default in subsequent release. |
Hi I updated the firebase but I don't see this flag. Will it be in the project's info.plist or somewhere else? Thanks for your quick response |
You'll need to add the key-value pair in the plist yourself. |
Firebase SDK version - 4.5.0
App crashed at this function but couldn't understand why? Please help!!!
The text was updated successfully, but these errors were encountered: