+[FBSDKAppEvents initialize] may cause app deadlock on very first start

viktorcode commented 3 years ago

Checklist

[x ] I've updated to the latest released version of the SDK
[x ] I've searched for existing GitHub issues
[x ] I've looked for existing answers on Stack Overflow, the Facebook Developer Community Forum and the Facebook Developers Group
[x ] I've read the Code of Conduct
[x ] This issue is not security related and can safely be disclosed publicly on GitHub

Environment

Describe your dev environment here, giving as many details as possible. If you have them, make sure to include:

Xcode Version: 12.1.0
Installation Platform & Verison: Manual version 8.1.0
Unity 2019.4.7

Goals

What do you want to achieve?

Launching a Unity game with Facebook SDK integrated.

Expected Results

What do you expect to happen?

The game should launch

Actual Results

What actually happened? Can you provide a stack trace?

On the very first launch the game hangs and causes crash. Subsequent launches work fine. Stack trace attached. symbolicated.txt

Steps to Reproduce

What are the steps necessary to reproduce this issue?

The issue is project specific and cannot easily be reproduced

Code Samples & Details

Please provide a code sample, as well as any additional details, to help us track down the issue. If you can provide a link to a test project that allows us to reproduce the issue, this helps us immensely in both the speed and quality of the fix.

The problem is caused by the following code in FBSDKAppEvents:

+ (void)initialize
{
  if (self == [FBSDKAppEvents class]) {
    g_overrideAppID = [[[NSBundle mainBundle] objectForInfoDictionaryKey:FBSDKAppEventsOverrideAppIDBundleKey] copy];
    [FBSDKBasicUtility anonymousID];
  }
}

On the very first app launch [FBSDKBasicUtility anonymousID] call ends up with creating a file with anonymous identifier. The problem here is the call being performed from initialize method, which has following limitations (from Apple docs):

NOTE Because initialize is called in a blocking manner, it’s important to limit method implementations to the minimum amount of work necessary possible. Specifically, any code that takes locks that might be required by other classes in their initialize methods is liable to lead to deadlocks. Therefore, you should not rely on initialize for complex initialization, and should instead limit it to straightforward, class local initialization.

To fix issue all you need to do is remove [FBSDKBasicUtility anonymousID] call from initialize. In that case anonymous identifier file will be created lazily when first requested.

vdugnist commented 3 years ago

I have same crashes on FBSDKCoreKit 9.1.0. Fixed it by calling [FBSDKSettings initialize]; before logging any events.

joesus commented 3 years ago

I have same crashes on FBSDKCoreKit 9.1.0. Fixed it by calling [FBSDKSettings initialize]; before logging any events.

Do you have the same issue if you initialize the SDK (not just Settings) before logging events? I think this is a better answer because in future versions things will 100% break if the SDK is not initialized correctly.

See: https://developers.facebook.com/docs/ios/upgrade-guide

vdugnist commented 3 years ago

I have same crashes on FBSDKCoreKit 9.1.0. Fixed it by calling [FBSDKSettings initialize]; before logging any events.

Do you have the same issue if you initialize the SDK (not just Settings) before logging events? I think this is a better answer because in future versions things will 100% break if the SDK is not initialized correctly.

See: https://developers.facebook.com/docs/ios/upgrade-guide

Oh, that's a better solution. Thank you! I will write 100 times "stop using the debugger, use the documentation".

viktorcode commented 3 years ago

In my case we have hang on start at the point of calling initialize, which is the first call to Facebook SDK. So logging events, etc. is happening later. But the hang doesn't happen on every project, so It is dependent on code base.

As I said, the solution is to remove [FBSDKBasicUtility anonymousID]; call. I still have to do it manually whenever we switch to a new version of Facebook SDK.

As I said, Apple's doc makes it perfectly clear that initialize call must be fast. And it clearly isn't. I hope Facebook will fix it some day.

heeveG commented 3 years ago

Hey. Encountered the same exact problem on FBSDK v8.2.

I see a new Facebook SDK v11.2.0 contains a fix for that problem: dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0), ^(void) { [FBSDKBasicUtility anonymousID]; });

Currently it is more convenient for me to change that one line, than to update SDK version. Should I use the fix from v11.2.0, or is it better to stick to @viktorcode's solution of just deleting call to [FBSDKBasicUtility anonymousID]; on my version of SDK?

@joesus

joesus commented 2 years ago

Currently it is more convenient for me to change that one line, than to update SDK version. Should I use the fix from v11.2.0, or is it better to stick to @viktorcode's solution of just deleting call to [FBSDKBasicUtility anonymousID]; on my version of SDK?

I cannot advocate for staying on an older version of the SDK. We are continually making improvements to stability, usability, and performance. It would be helpful to know where the pain points are in upgrading if you'd be willing to share those.

Closing the issue but not locking the comments. Would love to hear the feedback. Thanks!

facebook / facebook-ios-sdk