search

mixpanel / mixpanel-swift

Official iOS (Swift) Tracking Library for Mixpanel Analytics
https://mixpanel.com
Apache License 2.0
430 stars 236 forks source link

XCode 10 GM - Crash on Tapping push notifications (2.5.0) #241

Closed samwarfield closed 5 years ago

samwarfield commented 5 years ago 
Thread 1: closure argument passed as @noescape to Objective-C has escaped: 
file /mixpanel-swift/Mixpanel/AutomaticEvents.swift, line 289, column 77
screen shot 2018-09-20 at 4 21 57 pm 
0    Mixpanel                           0x0000000104ccfa30 NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 648
1    Mixpanel                           0x0000000104ccff6c @objc NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 124
2    UIKitCore                          0x00000001c3704f48 <redacted> + 3444
3    UIKitCore                          0x00000001c3f14fa0 <redacted> + 2432
4    UIKitCore                          0x00000001c3ed42f8 <redacted> + 772
5    UIKitCore                          0x00000001c3ed40f8 <redacted> + 432
6    UIKitCore                          0x00000001c3f17768 <redacted> + 220
7    UIKitCore                          0x00000001c3f182c4 _performActionsWithDelayForTransitionContext + 112
8    UIKitCore                          0x00000001c3f17604 <redacted> + 248
9    UIKitCore                          0x00000001c3f108a0 <redacted> + 368
10   UIKitCore                          0x00000001c371d76c <redacted> + 468
11   FrontBoardServices                 0x000000019986c4bc <redacted> + 228
12   libdispatch.dylib                  0x00000001080acdd4 _dispatch_client_callout + 16
13   libdispatch.dylib                  0x00000001080b0944 _dispatch_block_invoke_direct + 232
14   FrontBoardServices                 0x00000001998aa618 <redacted> + 40
15   FrontBoardServices                 0x00000001998aa12c <redacted> + 416
16   FrontBoardServices                 0x00000001998aa8b0 <redacted> + 56
17   CoreFoundation                     0x0000000196dca5a0 <redacted> + 24
18   CoreFoundation                     0x0000000196dca4e0 <redacted> + 88
19   CoreFoundation                     0x0000000196dc9d6c <redacted> + 176
20   CoreFoundation                     0x0000000196dc48d8 <redacted> + 1040
21   CoreFoundation                     0x0000000196dc4404 CFRunLoopRunSpecific + 436
22   GraphicsServices                   0x0000000199038520 GSEventRunModal + 100
23   UIKitCore                          0x00000001c36ef484 UIApplicationMain + 212

_Originally posted by @samwarfield in https://github.com/mixpanel/mixpanel-swift/issue_comments#issuecomment-423366393_

zihejia commented 5 years ago

hi @samwarfield , how did you integrate Mixpanel? If by pod, could you try run pod install to update 2.5.0, and do a clean build(clean build folder, delete derived folder, etc) again?

We have the issue on the previously release and @saitun has confirmed the latest 2.5.0 has fixed it. https://github.com/mixpanel/mixpanel-swift/issues/232#issuecomment-423373015

samwarfield commented 5 years ago

Used Carthage and said it was 2.5.0... I’ll try again to make sure

jmirota commented 5 years ago

Hi @zihejia

I have same issue. I used both 2.5.0 and swift4.2 branch. Crash is 100% reproducible. All I have to do is to schedule notification with action. When I tap on notification action, for example when iPhone is locked than app is crashing.

I've removed pods and resintall them. Same issue.

Thanks Jakub

zihejia commented 5 years ago

Hi @jmirota, could you help to paste the crash log? What is your Xcode version?

zihejia commented 5 years ago

Hi @samwarfield , from the log you pasted, it seems from an old version. There was a mistake for the tag 2.5.0(should be v2.5.0) and it was fixed right after you experienced this issue. Sorry about that. So if you delete Carthage folder and Cartfile.resolved, run carthage update again, it should point to the latest 2.5.0 version.

