#1630 Fix 8BADF00D crash

[ioanmo226]

This PR fixed 8BADF00D crash

close #1630 // if this PR closes an issue

---

Tests (delete all except exactly one):

• Does not need tests (refactor only, docs or internal changes)

---

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

• addresses the issue it closes (if any)
• code is readable and understandable
• is accompanied with tests, or tests are not needed
• is free of vulnerabilities
• is documented clearly and usefully, or doesn't need documentation

[ioanmo226]

@sosnovsky, @tomholub
As I mentioned in the #1630 issue, 8BADF00D code is error code that the watchdog raises when main thread(UI thread) is blocked so long time.
So my best guess is that in our appDelegate, we cancelled protectedDataDidBecomeAvailableNotification in applicationWillResignActive and as a result, user didn't receive that notification in below code.(Therefore ui thread was blocked after user inactivates and prewarm)
So I moved cancel logic to applicationWillTerminate(Or we won't need cancel logic at all).
Let me know your thoughts

    guard UIApplication.shared.isProtectedDataAvailable else {
        NotificationCenter
             .default
             .publisher(for: UIApplication.protectedDataDidBecomeAvailableNotification)
             .first()
             .sink { _ in
                 GlobalRouter().proceed() // Here
             }.store(in: &waitingForProtectedDataCancellable)
        return true
    }

[sosnovsky]

@ioanmo226 I checked crash report from #1630 and it seems crash happened because of another responder of applicationWillResignActive method:

9   FileProvider                  	0x00000001c23f5da0 -[FPDaemonConnection bookmarkableStringFromDocumentURL:error:] + 412 (FPDaemonConnection.m:340)
10  FileProviderOverride          	0x0000000101654618 -[FPFileProviderModule FPBookmarkableStringFromDocumentURL:error:] + 116 (FPFileProviderModule.m:53)
11  FileProvider                  	0x00000001c23ce8f4 FPCreateBookmarkableStringFromDocumentURL + 252 (FPUtilities.m:83)
12  UIKitCore                     	0x00000001ab7fe130 -[UIDocument _applicationWillResignActive:] + 792 (UIDocument.m:50)

I checked code - app observes UIApplication.willResignActiveNotification in ComposeViewController and calls stopDraftTimer when app goes to background. This view controller also uses UIDocumentPicker for picking attachments, so maybe crash happened when user opened document picker and then switched to another app.

Can you please check it?

[ioanmo226]

I checked deeply and here's my thought.
ComposeViewController indeed listens to willResignActiveNotification but there's nothing wrong with stopDraftTimer function. (I couldn't reproduce crash with above steps.)
I think user indeed opened document picker in ComposeViewController and switched app. Therefore below appDelegate code was called and app couldn't listen to protectedDataDidBecomeAvailableNotification notification after app prewarm.

I might be wrong. Please correct me if I'm wrong.

func applicationWillResignActive(_ application: UIApplication) {
        for cancellable in waitingForProtectedDataCancellable {
            cancellable.cancel()
        }
    }

[sosnovsky]

User can't reach ComposeViewController before UIApplication.willResignActiveNotification arrives, and as we observe only first notification there then it shouldn't affect app after startup.

I also found this line in crash report:

49  FlowCrypt  0x0000000100304144 $s9FlowCrypt11AppDelegateC5$mainyyFZ + 92 (SignInViewDecorator.swift:0)

