segment-integrations / analytics-ios-integration-mixpanel

The Mixpanel analytics-ios integration.
https://segment.com/docs/integrations/mixpanel
MIT License
7 stars 28 forks source link

Issues with Main Thread Sanitizer #26

Open pixelmatrix opened 7 years ago

pixelmatrix commented 7 years ago

Our app is reporting that Segment-Mixpanel is accessing UI code from the wrong thread in Xcode 9. Here's the stack trace it gives:

PID: 9001, TID: 6833697, Thread name: (none), Queue name: io.segment.analytics, QoS: 0
Backtrace:
4   Mixpanel                            0x00000001124ad894 -[Mixpanel setupAutomaticPushTracking] + 84
5   Mixpanel                            0x00000001124aa248 -[Mixpanel initWithToken:launchOptions:andFlushInterval:] + 2040
6   Mixpanel                            0x00000001124a9111 +[Mixpanel sharedInstanceWithToken:launchOptions:] + 241
7   Segment_Mixpanel                    0x00000001133bd11c -[SEGMixpanelIntegration initWithSettings:andLaunchOptions:] + 316
8   Segment_Mixpanel                    0x00000001133c01b8 -[SEGMixpanelIntegrationFactory createWithSettings:forAnalytics:] + 168
9   Analytics                           0x000000010fd85733 __57-[SEGIntegrationsManager updateIntegrationsWithSettings:]_block_invoke + 579
10  Analytics                           0x000000010fd7d2f1 __seg_dispatch_specific_block_invoke + 49
11  Analytics                           0x000000010fd7d23e seg_dispatch_specific + 206
12  Analytics                           0x000000010fd7d435 seg_dispatch_specific_sync + 85
13  Analytics                           0x000000010fd854b0 -[SEGIntegrationsManager updateIntegrationsWithSettings:] + 176
14  Analytics                           0x000000010fd853c6 -[SEGIntegrationsManager setCachedSettings:] + 278
15  Analytics                           0x000000010fd85ebd __41-[SEGIntegrationsManager refreshSettings]_block_invoke_3 + 77
16  Analytics                           0x000000010fd7d2f1 __seg_dispatch_specific_block_invoke + 49
17  libdispatch.dylib                   0x000000011adc2273 _dispatch_call_block_and_release + 12
18  libdispatch.dylib                   0x000000011adc32b5 _dispatch_client_callout + 8
19  libdispatch.dylib                   0x000000011adcae3f _dispatch_queue_serial_drain + 654
20  libdispatch.dylib                   0x000000011adcb690 _dispatch_queue_invoke + 329
21  libdispatch.dylib                   0x000000011adcda72 _dispatch_root_queue_drain + 568
22  libdispatch.dylib                   0x000000011adcd7dc _dispatch_worker_thread3 + 119
23  libsystem_pthread.dylib             0x000000011b2821ca _pthread_wqthread + 1387
24  libsystem_pthread.dylib             0x000000011b281c4d start_wqthread + 13

Seems the issue is that SEGIntegrationsManager updateIntegrationsWithSettings is using it's own queue, which eventually hits Mixpanel's initialization code, which accesses UIApplication.shared. delegate.

I'm honestly not sure which project the fix should belong to, but I wanted to raise the issue since we've been seeing it every time the app launches.

f2prateek commented 7 years ago

Thanks! I think the fix should be in the integration. Looks like Mixpanel needs to be initialized on the main thread, so the integration should queue calls to the main thread.

Surprised we haven't noticed this before though.

f2prateek commented 7 years ago

cc @ladanazita

pixelmatrix commented 7 years ago

No problem. We only noticed because Xcode 9 now throws warnings for this scenario.

jx2 commented 6 years ago

@f2prateek @ladanazita Any update on when this might be fixed, and an updated version released?

ladanazita commented 6 years ago

Hi @jx2 and @pixelmatrix ,

Sorry for the delay here. I'm going to track this internally and add this to our list to fix — I can't give an ETA at the moment but will update as soon as I have more.

Thanks for your patience! Ladan

jx2 commented 6 years ago

Thanks for the info - much appreciated!

jx2 commented 6 years ago

Hi @ladanazita. Do you know when this might be resolved?

ladanazita commented 6 years ago

@jx2 Hey John, Sorry for the delay here. It is still in our backlog at the moment. We haven't had any further reports on this, so it hasn't been prioritized.

I will keep you updated as soon as I have more.

Thanks for your patience