mixpanel / mixpanel-swift

Official iOS (Swift) Tracking Library for Mixpanel Analytics
https://mixpanel.com
Apache License 2.0
434 stars 234 forks source link

Crashes after updating to 3.1.2 #515

Closed ifabijanovic closed 2 years ago

ifabijanovic commented 2 years ago

Hello! We've recently updated the Mixpanel SDK from version 2.8.2 to 3.1.2 and are encountering crashes as we started to roll out the new app version. There are two new crashes which I believe represent the same underlying issue.

The first is very brief without much info:

Crashed: com.mixpanel.45c93e9160d1559cc951522c80f523f9.tracking)
0  libswiftCore.dylib             0x388c9c _swift_release_dealloc + 32
1  Mixpanel                       0x25b8c closure #1 in MixpanelInstance.track(event:properties:) + 951 (MixpanelInstance.swift:951)
2  Mixpanel                       0x1e5e0 thunk for @escaping @callee_guaranteed () -> () + 20 (<compiler-generated>:20)
3  libdispatch.dylib              0x2924 _dispatch_call_block_and_release + 32
4  libdispatch.dylib              0x4670 _dispatch_client_callout + 20
5  libdispatch.dylib              0xbdf4 _dispatch_lane_serial_drain + 672
6  libdispatch.dylib              0xc968 _dispatch_lane_invoke + 392
7  libdispatch.dylib              0x171b8 _dispatch_workloop_worker_thread + 656
8  libsystem_pthread.dylib        0x10f4 _pthread_wqthread + 288
9  libsystem_pthread.dylib        0xe94 start_wqthread + 8

The main thread for these crashes is shown in various different stages, which is expected since the crash happens in an async block. The second one does contain more info:

Crashed: com.mixpanel.45c93e9160d1559cc951522c80f523f9.tracking)
0  libswiftCore.dylib             0x3ad08c swift::_gatherGenericParameterCounts(swift::TargetContextDescriptor<swift::InProcess> const*, __swift::__runtime::llvm::SmallVectorImpl<unsigned int>&, swift::Demangle::__runtime::Demangler&) + 304
1  libswiftCore.dylib             0x3af454 swift::gatherWrittenGenericArgs(swift::TargetMetadata<swift::InProcess> const*, swift::TargetTypeContextDescriptor<swift::InProcess> const*, __swift::__runtime::llvm::SmallVectorImpl<swift::TargetMetadata<swift::InProcess> const*>&, swift::Demangle::__runtime::Demangler&) + 636
2  libswiftCore.dylib             0x37f4e4 swift::_swift_buildDemanglingForMetadata(swift::TargetMetadata<swift::InProcess> const*, swift::Demangle::__runtime::Demangler&) + 784
3  libswiftCore.dylib             0x37bf6c swift::nameForMetadata(swift::TargetMetadata<swift::InProcess> const*, bool) + 396
4  libswiftCore.dylib             0x381c30 getNonNullSrcObject(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*) + 92
5  libswiftCore.dylib             0x382e80 tryCastToObjectiveCClass(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*&, swift::TargetMetadata<swift::InProcess> const*&, bool, bool) + 88
6  libswiftCore.dylib             0x38120c tryCast(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*&, swift::TargetMetadata<swift::InProcess> const*&, bool, bool) + 844
7  libswiftCore.dylib             0x3814dc tryCast(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*&, swift::TargetMetadata<swift::InProcess> const*&, bool, bool) + 1564
8  libswiftCore.dylib             0x3814dc tryCast(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*&, swift::TargetMetadata<swift::InProcess> const*&, bool, bool) + 1564
9  libswiftCore.dylib             0x380db4 swift_dynamicCastImpl(swift::OpaqueValue*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*, swift::DynamicCastFlags) + 72
10 Mixpanel                       0x12f60 static JSONHandler.makeObjectSerializable(_:) + 63 (JSONHandler.swift:63)
11 Mixpanel                       0x13680 static JSONHandler.makeObjectSerializable(_:) + 93 (JSONHandler.swift:93)
12 Mixpanel                       0x13680 static JSONHandler.makeObjectSerializable(_:) + 93 (JSONHandler.swift:93)
13 Mixpanel                       0x12944 static JSONHandler.serializeJSONObject(_:) + 38 (JSONHandler.swift:38)
14 Mixpanel                       0x3ed20 Track.track(event:properties:timedEvents:superProperties:mixpanelIdentity:epochInterval:) + 65 (MixpanelPersistence.swift:65)
15 Mixpanel                       0x25b78 closure #1 in MixpanelInstance.track(event:properties:) + 951 (MixpanelInstance.swift:951)
16 Mixpanel                       0x1e5e0 thunk for @escaping @callee_guaranteed () -> () + 20 (<compiler-generated>:20)
17 libdispatch.dylib              0x2924 _dispatch_call_block_and_release + 32
18 libdispatch.dylib              0x4670 _dispatch_client_callout + 20
19 libdispatch.dylib              0xbdf4 _dispatch_lane_serial_drain + 672
20 libdispatch.dylib              0xc968 _dispatch_lane_invoke + 392
21 libdispatch.dylib              0x171b8 _dispatch_workloop_worker_thread + 656
22 libsystem_pthread.dylib        0x10f4 _pthread_wqthread + 288
23 libsystem_pthread.dylib        0xe94 start_wqthread + 8