SignInViewDecorator is used in SetupManuallyImportKeyViewController which also uses UIDocumentPickerViewController and calls url.startAccessingSecurityScopedResource() (https://github.com/FlowCrypt/flowcrypt-ios/blob/master/FlowCrypt/Controllers/Setup/SetupManuallyImportKeyViewController.swift#L203).

It's documentation says:

/// Given an NSURL created by resolving a bookmark data created with security scope, make the resource referenced by the url accessible to the process. When access to this resource is no longer needed the client must call stopAccessingSecurityScopedResource. Each call to startAccessingSecurityScopedResource must be balanced with a call to stopAccessingSecurityScopedResource (Note: this is not reference counted).
    @available(macOS 10.7, iOS 8.0, *)
    public func startAccessingSecurityScopedResource() -> Bool

Which looks similar to crash log from my previous post

9   FileProvider                  	0x00000001c23f5da0 -[FPDaemonConnection bookmarkableStringFromDocumentURL:error:] + 412 (FPDaemonConnection.m:340)
10  FileProviderOverride          	0x0000000101654618 -[FPFileProviderModule FPBookmarkableStringFromDocumentURL:error:] + 116 (FPFileProviderModule.m:53)
11  FileProvider                  	0x00000001c23ce8f4 FPCreateBookmarkableStringFromDocumentURL + 252 (FPUtilities.m:83)
12  UIKitCore                     	0x00000001ab7fe130 -[UIDocument _applicationWillResignActive:] + 792 (UIDocument.m:50)

So probably crash happens when user chose some file to import keys from it and then closed app.
What do you think?

[ioanmo226]

Yeah. That's why I thought ComposeViewController willResignActiveNotification didn't trigger app crash.

User can't reach ComposeViewController before UIApplication.willResignActiveNotification arrives, and as we observe only first notification there then it shouldn't affect app after startup.

0x8badf00d crash doesn't happen immediately after user interaction. But crash happens after some ui thread(main thread) block time which watchdog checks.
As I checked SetupManuallyImportKeyViewController UIDocumentPickerDelegate, i couldn't find ui blocks.
Let me know your thoughts.

https://suelan.github.io/2021/03/05/20210305-crash-ate-bad-food/#:~:text=Thread%3A%20%200-,WatchDog,-As%20we%20know

[sosnovsky]

Maybe func parseUserProvided(data keyData: Data) was executed when app was closed and it's defer callback or func proceedToPassPhrase(with email: String, keys: [KeyDetails]) (which performs some ui updates) was called when app was in background

[ioanmo226]

Hmm... IMHO even if proceedToPassPhrase was called in background, it won't lead app to 0x8badf00d crash.
And parseUserProvided function should be completed less than in one second and therefore defer callback should be completed. If something went wrong, it would fatalError(not 8badf00d error) I think

[sosnovsky]

Yeah, I understand that normally it should work without any crashes, but possibly it was some rare condition when user chose some large file which was parsed longer than usual, then quickly closed the app and it lead to reported crash.

My point here is that implemented fix probably won't change anything, as crash report doesn't include anything related to cancelling waitingForProtectedDataCancellable. But it mentions FPCreateBookmarkableStringFromDocumentURL which should be executed during startAccessingSecurityScopedResource

Maybe you can try to reproduce this crash in SetupManuallyImportKeyViewController by trying to choose different files for parsing and then closing app? This crash can also occur only on device, while working well in simulator, as watchdog can work differently there.

[ioanmo226]

Thanks for your explanation.
Btw I couldn't reproduce crash in any cases(tried on both device & simulator though) and I think 8badf00d crash won't happen here.
I think we need more information to handle this crash.

[sosnovsky]

@tomholub it seems we found where this crash happened, but weren't able to reproduce it.

Probably it happened because of some very rare conditions. Let's monitor it further to get some additional data or maybe it won't happen anymore.

[tomholub]

ok. Thanks

[tomholub]

Often, one can guess-implement a fix - I figured from your conversation that wasn't possible this time.

Other times one can guess-implement a way to make the problem more obvious next time. Do you think we can do anything to surface the real problem easier next time it happens?

[sosnovsky]

@tomholub I think it won't be possible to make it easier to catch next time, as this crash probably happens when app is in background and for some reason ui operation takes too much time.
If there will be another 8BADF00D crash in reports then we'll be able to search similarities with this one to find exact crash reason.

[tomholub]

ok. thanks - closing