[3.0.0beta] Crash in MixpanelInstance.track

[RamblinWreck77]

Hey team, seeing a small but growing number of crashes in the 3.0.0beta.

closure #1 in MixpanelInstance.track(event:properties:)

0 | libswiftCore.dylib | 0x383c88 | _swift_release_dealloc | + 24
-- | -- | -- | -- | --
1 | App | 0x8cd1d4 | closure #1 in MixpanelInstance.track(event:properties:) | + 276
2 | App | 0x8cd1d4 | closure #1 in MixpanelInstance.track(event:properties:) | + 276
3 | App | 0x8c58c8 | thunk for @escaping @callee_guaranteed () -> () | + 20
4 | libdispatch.dylib | 0x2a84 | _dispatch_call_block_and_release | + 32
5 | libdispatch.dylib | 0x481c | _dispatch_client_callout | + 20
6 | libdispatch.dylib | 0xc004 | _dispatch_lane_serial_drain | + 620
7 | libdispatch.dylib | 0xcc00 | _dispatch_lane_invoke | + 404
8 | libdispatch.dylib | 0x174bc | _dispatch_workloop_worker_thread | + 764
9 | libsystem_pthread.dylib | 0x37a4 | _pthread_wqthread | + 276

EXC_BAD_ACCESS: KERN_INVALID_ADDRESS

[RamblinWreck77]

For some extra context, this crash always happens 1-2s after a POST to your tracking endpoint.

I haven't looked at the source so I'm spitballing here, but I would guess a closure somewhere isn't fully/explicitly capturing everything it needs to.

[RamblinWreck77]

Since I am apparently incapable of posting an issue without poking around the codebase :P, I just posted a small PR #480 @zihejia

Also, quick question about something else I noticed. In MixpanelInstance line 194 you define:

var trackingQueue: DispatchQueue!

and later in init a ton of stuff happens before

let label = "com.mixpanel.\(self.apiToken)"
trackingQueue = DispatchQueue(label: "\(label).tracking)", qos: .utility)

I have a few concerns, namely that:

1) A lot of stuff happens in that init before trackingQueue gets setup. If anything inside those setups (say, an unexpected error condition) causes a call to to trackingQueue then the app blows up.

2) delegate callbacks like @objc private func applicationDidEnterBackground have direct, unprotected calls to trackingQueue. If iOS does some monkey business and calls those delegate callbacks before you're done initializing (I know this should be rare, but still) then boom goes the app again.

As far as I can tell, the only reason to have trackingQueue be a force-unwrapped var in the first place is to embed the mixpanel API token in DispatchQueueLabel. Is that really worth it? Unless there is some benefit I'm unaware of, it seems the best course of action is to change trackingQueue to:

let trackingQueue: DispatchQueue = DispatchQueue(label: "com.mixpanel.trackingQueue", qos: .utility)

If you're worried about name collisions (which AFAIK is not a problem to iOS) you could make all DispatchQueue's static let's. There shouldn't ever be multiple MixpanelInstance's anyways right?

[foxware00]

@jaredmixpanel I am seeing the same as @RamblinWreck77. It looks like there are some memory/race conditions in the new beta which are causing crashes.

[zihejia]

hi @RamblinWreck77 , I'm very sorry for the late response. You are right, the implicitly unwrapped optionals for trackingQueue is not necessary any more cause we initialize it in the init(). I will also move it ahead of everything else so there will be nothing happen before it's ready. The callback for applicationDidEnterBackground depends on setupListeners() which is also part of the init so it will only be triggered after trackingQueue is initialized.

I will remove the need of using trackingQueue and calling archive() in applicationDidEnterBackground because in the current SQLite persistence, there is no need to do any extra archive. The same to applicationWillTerminate.

Unfortunately we have to support multiple Mixpanel Instances so we can't make it static. However, I don't think trackingQueue being non static is the main culprit for this crash.

Since you mentioned this happens after post, I suspect this is something to do the network request, it is currently part of the trackingQueue. If it is taking too long, it will block the normal track actions in the trackingQueue. In the background state, anything could mess up the memory management. I will try to separate the network request from the trackingQueue to better off the situation. If there are more information regarding the crash, that'd be great.

I will open a PR on top of yours. As always, thank you so much for bringing it up and providing so much help.

[zihejia]

hi @RamblinWreck77 , I'm closing this one now. I believe we have addressed the issues in v3.0.0.beta.6. Please feel free to reopen or open new issues if you find anything else.