Skip to content
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 on FBSDKAppLinkNavigation #1596

Closed
5 tasks done
Kry256 opened this issue Dec 16, 2020 · 6 comments
Closed
5 tasks done

Crash on FBSDKAppLinkNavigation #1596

Kry256 opened this issue Dec 16, 2020 · 6 comments

Comments

@Kry256
Copy link
Contributor

Kry256 commented Dec 16, 2020

Checklist

Environment

Xcode Version: 12.2
Swift Version: 5.3
Installation Platform & Verison: Swift Package
The crash happens on all Facebook iOS 8.x.x SDK versions.

Steps to Reproduce

I'm facing into a crash when invoking
+ (void)resolveAppLink:(NSURL *)destination handler:(FBSDKAppLinkBlock)handler
of FBSDKAppLinkNavigation class

I use it inside
- (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserActivity *)userActivity restorationHandler:(void(^)(NSArray<id<UIUserActivityRestoring>> * __nullable restorableObjects))restorationHandler
of my app delegate class in this way
[FBSDKAppLinkNavigation resolveAppLink:userActivity.webpageURL
handler:^(FBSDKAppLink * _Nullable appLink, NSError * _Nullable error) {

Every time the method is invoked, it makes the app crash, below the log, hoping it could be useful
Screenshot 2020-12-16 at 11 22 36

I installed FacebookCore SDK via swift package (actually 8.2.0 version), so I was able to navigate into the code, and I saw that this crash happens in the line number 167 of FBSDKWebViewAppLinkResolver class, specifically when try to initialize an MKWebView inside the method
- (void)appLinkFromURL:(NSURL *)url handler:(FBSDKAppLinkBlock)handler
..
..
WKWebView *webView = [[WKWebView alloc] init]; (line 167)

At the very beginning of the method I can see those lines
dispatch_async(dispatch_get_main_queue(), ^{
[self followRedirects:url handler:^(NSDictionary<NSString *,id> *result, NSError * _Nullable error) {

IMO I think that followRedirects:url are not responding on main thread, so maybe putting
dispatch_async(dispatch_get_main_queue(), ^{
inside the followRedirects:url handler, could solve it.

Anyone else with the same issue?

@joesus
Copy link
Contributor

joesus commented Dec 17, 2020

I'm trying to work out what UI is being called from a background thread since the appLinkFromURL:handler: callback is already dispatched on the main thread and that's what would display UI.

I think it might have to do with how you're using

- (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserActivity *)userActivity restorationHandler:(void(^)(NSArray<id<UIUserActivityRestoring>> * __nullable restorableObjects))restorationHandler

According to the Apple Docs:

restorationHandler
A block to execute if your app creates objects to perform the task. Calling this block is optional and you can copy this block > and call it at a later time. When calling a saved copy of the block, you must call it from the app’s main thread.

Can you confirm that you are calling the restoration handler on the main thread?

@Kry256
Copy link
Contributor Author

Kry256 commented Dec 18, 2020

Yes I can confirm that.

As I mentioned, my suspicious is that the issue is inside the completion handler of
[self followRedirects:url handler:^(NSDictionary<NSString *,id> *result, NSError * _Nullable error) {
It is not sufficient to call the entire method on the main thread, if the completion handler responds on another thread.

Taking a look at the code, that handler is called as a completion handler of an NSURLSessionDataTask that responds in a thread different than the main

facebook-github-bot pushed a commit that referenced this issue Jan 26, 2021
Summary:
Thanks for proposing a pull request!

To help us review the request, please complete the following:

- [x] sign [contributor license agreement](https://developers.facebook.com/opensource/cla)
- [x] I've ensured that all existing tests pass and added tests (when/where necessary)
- [x] I've updated the documentation (when/where necessary) and [Changelog](CHANGELOG.md) (when/where necessary)
- [x] I've added the proper label to this pull request (e.g. `bug` for bug fixes)

## Pull Request Details

It should solve this crash

#1596 (comment)

Pull Request resolved: #1597

Test Plan: **Add your test plan here**

Reviewed By: jingping2015

Differential Revision: D25589734

Pulled By: joesus

fbshipit-source-id: 5d54caaf2d3eb3a63510fa28a3077416cd662f39
@Kry256
Copy link
Contributor Author

Kry256 commented Jan 27, 2021

I updated to master branch, now I can facing another crash invoking the same method, but now the class involved isFBSDKWebViewAppLinkResolverWebViewDelegate

In particular there is a missed check inside this method

- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
{
if (self.hasLoaded) {
self.didFinishLoad(webView);
decisionHandler(WKNavigationActionPolicyCancel);
}
self.hasLoaded = YES;
decisionHandler(WKNavigationActionPolicyAllow);
}`

that makes the app crash because of

Thread 1: "Completion handler passed to -[FBSDKWebViewAppLinkResolverWebViewDelegate webView:decidePolicyForNavigationAction:decisionHandler:] was called more than once"

@joesus
Copy link
Contributor

joesus commented Jan 28, 2021

This fix looks reasonable enough. Can you share a little more about the use case here? I'm trying to sort out how to test the fixes.

Is it accurate that you're handling a universal link, extracting app link data out of the metadata from that universal link, and that the metadata includes a al:ios:url meta tag that has a url that redirects?

@Kry256
Copy link
Contributor Author

Kry256 commented Jan 28, 2021

Yes it is correct, it is exactely what I'm trying to do.

Unfortunately I can't share with you the link(s) that I'm using, btw this issues started for me when moved from the old Bolt framework to this one integrated in CoreKit

facebook-github-bot pushed a commit that referenced this issue Jan 28, 2021
Summary:
Thanks for proposing a pull request!

To help us review the request, please complete the following:

- [x] sign [contributor license agreement](https://developers.facebook.com/opensource/cla)
- [x] I've ensured that all existing tests pass and added tests (when/where necessary)
- [x] I've updated the documentation (when/where necessary) and [Changelog](CHANGELOG.md) (when/where necessary)
- [x] I've added the proper label to this pull request (e.g. `bug` for bug fixes)

## Pull Request Details

It should solve this crash

#1596 (comment)

Pull Request resolved: #1624

Reviewed By: dreamolight

Differential Revision: D26103826

Pulled By: joesus

fbshipit-source-id: 3589b6b610b65e9a14506e15980aca1a8f93bd85
@joesus
Copy link
Contributor

joesus commented Feb 4, 2021

This fix should make it into the next patch release. Thanks for the help!

@joesus joesus closed this as completed Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants