CleverTap / clevertap-ios-sdk

CleverTap iOS SDK
https://clevertap.com
MIT License
56 stars 54 forks source link

Crashes in -[CleverTap generateAppFields] #103

Closed iaagg closed 3 years ago

iaagg commented 3 years ago

Describe the bug Different crashes caused by the same method generateAppFields

Environment We have CleverTap plugged in via Segment, using CleverTap Segment iOS SDK 1.1.8 And therefore we are using CleverTap iOS SDK 3.9.2

Additional context Crashes appeared after we updated to CleverTap Segment iOS SDK 1.1.6 And therefore we were using CleverTap iOS SDK 3.9.0

So update to the latest version didn't help, crashes still appear. We can't reproduce them though.

Stack traces Crash type №1

Crashed: com.apple.main-thread
0  libobjc.A.dylib                0x1bb4ab020 objc_release + 16
1  CleverTapSDK                   0x1033286cc -[CleverTap generateAppFields] + 484
2  CleverTapSDK                   0x10332a06c -[CleverTap recordAppLaunched:] + 456
3  CleverTapSDK                   0x103329ca8 -[CleverTap _appEnteredForeground] + 124
4  CleverTapSDK                   0x103329a7c -[CleverTap _appEnteredForegroundWithLaunchingOptions:] + 196
5  CleverTapSDK                   0x1033367dc -[CleverTap notifyApplicationLaunchedWithOptions:] + 224
6  Segment_CleverTap              0x104ada838 -[SEGCleverTapIntegration launchWithAccountId:token:region:] + 196
7  libdispatch.dylib              0x1a6065db0 _dispatch_client_callout + 20
8  libdispatch.dylib              0x1a6074504 _dispatch_async_and_wait_invoke + 96
9  libdispatch.dylib              0x1a6065db0 _dispatch_client_callout + 20
10 libdispatch.dylib              0x1a60737ac _dispatch_main_queue_callback_4CF + 836
11 CoreFoundation                 0x1a63ed11c __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
12 CoreFoundation                 0x1a63e7120 __CFRunLoopRun + 2508
13 CoreFoundation                 0x1a63e621c CFRunLoopRunSpecific + 600
14 GraphicsServices               0x1bdfb2784 GSEventRunModal + 164
15 UIKitCore                      0x1a8e26ee8 -[UIApplication _run] + 1072
16 UIKitCore                      0x1a8e2c75c UIApplicationMain + 168
17 Bolt Driver                    0x1023fb184 main + 16 (AppDelegate.swift:16)
18 libdyld.dylib                  0x1a60a66b0 start + 4

Crash type №2

Crashed: com.apple.main-thread
0  libobjc.A.dylib                0x20318b6b0 objc_msgSend + 16
1  libobjc.A.dylib                0x20318bf7c objc_release + 140
2  CleverTapSDK                   0x1041f4674 -[CleverTap generateAppFields] + 396
3  CleverTapSDK                   0x1041f606c -[CleverTap recordAppLaunched:] + 456
4  CleverTapSDK                   0x1041f5ca8 -[CleverTap _appEnteredForeground] + 124
5  CleverTapSDK                   0x1041f5a7c -[CleverTap _appEnteredForegroundWithLaunchingOptions:] + 196
6  CleverTapSDK                   0x1042027dc -[CleverTap notifyApplicationLaunchedWithOptions:] + 224
7  Segment_CleverTap              0x10570e838 -[SEGCleverTapIntegration launchWithAccountId:token:region:] + 196
8  libdispatch.dylib              0x20397d888 _dispatch_client_callout + 20
9  libdispatch.dylib              0x20398a4e0 _dispatch_async_and_wait_invoke + 96
10 libdispatch.dylib              0x20397d888 _dispatch_client_callout + 20
11 libdispatch.dylib              0x20398973c _dispatch_main_queue_callback_4CF + 1012
12 CoreFoundation                 0x203efe734 <redacted> + 16
13 CoreFoundation                 0x203ef93e4 <redacted> + 1888
14 CoreFoundation                 0x203ef8964 CFRunLoopRunSpecific + 452
15 GraphicsServices               0x206139d8c GSEventRunModal + 108
16 UIKitCore                      0x231395758 UIApplicationMain + 216
17 Bolt Driver                    0x10310b184 main + 16 (AppDelegate.swift:16)
18 libdyld.dylib                  0x2039b4fd8 start + 4

