SentryCrash: EXC_BREAKPOINT accessing ServiceLocator property

[sentry-io[bot]]

Sentry Issue: WOOCOMMERCE-IOS-1CFK

EXC_BREAKPOINT: Exception 6, Code 2149166540, Subcode 8
  File "Classes/ServiceLocator/ServiceLocator.swift", line 18, in ServiceLocator._stores.unsafeMutableAddressor
  File "Classes/ServiceLocator/ServiceLocator.swift", line 83, in ServiceLocator.stores.getter
  File "<compiler-generated>", line 89, in PushNotificationsConfiguration.default.getter
  File "Classes/Notifications/PushNotificationsConfiguration.swift", line 25, in PushNotificationsConfiguration.storesManager.getter
  File "PushNotificationsManager.swift", line 435, in PushNotificationsManager.unregisterDotcomDevice
...
(75 additional frame(s) were not displayed)

Seems like a race condition when accessing ServiceLocator properties

created by @ecarrion

[Ecarrion]

17 events, 3 users as of December 17 of 2020

[sentry-io[bot]]

Sentry issue: WOOCOMMERCE-IOS-1D7Y

[shiki]

Could be related to #2370.

[joshheald]

Prioritization: Severity – high: crash Impact – Low: 146/131k (30 days) = medium

One of our bigger crashes, and could affect anywhere in the app.

[jaclync]

Cannot find the crash in Sentry, but it's the top crash in Xcode Organizer:

312 devices in the last 2 weeks.

[jaclync]

As in https://github.com/woocommerce/woocommerce-ios/pull/11711#issuecomment-1897789179, the attempted alleviation still results in a following one-time crash that seems to be caused by a circular reference of:

WooAnalytics.refreshUserData --> DefaultStoresManager.deauthenticate --> WooAnalytics.refreshUserData again with the ServiceLocator static singleton dance.

We can consider moving the deauthentication calls from DefaultStoresManager.deauthenticate to AppDelegate (and any other ServiceLocator singleton references in DefaultStoresManager) to fix such circular references. In DefaultStoresManager.deauthenticate, it already posts a notification via the Notification Center https://github.com/woocommerce/woocommerce-ios/blob/f3786999c9f2cecb7f78486657ab4653d7323cd0/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift#L252

maybe we can use that notification to tear down the other dependencies from the ServiceLocator in a difference place like AppDelegate:

https://github.com/woocommerce/woocommerce-ios/blob/f3786999c9f2cecb7f78486657ab4653d7323cd0/WooCommerce/Classes/Yosemite/DefaultStoresManager.swift#L245-L248

[jaclync]

In Sentry, there are still 6 related issues from the same or different entry point (one of the global static singletons with circular reference) with ServiceLocator._stores.unsafeMutableAddressor error:

• WooAnalytics.refreshUserData - 15 users, 157 events all time
• WooAnalytics.refreshUserData - 12 users, 57 events all time
• setupPushNotificationsManagerIfPossible - 76 users, 78 events all time
• WooAnalytics.refreshUserData - 3 users, 117 events all time
• setupPushNotificationsManagerIfPossible - 1 user, 15 events all time
• WooAnalytics.refreshUserData - 11 users, 639 events all time

The error code was slightly different in these issues, so I didn't merge them but it's probably safe to think they are the same issue from different entry points.

It's pretty clear that two entry points WooAnalytics.refreshUserData and setupPushNotificationsManagerIfPossible in the app delegate setup would be the main focus to tackle the circular reference issue. I plan to give it a try in the upcoming HACK Week next week.