zihejia commented 5 years ago

Related https://github.com/mixpanel/mixpanel-swift/issues/237

jmirota commented 5 years ago

Hi @zihejia

Sure. I'm using Xcode 10. I've migrated project to Swift 4.2. Mixpanel is using Swift 4. 

2018-09-21 08:18:53.222647 appName[2990:834911] [Mixpanel] Notification Action: SelectNotificationAction-Medication Reminder
0    Mixpanel                           0x0000000101f8758c NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 648
1    Mixpanel                           0x0000000101f87ac8 @objc NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 124
2    UIKit                              0x000000018daabfac <redacted> + 4216
3    UIKit                              0x000000018dab1250 <redacted> + 2412
4    UIKit                              0x000000018dab0e18 <redacted> + 452
5    UIKit                              0x000000018da9c548 <redacted> + 152
6    UIKit                              0x000000018da9beec <redacted> + 888
7    UIKit                              0x000000018ddcd9d4 <redacted> + 464
8    FrontBoardServices                 0x0000000189533abc <redacted> + 208
9    FrontBoardServices                 0x0000000189561898 <redacted> + 36
10   FrontBoardServices                 0x0000000189561678 <redacted> + 176
11   FrontBoardServices                 0x0000000189561a98 <redacted> + 56
12   CoreFoundation                     0x0000000187968260 <redacted> + 24
13   CoreFoundation                     0x00000001879679b4 <redacted> + 524
14   CoreFoundation                     0x000000018796549c <redacted> + 804
15   CoreFoundation                     0x0000000187893e8c CFRunLoopRunSpecific + 444
16   GraphicsServices                   0x000000018931a0e4 GSEventRunModal + 180
17   UIKit                              0x000000018d880050 <redacted> + 684
18   UIKit                              0x000000018d87af64 UIApplicationMain + 208
19   appName                      0x00000001002ecdd4 main + 208
20   libdyld.dylib                      0x00000001868785b4 <redacted> + 4
piersebdon commented 5 years ago

@jmirota need to run pod update

jmirota commented 5 years ago

@piers12 did run pod update. Still crashing

piersebdon commented 5 years ago

@jmirota your project shouldn't build if it is on Swift 4.2 and the Mixpanel pod is a lower version, so shouldn't crash. I had to take it out of my project last week and brought it back in today and it is working. I ran pod Install and then pod update. I only have local notifications and not push notifications at the moment, so can't help on that crash I'm afraid.

zihejia commented 5 years ago

hi @jmirota , if your Mixpanel is using Swift 4, that could be an old version. The latest 2.5.0 only runs in swift 4.2, could you double check if your local pod config enables you to upgrade to 2.5.0?

jmirota commented 5 years ago

@zihejia it's now using Swift 4.2 but it still crashes

zrzut ekranu 2018-09-21 o 09 18 40 zrzut ekranu 2018-09-21 o 09 19 29
zihejia commented 5 years ago

Hi @jmirota , sorry I still couldn't replicate, did you try to do a clean build? Such as clean the build folder, delete derived folder, delete the app the device, etc If the problem still exists, How is the implementation of your delegate method for userNotificationCenter(_:newDidReceive:withCompletionHandler:)? Or even better, if you could provide a sample app or code snippet. We use swizzle to automatically track the push notification event. If you have any 3rd party library who does the similar thing, that might cause the issue.

It is a high priority for us to solve this issue, however, in the meantime, as a workaround, you can try calling the below method to init Mixpanel instance and turn automaticPushTracking off. init(apiToken: String?, launchOptions: [UIApplicationLaunchOptionsKey : Any]?, flushInterval: Double, name: String, automaticPushTracking: Bool = true, optOutTrackingByDefault: Bool = false)

Let me know how it goes.

jmirota commented 5 years ago