Crash type №3

Crashed: com.clevertap.serialQueue:R97-779-6R5Z
0  libobjc.A.dylib                0x19c364c90 objc_retain + 16
1  CoreFoundation                 0x187eb1b4c -[__NSDictionaryM setObject:forKeyedSubscript:] + 368
2  CleverTapSDK                   0x1055b06c4 -[CleverTap generateAppFields] + 476
3  CleverTapSDK                   0x1055afc68 -[CleverTap batchHeader] + 64
4  CleverTapSDK                   0x1055ba31c -[CleverTap sendQueue:] + 344
5  CleverTapSDK                   0x1055b9318 -[CleverTap sendQueues] + 92
6  CleverTapSDK                   0x1055b6338 -[CleverTap runSerialAsync:] + 68
7  CleverTapSDK                   0x1055b91b4 -[CleverTap flushQueue] + 172
8  CleverTapSDK                   0x1055b8ec8 -[CleverTap processEvent:withType:] + 2448
9  CleverTapSDK                   0x1055b6338 -[CleverTap runSerialAsync:] + 68
10 CleverTapSDK                   0x1055b83e0 -[CleverTap queueEvent:withType:] + 516
11 CleverTapSDK                   0x1055c8a5c -[CleverTap fetchFeatureFlags] + 240
12 libdispatch.dylib              0x187bf9298 _dispatch_call_block_and_release + 24
13 libdispatch.dylib              0x187bfa280 _dispatch_client_callout + 16
14 libdispatch.dylib              0x187bd64f0 _dispatch_lane_serial_drain$VARIANT$armv81 + 568
15 libdispatch.dylib              0x187bd6fdc _dispatch_lane_invoke$VARIANT$armv81 + 404
16 libdispatch.dylib              0x187be0800 _dispatch_workloop_worker_thread + 692
17 libsystem_pthread.dylib        0x1d07515a4 _pthread_wqthread + 272
18 libsystem_pthread.dylib        0x1d0754874 start_wqthread + 8

Please let me know if you need any additional information

Aditi3 commented 3 years ago

Hi @iaagg,

Allow me some time to examine the stack traces at my end and I will get back to you with an update shortly.

aakarshsasi commented 3 years ago

+1 Seeing several crashes for this recently but not able to reproduce.

Aditi3 commented 3 years ago

I haven't been able to repro the crash at my end as well. I am still working on it to understand where the aforementioned crashes could happen. In case, I find anything meaningful, I will surely get it fixed in the upcoming SDK releases.

iaagg commented 3 years ago

If that helps I found some messages in our crash reports in Firebase.

It's about crash type №2 from the list in the initial message: malloc: Non-aligned pointer 0x31656e6f6850690a being freed Detected over-release of a CFTypeRef 0x28240c8e0 (0 / Not A Type) Detected over-release of a CFTypeRef 0x282414d00 (7 / CFString)

Didn't find anything similar to this in other crash types reports

iaagg commented 3 years ago

@Aditi3 sorry, but maybe you need any additional info for this one? Two of these 3 crashes are in our TOP 5 for a month period and affect our app stability significantly 😢

Aditi3 commented 3 years ago

@iaagg @aakarshsasi Thanks for sharing the details! I have made some modifications to mitigate the crashes as our best bet here considering crash(s) is not reproducible.

We have already added changes in our release branch. You can expect a release by the end of this month.

aakarshsasi commented 3 years ago

@Aditi3 any updates on when the release will be out?

