Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datarace using pinpoint identifyUser #3439

Closed
gbitaudeau opened this issue Dec 30, 2023 · 5 comments
Closed

Datarace using pinpoint identifyUser #3439

gbitaudeau opened this issue Dec 30, 2023 · 5 comments
Assignees
Labels
analytics Issues related to the Analytics category bug Something isn't working push-notifications Issues related to the Push Notifications category

Comments

@gbitaudeau
Copy link

Describe the bug

Using both Amplify.Notifications.Push.identifyUser and Amplify.Analytics.identifyUser causes crashes due to datarace.

The documentation is not very clear about which identifyUser I should use so I call both, but in production, a lot of crashes occur.

Steps To Reproduce

Here the code:

let analitycsUserProfile = AnalyticsUserProfile(name: user.username,
                                                email: user.email,
                                                 plan: previousPurchaseStatus?.rawValue,
                                           properties: [
                              "notificationEnabled" : previousNotificationEnabled,
                                             "lang" : Locale.current.identifier,
                                     "lastRateDate" : previousLastRateDate?.timeIntervalSince1970 ?? 0,])
Amplify.Analytics.identifyUser(userId: user.username, userProfile: analitycsUserProfile)

let pushUserProfile = BasicUserProfile(name: user.username,
                                      email: user.email,
                                       plan: previousPurchaseStatus?.rawValue,
                           customProperties: [
                             "notificationEnabled" : previousNotificationEnabled,
                                            "lang" : Locale.current.identifier,
                                            "lastRateDate" : previousLastRateDate?.timeIntervalSince1970 ?? 0,])
try await Amplify.Notifications.Push.identifyUser(userId: user.username, userProfile: pushUserProfile)


### Expected behavior

No crash

### Amplify Framework Version

2.23.0

### Amplify Categories

Analytics

### Dependency manager

Swift PM

### Swift version

5.9.2

### CLI version

12.8.2

### Xcode version

15.1 (15C65)

### Relevant log output

```shell
Thread 14 Crashed:
0   libswiftCore.dylib            	0x00000001a49f9034 swift_isUniquelyReferenced_nonNull_native + 0 (SwiftObject.mm:1375)
1   Whosnext                      	0x000000010362001c specialized Dictionary._Variant.isUniquelyReferenced() + 8 (<compiler-generated>:0)
2   Whosnext                      	0x000000010362001c specialized Dictionary._Variant.setValue(_:forKey:) + 52
3   Whosnext                      	0x0000000103630ef8 specialized PinpointEndpointProfile.setCustomProperty(_:forKey:) + 364 (<compiler-generated>:0)
4   Whosnext                      	0x000000010362f4a8 PinpointEndpointProfile.setCustomProperty(_:forKey:) + 40 (PinpointEndpointProfile.swift:0)
5   Whosnext                      	0x000000010362f4a8 PinpointEndpointProfile.addCustomProperties(_:) + 380
6   Whosnext                      	0x000000010362f01c PinpointEndpointProfile.addUserProfile(_:) + 384 (PinpointEndpointProfile.swift:65)
7   Whosnext                      	0x0000000103642f10 AWSPinpointPushNotificationsPlugin.identifyUser(userId:userProfile:) + 152 (AWSPinpointPushNotificationsPlugin+ClientBehaviour.swift:26)
8   Whosnext                      	0x0000000103644e89 protocol witness for PushNotificationsCategoryBehaviour.identifyUser(userId:userProfile:) in conformance AWSPinpointPushNotificationsPlugin + 1 (<compiler-generated>:0)
9   Whosnext                      	0x000000010278606d PushNotificationsCategory.identifyUser(userId:userProfile:) + 1 (PushNotificationsCategory+ClientBehaviour.swift:13)
10  Whosnext                      	0x000000010259ffd5 AnalyticsManager.updateUser(newUser:) + 1 (AnalyticsManager.swift:187)
11  Whosnext                      	0x000000010259d4a1 closure #1 in closure #1 in AnalyticsManager.startListenTasks() + 1 (AnalyticsManager.swift:150)
12  Whosnext                      	0x00000001025a8d4d partial apply for closure #1 in closure #1 in AnalyticsManager.startListenTasks() + 1
13  Whosnext                      	0x000000010258a579 specialized thunk for @escaping @callee_guaranteed @Sendable @async () -> (@out A) + 1
14  Whosnext                      	0x00000001025a8d91 thunk for @escaping @callee_guaranteed @Sendable @async () -> (@out A)specialized partial apply + 1
15  libswift_Concurrency.dylib    	0x00000001b63f7ac5 completeTaskWithClosure(swift::AsyncContext*, swift::SwiftError*) + 1 (Task.cpp:463)

Is this a regression?

No

Regression additional context

No response

Platforms

iOS

OS Version

iOS 16.6

Device

iPhone / iPad

Specific to simulators

No

Additional context

crashs.zip
3 crashes downloaded from Xcode

@lawmicha lawmicha added bug Something isn't working analytics Issues related to the Analytics category push-notifications Issues related to the Push Notifications category labels Jan 1, 2024
@lawmicha
Copy link
Member

lawmicha commented Jan 1, 2024

Thanks for reporting this, we'll investigate and provide an update as soon as we have more information.

@ruisebas ruisebas self-assigned this Jan 2, 2024
@ruisebas
Copy link
Member

ruisebas commented Jan 2, 2024

Hi @gbitaudeau! Both Notifications.Push.identifyUser and Amplify.Analytics.identifyUser do the exact same thing, which is to update the local endpoint information and send a updateEndpoint API request to Pinpoint.

This is likely what's causing the race condition, so while I work on a fix you can try removing one invocation as a potential quick fix.

@gbitaudeau
Copy link
Author

Hi @ruisebas, I will try and let you know the result. Thanks

@harsh62
Copy link
Member

harsh62 commented Jan 15, 2024

The fix has been released. Please use the latest version of Amplify.

Closing the issue. Reopen if you have more questions.

@harsh62 harsh62 closed this as completed Jan 15, 2024
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analytics Issues related to the Analytics category bug Something isn't working push-notifications Issues related to the Push Notifications category
Projects
None yet
Development

No branches or pull requests

4 participants