exponea / exponea-ios-sdk

MIT License
19 stars 27 forks source link

Crash when configuring due to force-unwrap inside the Exponea SDK: TrackingManager #71

Closed rberggreenNLTG closed 18 hours ago

rberggreenNLTG commented 2 weeks ago

Issue

Steps to reproduce

Call ExponeaInternal.configure very early after the app launches, e.g. early in AppDelegate.application(_:didFinishLaunchingWithOptions:)

Make the call from a context where it's not guaranteed to run on the main thread, e.g. from an actor.

Expected

The configure call should work and Exponea SDK should be set up

Actual

In some situations, the app crashes, due to a force-unwrap inside the Exponea SDK

Technical details

In TrackingManager.init on line 103, an observer is added for UIApplication.didBecomeActiveNotification. The callback method from that notification, TrackingManager.applicationDidBecomeActiveUnsafe is then sometimes immediately called if the application did become active at that moment. In that callback method, the lazy var TrackingManager.notificationManager is referenced for the first time, causing it to be created, which in its turn references the global variable Exponea.shared.trackingConsentManager using a force-unwrap (!). This property, however, is not created until line 113 in TrackingManager.init, where the trackManagerInitializator closure is called which sets this global variable to a value for the first time. Therefore the app crashes.

A quick fix to this may be to move the trackManagerInitializator(self) call in TrackingManager.init to before the NotificationCenter.addObserver calls. However, I would advise you to please, never use global variables, and never use force-unwraps. The code should preferably be re-arranged in a way that guarantees that objects are created in the right order and passed as guaranteed-to-exist values to the facilities that need them.

Environment

Exponea SDK 2.26.1 iPhone XR iOS 17.2.1

adam1929 commented 2 weeks ago

Thank you @rberggreenNLTG for detailed description. We look into it.

rberggreenNLTG commented 1 week ago

I may not have been entirely right regarding what stack of calls are made when this force-unwrap happens. It may not be the notification that causes it, but something else, and it still happens if we call on the main thread. Anyway, it appears that whenever we call ExponeaInternal.configure, a series of calls are made internally in the Exponea SDK that always result in a crash due to the force-unwrap Exponea.shared.trackingConsentManager!. I'm attaching a crash report:

Incident Identifier: [censored]
Hardware Model:      iPhone11,8
Process:             [censored] [2484]
Path:                /private/var/containers/Bundle/Application/[censored]
Identifier:          [censored]
Version:             5.2.6 (20240620.1)
AppStoreTools:       15F31e
AppVariant:          1:iPhone11,8:17
Beta:                YES
Code Type:           ARM-64 (Native)
Role:                Foreground
Parent Process:      launchd [1]
Coalition:           [censored] [519]

Date/Time:           2024-06-24 11:34:45.4725 +0200
Launch Time:         2024-06-24 11:34:45.2213 +0200
OS Version:          iPhone OS 17.2.1 (21C66)
Release Type:        User
Baseband Version:    6.00.00
Report Version:      104

Exception Type:  EXC_BREAKPOINT (SIGTRAP)
Exception Codes: 0x0000000000000001, 0x000000010852c144
Termination Reason: SIGNAL 5 Trace/BPT trap: 5
Terminating Process: exc handler [2484]

Triggered by Thread:  0

Thread 0 name:
Thread 0 Crashed:
0   CAPPFeatures                    0x000000010852c144 Swift runtime failure: Unexpectedly found nil while unwrapping an Optional value + 0 (<compiler-generated>:0)
1   CAPPFeatures                    0x000000010852c144 TrackingManager.notificationsManager.getter + 864 (TrackingManager.swift:39)
2   CAPPFeatures                    0x0000000108444008 protocol witness for TrackingManagerType.notificationsManager.getter in conformance TrackingManager + 4 (ExponeaInternal.swift:0)
3   CAPPFeatures                    0x0000000108444008 ExponeaInternal.pushNotificationsDelegate.setter + 60
4   CAPPFeatures                    0x000000010843f3e0 closure #1 in ExponeaInternal.configure(_:pushNotificationTracking:automaticSessionTracking:defaultProperties:inAppContentBlocksPlaceholders:flushingSetup:allowDefaultCustomerProperties:advancedAut... + 1628 (Exponea+Configuration.swift:218)
5   CAPPFeatures                    0x000000010843ed30 ExponeaInternal.configure(_:pushNotificationTracking:automaticSessionTracking:defaultProperties:inAppContentBlocksPlaceholders:flushingSetup:allowDefaultCustomerProperties:advancedAuthEnabled:) + 636 (Exponea+Configuration.swift:244)
6   CAPPFeatures                    0x000000010823ada4 specialized DefaultBloomreachManager.init(configuration:exponeaInternal:deviceTokenCache:authenticationLayer:graphQLClientService:analyticsManager:) + 800 (DefaultBloomreachManager.swift:56)
7   CAPPFeatures                    0x0000000108236fc8 DefaultBloomreachManager.init(configuration:exponeaInternal:deviceTokenCache:authenticationLayer:graphQLClientService:analyticsManager:) + 32 (<compiler-generated>:0)
8   [censored]                          0x0000000104248f40 static DefaultBloomreachManager.makeDefault(configuration:deviceTokenCache:authenticationLayer:graphQLClientService:analyticsManager:) + 128 (<compiler-generated>:0)
9   [censored]                          0x0000000104248f40 specialized static BaseContainer.makeDefault() + 976
10  [censored]                          0x0000000104248514 static BaseContainer.makeDefault() + 4 (<compiler-generated>:0)
11  [censored]                          0x0000000104248514 @objc static BaseContainer.makeDefault() + 12
12  [censored]                          0x000000010421a09c -[CAPPAppDelegate application:didFinishLaunchingWithOptions:] + 112 (CAPPAppDelegate.m:80)

[...]
adam1929 commented 1 week ago

Hi @rberggreenNLTG Thank you for additional info in stacktrace. We need to ensure that accessing to any internal part of SDK has to be initialized before usage.

rberggreenNLTG commented 1 week ago

We have now concluded that issue occurred due to our project importing two copies of the ExponeaSDK, in two different modules. This led to undefined behavior, where the SDK's internal usage of its own symbols was sometimes dynamically directed to the other copy of the SDK which had different state than expected, e.g. a nil trackingConsentManager property. We solved the issue by ensuring that only a single ExponeaSDK exists in the app.

adam1929 commented 18 hours ago

Thank you @rberggreenNLTG for confirmation. Yes indeed, SDK should be used as singleton. Anyway we look for some prevention too.