It looks as if the event properties contain a dictionary of dictionaries of... something? that crashes. I've checked our git logs and was unable to find any recently added events that would log such a structure. We additionally prune the properties before even calling MixpanelInstance.track(event:properties:) which should prevent any invalid values from making it to the SDK code:

private func validateAndConvertDictionary(_ properties: [String: Any]) throws -> [String: MixpanelType] {
    return try properties.mapValues { value in
        switch value {
        case let array as [Any]: return try array.map { v in
            try validateAndConvertValue(v)
            }
        case let dict as [String: Any]: return try validateAndConvertDictionary(dict)
        default: return try validateAndConvertValue(value)
        }
    }
}

private func validateAndConvertValue(_ value: Any) throws -> MixpanelType {
    if let mixpanelType = value as? MixpanelType {
        return mixpanelType
    }
    guard let mixpanelConvertible = value as? MixpanelTypeConvertible else {
        throw AnalyticsError.unableToConvertParameter(value)
    }
    return try mixpanelConvertible.mixpanelValue()
}

Can you provide any assistance? Is this a known issue already, maybe related to the new persistence layer?

zihejia commented 2 years ago

hi @ifabijanovic , thanks for bringing it up. So you don't have this issue in v2.8.2?

zihejia commented 2 years ago

hi @ifabijanovic , from your crash log, it seems you have stored some Objective-C class type into the dictionary, is that the case? This will help us narrow down the issue.

ifabijanovic commented 2 years ago

hi @ifabijanovic , thanks for bringing it up. So you don't have this issue in v2.8.2?

I am not sure yet, we're rolling back the SDK version as a hot fix and I can report back afterwards. Since the issue started happening in the version of the app where we updated the SDK that is my prime suspect.

hi @ifabijanovic , from your crash log, it seems you have stored some Objective-C class type into the dictionary, is that the case? This will help us narrow down the issue.

It is possible, but if that is the case it seems this worked in v2.8.2. I will add additional logging to try and pinpoint the offending type

ifabijanovic commented 2 years ago

I am unable to reproduce the crash by sending an ObjC instance as part of properties. Something like:

["foo": SomeObjcType()]

is caught by the above pruning code and not even sent to Mixpanel SDK. I could bypass the above pruning code by using a different key type:

[
  "foo": [
    1: SomeObjcType(),
    SomeObjcType(): 2
  ]
]

But that didn't cause the crash either, it went into the "enforcing string on object" default case in JSONHandler

zihejia commented 2 years ago

hi @ifabijanovic , yeah, it's super weird. Looking forward to your result on the old version. Do you use any super properties?

zihejia commented 2 years ago

hi @ifabijanovic , 3.1.4 has been released and it has several fixes for race conditions. I have a good level of confidence this will address the crash. I will close this one and merge this ticket to #495. Feel free to update anything you've got to there.