pusher / push-notifications-swift

Swift SDK for the Pusher Beams product:
https://www.pusher.com/beams
MIT License
34 stars 24 forks source link

Crash when Registering for Remote Notifications #71

Closed elizabethsdavis closed 6 years ago

elizabethsdavis commented 6 years ago

I've integrated the Beams iOS SDK 0.10.7 into one of my apps and have consistently received a crash when users are registering devices for notifications.

PushNotifications.swift line 116 specialized PushNotifications.(registerDeviceToken(Data, completion : () -> ()) -> ()).(closure #1)

Here are the crash logs from Fabric:

Crashed: NSOperationQueue 0x1c0024ea0 (QOS: UNSPECIFIED)
0  libdispatch.dylib              0x1835b5a40 _dispatch_queue_resume$VARIANT$mp + 504
1  PushNotifications              0x1010b8b74 specialized PushNotifications.(registerDeviceToken(Data, completion : () -> ()) -> ()).(closure #1) (PushNotifications.swift:116)
2  PushNotifications              0x1010bb294 partial apply for PushNotifications.(registerDeviceToken(Data, completion : () -> ()) -> ()).(closure #1) (PushNotifications.swift)
3  PushNotifications              0x1010b1e68 specialized NetworkService.(register(deviceToken : Data, instanceId : String, completion : (String?, Bool) -> ()) -> ()).(closure #1) (NetworkService.swift:24)
4  PushNotifications              0x1010b2898 partial apply for NetworkService.(register(deviceToken : Data, instanceId : String, completion : (String?, Bool) -> ()) -> ()).(closure #1) (NetworkService.swift)
5  PushNotifications              0x1010b2748 specialized NetworkService.(networkRequest(URLRequest, session : URLSession, completion : (NetworkResponse) -> ()) -> ()).(closure #1) (NetworkService.swift:120)
6  PushNotifications              0x1010b27e4 partial apply for NetworkService.(networkRequest(URLRequest, session : URLSession, completion : (NetworkResponse) -> ()) -> ()).(closure #1) (NetworkService.swift)
7  PushNotifications              0x1010b6214 @callee_owned (@owned Data?, @owned URLResponse?, @owned Error?) -> ()NSData?NSData (PushNotifications.swift)
8  Stashe                         0x10047c494 __InstrumentDataTaskWithRequestCompletionHandler_block_invoke.82 + 4310631572
9  CFNetwork                      0x18424de4c __75-[__NSURLSessionLocal taskForClass:request:uploadFile:bodyData:completion:]_block_invoke + 32
10 CFNetwork                      0x184266b6c __49-[__NSCFLocalSessionTask _task_onqueue_didFinish]_block_invoke + 152
11 Foundation                     0x1846e6e88 __NSBLOCKOPERATION_IS_CALLING_OUT_TO_A_BLOCK__ + 16
12 Foundation                     0x1846288d0 -[NSBlockOperation main] + 72
13 Foundation                     0x184627cac -[__NSOperationInternal _start:] + 848
14 libdispatch.dylib              0x1835b0a60 _dispatch_client_callout + 16
15 libdispatch.dylib              0x1835b8170 _dispatch_block_invoke_direct$VARIANT$mp + 224
16 libdispatch.dylib              0x1835b0a60 _dispatch_client_callout + 16
17 libdispatch.dylib              0x1835b8170 _dispatch_block_invoke_direct$VARIANT$mp + 224
18 libdispatch.dylib              0x1835b805c dispatch_block_perform$VARIANT$mp + 104
19 Foundation                     0x1846e8750 __NSOQSchedule_f + 376
20 libdispatch.dylib              0x1835b0a60 _dispatch_client_callout + 16
21 libdispatch.dylib              0x1835b8e94 _dispatch_continuation_pop$VARIANT$mp + 424
22 libdispatch.dylib              0x1835b77cc _dispatch_async_redirect_invoke$VARIANT$mp + 604
23 libdispatch.dylib              0x1835bdcac _dispatch_root_queue_drain + 588
24 libdispatch.dylib              0x1835bd9fc _dispatch_worker_thread3 + 120
25 libsystem_pthread.dylib        0x1838e3fac _pthread_wqthread + 1176
26 libsystem_pthread.dylib        0x1838e3b08 start_wqthread + 4

For more context around our specific situation:

(1) Also even our users who have this crash are still successfully registered and can receive notifications.

(2) We implement the SDK documentation almost exactly, with pushNotifications.registerDeviceToken (and subscribing) in the application:didRegisterForRemoteNotificationsWithDeviceToken: handler and pushNotifications.start in the application:didFinishLaunchingWithOptions: handler.

One notable difference:

While the documentation recommends registering for notifications in the application:didFinishLaunchingWithOptions: handler, we prefer to wait to request notification authorization from the user until a little later in our new user onboarding flow.

Instead we manually request notification authorization later, using the standard UNUserNotificationCenter documentation. From reading through your source code, it seems to parallel what you do in your pushNotifications.registerForRemoteNotifications method implemented, which we do not call.

My initial hunch:

Since the crash is on serialQueue.resume, my hypothesis is that registering for notifications way later in our flow may affect the suspension of the com.pusher.pushnotifications.sdk dispatch queue done in pushNotifications.start. If so, is there a potential workaround for this?

Otherwise, I was hoping to know if you had any more information to provide that could be helpful around debugging this issue. Thank you!

lukabratos commented 6 years ago

Hi @elizabethsdavis

I'm sorry for the issue that you're experiencing, I'll look into it and get back to you. Many thanks for the very detailed and throughout writeup about the issue.

We're doing some changes related to the synchronisation strategy in the SDK so future version of the SDK might resolve your issue (#72).

In the meantime I would also recommend updating the SDK version to the latest stable version: 0.10.10.

lukabratos commented 6 years ago

I've noticed that you've opened a ticket on the Intercom. I will continue the conversation there.

dansinclair25 commented 5 years ago

I appreciate that this is closed (and old) but I too am experiencing this issue on v1.2.0 - was a solution ever made for this?

As per the original issue, I too am registering the device sometime after the instance is started.

lukabratos commented 5 years ago

Hey @dansinclair25

We're working on a fix for this. It should be fixed in the upcoming release that is coming out soon.

Thanks!