@zihejia I'm using Urbanairship for push notifications. I will try to create sample app with same crash.

zihejia commented 5 years ago

hi @jmirota, thank you so much!

samwarfield commented 5 years ago

@zihejia

I am using Xcode 10 from App Store.

I did clean out Carthage and updated. Cartfile.resolved shows: github "mixpanel/mixpanel-swift" "v2.5.0"

I ran: carthage update --platform iOS --configuration Release mixpanel-swift

*** Checking out mixpanel-swift at "v2.5.0"
*** xcodebuild output can be found in /var/folders/5t/88948k8s7yl01mrbmb16sx2m0000gn/T/carthage-xcodebuild.VzhESU.log
*** Downloading mixpanel-swift.framework binary at "v2.5.0 - Xcode 10 and swift 4.2 support"

I am not using push notifications, just scheduling a local notification 1 minute out and opening it on iPhone X with latest iOS.

Thread 1: closure argument passed as @noescape to Objective-C has escaped: 
file /Users/zihejia/Documents/Projects/Develop/mixpanel-swift/Mixpanel/AutomaticEvents.swift, 
line 289,
column 77
0    Mixpanel                           0x0000000106a540c0 NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 648
1    Mixpanel                           0x0000000106a545fc @objc NSObject.userNotificationCenter(_:newDidReceive:withCompletionHandler:) + 124
2    UIKitCore                          0x00000001c3704f48 <redacted> + 3444
3    UIKitCore                          0x00000001c3f14fa0 <redacted> + 2432
4    UIKitCore                          0x00000001c3ed42f8 <redacted> + 772
5    UIKitCore                          0x00000001c3ed40f8 <redacted> + 432
6    UIKitCore                          0x00000001c3f17768 <redacted> + 220
7    UIKitCore                          0x00000001c3f182c4 _performActionsWithDelayForTransitionContext + 112
8    UIKitCore                          0x00000001c3f17604 <redacted> + 248
9    UIKitCore                          0x00000001c3f108a0 <redacted> + 368
10   UIKitCore                          0x00000001c371d76c <redacted> + 468
11   FrontBoardServices                 0x000000019986c4bc <redacted> + 228
12   libdispatch.dylib                  0x0000000109df8dd4 _dispatch_client_callout + 16
13   libdispatch.dylib                  0x0000000109dfc944 _dispatch_block_invoke_direct + 232
14   FrontBoardServices                 0x00000001998aa618 <redacted> + 40
15   FrontBoardServices                 0x00000001998aa12c <redacted> + 416
16   FrontBoardServices                 0x00000001998aa8b0 <redacted> + 56
17   CoreFoundation                     0x0000000196dca5a0 <redacted> + 24
18   CoreFoundation                     0x0000000196dca4e0 <redacted> + 88
19   CoreFoundation                     0x0000000196dc9d6c <redacted> + 176
20   CoreFoundation                     0x0000000196dc48d8 <redacted> + 1040
21   CoreFoundation                     0x0000000196dc4404 CFRunLoopRunSpecific + 436
22   GraphicsServices                   0x0000000199038520 GSEventRunModal + 100
23   UIKitCore                          0x00000001c36ef484 UIApplicationMain + 212
samwarfield commented 5 years ago

I will double check

userNotificationCenter(_:newDidReceive:withCompletionHandler:)?
samwarfield commented 5 years ago

I have a crashing sample project.

The crash happens if you do this in your delegate method:

func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) {
    DispatchQueue.main.async {
        completionHandler()
    }
}

MixpanelCrash.zip

zihejia commented 5 years ago

thanks so much @samwarfield , I will have a look now.

zihejia commented 5 years ago

Hi @samwarfield, the sample code can reproduce the crash, this is super helpful. Thanks again!

We will fix this asap. In the meantime, you can initialize Mixpanel by Mixpanel.initialize(token: token, launchOptions: launchOptions, automaticPushTracking: false) as a workaround.