Aditi3 commented 3 years ago

At present, the upcoming version 3.9.3 is in the QA phase and planned to go out early next week.

aakarshsasi commented 3 years ago

@Aditi3 any updates? Wasn't it supposed to go out this week? The crash rates are one of the top three for this crash in our app :/

aakarsh-sasi commented 3 years ago

Updated to 3.9.3 but the crash is still happening.

iaagg commented 3 years ago

Confirm all 3 crashes reported in the initial message still reproduce after the update to 3.9.3

Aditi3 commented 3 years ago

I will take another look at this and try again to reproduce the crash.

In the meantime, if anyone has repro steps and share the same with us then we would surely get it fixed in the upcoming SDK release.

iaagg commented 3 years ago

Hello @Aditi3 Apparently,

we managed to fix this on our side by applying this fix to our fork of CleverTap ios SDK. We found out that generateAppFields method can be called from two different threads (on main thread on app launch from recordAppLaunched method, and on internal clever tap serial queue), which resulted sometimes to threadsafety issue during accessing properties of CTDeviceInfo. We also noticed some properties marked as atomic, but due to overridden getters atomicity is lost, since it should be implemented by developer in case of overriding.

Looks like all crashes happened on accessing atomic model property of CTDeviceInfo and synchronising access to it fixed the problem.

We do consider these changes as a temporary solution, since in our opinion the proper fix should be applied on the level of CleverTap object, but I guess it will be your decision.

We will wait for further updates of SDK, hopefully with fix, meanwhile we are good to proceed with our local solution.

diff --git a/Pods/CleverTap-iOS-SDK/CleverTapSDK/CTDeviceInfo.m b/Pods/CleverTap-iOS-SDK/CleverTapSDK/CTDeviceInfo.m
index 9d340b5b5..e2ca58b80 100644
--- a/Pods/CleverTap-iOS-SDK/CleverTapSDK/CTDeviceInfo.m
+++ b/Pods/CleverTap-iOS-SDK/CleverTapSDK/CTDeviceInfo.m
@@ -117,11 +117,16 @@ - (NSString *)description {
 }

 - (NSString *)deviceId {
-    return _deviceId ? _deviceId : self.fallbackDeviceId;
+    [deviceIDLock lock];
+    NSString *tmpId = [(_deviceId ? _deviceId : self.fallbackDeviceId) copy];
+    [deviceIDLock unlock];
+    return tmpId;
 }

 - (void)setDeviceId:(NSString *)deviceId {
+    [deviceIDLock lock];
     _deviceId = deviceId;
+    [deviceIDLock unlock];
 }

 - (BOOL)isErrorDeviceID {
@@ -371,8 +376,10 @@ - (NSString *)manufacturer {
 }

 - (NSString *)model {
-    if (!_model) {
-        _model = [[self class] getPlatformName];
+    @synchronized (self) {
+        if (!_model) {
+            _model = [[self class] getPlatformName];
+        }
     }
     return _model;
 }
@@ -400,8 +407,10 @@ - (NSString *)deviceHeight {
 }

 - (NSString *)deviceName {
-    if (!_deviceName) {
-        _deviceName = [UIDevice currentDevice].name;
+    @synchronized (self) {
+        if (!_deviceName) {
+            _deviceName = [UIDevice currentDevice].name;
+        }
     }
     return _deviceName;
 }
Aditi3 commented 3 years ago

Hey @iaagg Thank you for suggesting a solution and working on this.

Theoretically, the proposed solution makes sense to me and I'm looking forward to implementing the changes in our development branch.

I will get back to you with a summary of my quick testing and tentative release date.

Thanks again!

Aditi3 commented 3 years ago

Thank you all for your support here. We have merged the changes in our development branch and will be shipping in the next SDK release.

aakarsh-sasi commented 3 years ago

When will v3.10.0 be released?

Aditi3 commented 3 years ago

@aakarsh-sasi We're aiming to release on coming Monday.