optimizely / swift-sdk

Swift SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://www.optimizely.com/products/full-stack/
Apache License 2.0
21 stars 30 forks source link

[FSSDK-9389] Crash on `createUserContext` #508

Closed TheNamesJamesW closed 1 year ago

TheNamesJamesW commented 1 year ago

Hi,

First time raising an issue here for Optimizely, so if this issue is being tracked elsewhere, or if should be reported differently, let me know! 😄

We're experiencing a high volume of iOS app crashes at the point of deciding an FF variation, seemingly as we're creating a User Context (as per Decide methods)

We're seeing a stack trace like:

# Crashlytics - Stack trace
# Application: [redacted]
# Platform: apple
# Version: [redacted]
# Issue: [redacted]
# Session: [redacted]
# Date: Wed May 24 2023 11:53:05 GMT+0100 (British Summer Time)

Crashed: com.apple.main-thread
0  [redacted]                     0x7bab30 specialized _dictionaryUpCast<A, B, C, D>(_:) + 4378946352 (<compiler-generated>:4378946352)
1  [redacted]                     0x7bc934 OptimizelyClient.createUserContext(userId:attributes:) + 31 (OptimizelyClient+Decide.swift:31)
2  [redacted]                     0x492ff4 protocol witness for OptimizelyClientWrapper.isEnabled(flagName:visitorId:userInfo:) in conformance OptimizelyClient + 192 (OptimizelyController.swift:192)
...

According to the crashlog and the SDK version (3.10.2*) we're using, the crash happens on OptimizelyClient+Decide.swift Line 31: https://github.com/optimizely/swift-sdk/blob/26c5b02b8889f4ae8d53627018023fc36a11ab9a/Sources/Optimizely%2BDecide/OptimizelyClient%2BDecide.swift#L29-L31

Notably, the parameter of attributes here is [String: Any]?, which is passed to OptimizelyUserContext.init which expects attributes: [String: Any?]?. I wonder if that's the cause of this _dictionaryUpCast(_:) failure?

For those curious, our `OptimizelyClientWrapper.isEnabled(...)` looks like:

```swift extension OptimizelyClient: OptimizelyClientWrapper { func isEnabled(flagName: String, visitorId: String, userInfo: [String: Any]?) -> Bool { let decision = decision( flagName: flagName, visitorId: visitorId, userInfo: userInfo ) return decision.enabled } private func decision(flagName: String, visitorId: String, userInfo: [String: Any]?) -> OptimizelyDecision { let user = createUserContext( userId: visitorId, attributes: userInfo ) return user.decide(key: flagName) } } ```

*It's not a new issue in this SDK version. We've seen similar volumes from previous app releases, using previous Optimizely SDK versions

russell-loube-optimizely commented 1 year ago

Hi @TheNamesJames , we're currently looking into this. I'll provide an update here as soon as I have one.

russell-loube-optimizely commented 1 year ago

Hi @TheNamesJames, we'd appreciate some additional information to enable our troubleshooting. Please find our questions below:

  1. Could you provide some sample(s) type(s) of attributes you are using?
  2. Are you using the SDK asynchronously in any fashion?
  3. Are the crashes consistent for all users, or seemingly intermittent?
  4. Have you been able to reproduce this behavior consistently in your environment?

Thanks in advance for your input.

TheNamesJamesW commented 1 year ago

Thanks Russell,

Great questions. I'll answer those questions in reverse, since each answer relates

Are the crashes consistent for all users, or seemingly intermittent? Have you been able to reproduce this behavior consistently in your environment?

We've not yet been able to identify a pattern of behaviour that would reproduce the crash. This crash isn't happening to all our users but, for those users who do experience this crash, it seems to happen roughly once. To give some insight, we recently saw 3,800 of these crashes affecting 3,700 users, over a 30 day period

Are you using the SDK asynchronously in any fashion?

Upon further inspection of crash logs, yes. The SDK's Decide methods may be called from one of 2 threads: Main thread, or a custom global thread dedicated to Analytics. The majority of the crashes seem to occur on the main thread (but of course, there's no ruling out a data race between the 2 threads, which would be more difficult to detect just from crash logs) Q: I don't see much documentation about thread safety (either in support of or against). Do you recommend something either way?

Could you provide some sample(s) type(s) of attributes you are using?

The values of the userInfo dictionary (and therefore that are passed to OptimizelyClient.createUserContext(userId:attributes:)) are always first cast to String. The userInfo parameter you see in the OptimizelyClientWrapper.isEnabled(...) snippet is a stored variable that is only ever modified like so:

/// `StatisticsEvent` has a property: `parameters: [String: Any]`
func updateUserInfo(with event: StatisticsEvent) {
    let usefulParameters: [String: _] = [
        "learning_language": event.parameters[.learning],
        "interface_language": event.parameters[.interface],
        "app_version": event.parameters[.appVersion],
        "user_role": event.parameters[.userRole]
    ]

    // intentionally ignore `nil`s and non-String values
    let usefulUserInfo = usefulParameters.compactMapValues { $0 as? String }
    for (key, value) in usefulUserInfo { // Both key and value are String here
        optimizelyClientWrapper.userInfo[key] = value
    }
}
jaeopt commented 1 year ago

@TheNamesJames Thanks for the details. The swift sdk is designed to be thread-safe and your code all looks good to me. One thing I want to clarify - any chance of concurrency conflicts between updateUserInfo and isEnabled? If userInfo is updated while being accessed for createUserContext may cause the crash.

jaeopt commented 1 year ago

If you see other stack traces related to the crash, please share them as well.

jaeopt commented 1 year ago

@TheNamesJames this will be closed for now. Let us know to open it again if you see any further issues regarding the topic.