zihejia commented 5 years ago

Hi @samwarfield, In your implementation of the delegate method, you use DispatchQueue.main.async but it should not change the closure argument to @noescape. So I am not quite sure why it’s crashed for this runtime error. "Closure argument passed as @noescape to Objective-C has escaped" , maybe it’s a Swift bug. Will do some further investigations.

Is there any reason you have to execute completionHandler() inside DispatchQueue.main.async? If you just call completionHandler() directly which is often the common practice, the crash will go away.

samwarfield commented 5 years ago

@zihejia I was doing a core data fetch off the main thread and putting it back on the main...

I don't see in the documentation that it is required to call this completionHandler() before the scope ends here. Its been fine up until now...

samwarfield commented 5 years ago

@zihejia I wonder if I can call completionHandler() early... or if I should... I think its probably ok to call it sooner then I think.

zihejia commented 5 years ago

hi @samwarfield , i think in this case it should be fine. Because as long as the user response the notification, the app will be in the foreground, iOS won't kill it.

Macmee commented 5 years ago

hey @zihejia I'm still getting the crash on 2.5.0 as well. I'll try out your workaround but I was wondering if there's a beta release out somewhere with this fixed. Thanks!

zihejia commented 5 years ago

hi @Macmee , would you share the crash log? Is it triggered by Push as well?

Macmee commented 5 years ago

hey @zihejia I think I figured it out. The typealias for the completionHandler where didReceiveRemoteNotification gets swizzled was missing @escaping.

I put a PR up that hopefully fixes the issue here

@samwarfield hope this helps you as well. Feel free to use this fork in the meantime:

pod 'Mixpanel-swift', :git => 'https://github.com/Macmee/mixpanel-swift.git', :branch => 'fix-notifications-noescape-crash-issue'

samwarfield commented 5 years ago

@zihejia I refactored so the completion is called before the end of the scope ends and has been working fine.

squarelab-jwlee commented 5 years ago

hello. I have exactly the same issue as this. same crash code line, same message. is there any progress for this? I don't think I can workaround by not calling callback in async queue, because my notification delegate method is swizzled by other framework, and it seems that framework uses async queue.

Macmee commented 5 years ago

@squarelab-jwlee I fixed it and submitted a PR which was accepted but sadly not merged in :(

In the meantime I made and am using this in my profile:

 pod 'Mixpanel-swift', :git => 'https://github.com/Macmee/mixpanel-swift.git', :branch => 'fix-notifications-noescape-crash-issue'
zihejia commented 5 years ago

sorry @Macmee , I just merged into master. We will release a new version tomorrow with some other fixes. @squarelab-jwlee, would you share which delegate callback causes the crash? In the meantime, it might worth to try the master branch and let me know if it fixes your problem.

zihejia commented 5 years ago

Hi @squarelab-jwlee, we have released v2.5.1, are you able to test it and see how it goes?

squarelab-jwlee commented 5 years ago

@zihejia Thank you for replying me. This is the code line that causes the crash. It's the same as the first comment of this thread.

2018-10-08 13 58 09

I tested this with Mixpanel-swift version 2.5.1 on Xcode10. When I touch push notification, it crashed right after app opened.

I saw @Macmee 's PR, I think that's the right point I need. Thanks @Macmee !! But I think there should be one more @escaping to fix my crash .

zihejia commented 5 years ago

Hi @squarelab-jwlee, thanks for your PR and it's been merged in 2.5.2!

gabrieltodaro commented 5 years ago

Hi guys. I am with this issue in the 2.5.7 version.

Captura de Tela 2019-03-14 às 20 09 55

I'm using the latest version of Xcode, testing on iPhone X and iOS 12.1.4. But the difference of my bug (based on what I understood on the other replies) is that I am opening the notification with my app in foreground. I am showing the banner even if my app is in foreground, but I'll try to replicate this bug with the app inn background.