mParticle / mparticle-apple-sdk

mParticle Apple SDK
Apache License 2.0
45 stars 66 forks source link

Crash due to a race condition on logging user notification #257

Closed leontiy closed 6 months ago

leontiy commented 6 months ago

I see dozens of crashes a week caused by mParticle SDK. with this and similar stack traces:

1
objc_release
‎libobjc.A.dylib
2
-[MPBackendController logUserNotification:]
‎MPBackendController.mm:2311
3
-[MParticle logNotificationWithUserInfo:behavior:andActionIdentifier:]
‎mParticle.m:1857
4
-[MParticle logNotificationOpenedWithUserInfo:andActionIdentifier:]
‎mParticle.m:1839
5
-[MPAppNotificationHandler userNotificationCenter:didReceiveNotificationResponse:]
‎MPAppNotificationHandler.mm:246
6
-[MParticle userNotificationCenter:didReceiveNotificationResponse:]
‎mParticle.m:1553
7
MParticlePushNotificationService.didReceive
‎:0
8
protocol witness for PushNotificationService.didReceive
...

How it reproduces:

  1. App goes to background for a long time (not killed)
  2. User receives and taps notification
  3. App opens and at the same, concurrently, time attempts to a. on main queue, log user notification with _session property in https://github.com/mParticle/mparticle-apple-sdk/blob/d604e598b74503ff6f675c0989a014a0f266f21d/mParticle-Apple-SDK/MPBackendController.m#L1986 b. on internal message queue, end old session and begin a new one in https://github.com/mParticle/mparticle-apple-sdk/blob/d604e598b74503ff6f675c0989a014a0f266f21d/mParticle-Apple-SDK/MPBackendController.m#L2200

The read on main queue in logUserNotification: method is done without synchronisation, resulting in occasional crashes on notification interactions.

einsteinx2 commented 6 months ago

Thank you for reporting this issue, we'll look into this shortly and update this issue with more information.

Tracking internally as https://go.mparticle.com/work/SQDSDKS-6192

einsteinx2 commented 6 months ago

Should be resolved by https://github.com/mParticle/mparticle-apple-sdk/pull/261

leontiy commented 6 months ago

Thanks @einsteinx2! What release can we expect this fix to land with? 8.21? What's the expected timeline for the release?