mixpanel/mixpanel-iphone

Crash & Data Race in [MixPanel archiveEvents]

Closed this issue · 2 comments

Integration Method: CocoaPods
Xcode Version: 12.2
Library Version: 3.6.3 (also in main)
Platform: iOS
Language: Objective-C
Description: Crash in [MixPanel archiveEvents]
Expected Behavior: No crashes

Our users are getting crashes in [MixPanel archiveEvents]. A quick look in the code seems to reveal a simple data race:

- (void)archiveEvents
{
    NSString *filePath = [self eventsFilePath];
    MPLogInfo(@"%@ archiving events data to %@: %@", self, filePath, self.eventsQueue);
    dispatch_sync(self.archiveQueue, ^{
        NSArray *shadowEventsQueue =  [self.eventsQueue copy];
        if (![self archiveObject:shadowEventsQueue withFilePath:filePath]) {
            MPLogError(@"%@ unable to archive event data", self);
        }
    });
}

self.eventsQueue is copied here on the archiveQueue, while it's also accessed simultaneously on other queues. Without any locking here. [NSArray copy] is not an atomic operation, this needs to be protected by a lock or moved out of the queue.

This is a simple data race. Internally, [NSArray copy] does something like:

NSRange range = NSRangeMake(0, self.count);
[[NSArray alloc] initWithArray:self range:range copyItems:NO];

So, it's easy to see how this results in a data race and a crash: If the queue is flushed on the dispatchOnNetworkQueue in between the self.count and the copy of items, the copy thinks there are more items and thus crashes.

This seems to happen with a lot of other data queue accesses as well, by the way. There seem to be several serial queues accessing the same data, which kind of defeats the point of using serial queues to protect data?

Crashing thread:

Thread 10 Crashed:
0   CoreFoundation                  0x33200c878         <redacted>
1   libobjc.A.dylib                 0x37b41fc50         objc_exception_throw
2   CoreFoundation                  0x33207ce1c         <redacted>
3   CoreFoundation                  0x331eeb3b8         <redacted>
4   SegmentAnalytics                0x103633efc         __25-[Mixpanel archiveEvents]_block_invoke
5   libdispatch.dylib               0x19d25cdb0         <redacted>
6   libdispatch.dylib               0x19d26b428         <redacted>
7   SegmentAnalytics                0x103633e94         -[Mixpanel archiveEvents]
8   SegmentAnalytics                0x10363044c         __29-[Mixpanel track:properties:]_block_invoke
9   libdispatch.dylib               0x19d25b24c         <redacted>
10  libdispatch.dylib               0x19d25cdb0         <redacted>
11  libdispatch.dylib               0x19d26410c         <redacted>
12  libdispatch.dylib               0x19d264c5c         <redacted>
13  libdispatch.dylib               0x19d26ed78         <redacted>
14  libsystem_pthread.dylib         0x368709804         _pthread_wqthread
15  libsystem_pthread.dylib         0x36871075c         start_wqthread

Other thread accessing the eventsQueue:

Thread 2
0   libobjc.A.dylib                 0x37b41b11c         objc_msgSend
1   CFNetwork                       0x31e923d18         __CFTubeSetTubeTypeNotifier
2   CFNetwork                       0x31e77cf9c         _CFURLConnectionCopyTimingData
3   CFNetwork                       0x31e77cb90         _CFURLConnectionCopyTimingData
4   CFNetwork                       0x31e9174f0         __CFTubeSetTubeTypeNotifier
5   CFNetwork                       0x31e9196b8         __CFTubeSetTubeTypeNotifier
6   CFNetwork                       0x31e6e6578         _CFHTTPMessageSetResponseProxyURL
7   CFNetwork                       0x31e6d2c7c         CFNetServiceBrowserSearchForServices
8   SegmentAnalytics                0x10364a3bc         -[MPNetwork flushQueue:endpoint:]
9   SegmentAnalytics                0x103633afc         __32-[Mixpanel flushWithCompletion:]_block_invoke
10  libdispatch.dylib               0x19d25b24c         <redacted>
11  libdispatch.dylib               0x19d25cdb0         <redacted>
12  libdispatch.dylib               0x19d26410c         <redacted>
13  libdispatch.dylib               0x19d264c5c         <redacted>
14  libdispatch.dylib               0x19d26ed78         <redacted>
15  libsystem_pthread.dylib         0x368709804         _pthread_wqthread

It seems that #912 is related to this very same issue.

Other archive* methods seem to follow the same pattern, like archiveGroups.

We've also got crashes on [Mixpanel archiveProperties], which follows the same code pattern:

OS Version: iOS 14.3 (18C66)
Report Version: 104

Exception Type: EXC_BAD_ACCESS (SIGBUS)
Exception Codes: BUS_NOOP at 0x0000bfbb209ea760
Crashed Thread: 0

Application Specific Information:
emplate:columnSpan:showAccessoryText: > replacementObjectForKeyedArchiver: >
Attempted to dereference garbage pointer 0xbfbb209ea760.

Thread 0 Crashed:
0   libobjc.A.dylib                 0x3863e00e8         objc_msgSend
1   Foundation                      0x326eb010c         <redacted>
2   Foundation                      0x326db8534         <redacted>
3   Foundation                      0x326db5c58         <redacted>
4   Foundation                      0x326eb04f4         <redacted>
5   Foundation                      0x326db8534         <redacted>
6   Foundation                      0x326ddc8bc         <redacted>
7   Foundation                      0x326eb04f4         <redacted>
8   Foundation                      0x326dff260         <redacted>
9   SegmentAnalytics                0x102038b10         -[Mixpanel archiveObject:withFilePath:]
10  SegmentAnalytics                0x102038688         __29-[Mixpanel archiveProperties]_block_invoke
11  libdispatch.dylib               0x1a5445db0         <redacted>
12  libdispatch.dylib               0x1a5454840         <redacted>
13  libdispatch.dylib               0x1a54542cc         <redacted>
14  SegmentAnalytics                0x1020382d8         -[Mixpanel archiveProperties]
15  libdispatch.dylib               0x1a544424c         <redacted>
16  libdispatch.dylib               0x1a5445db0         <redacted>
17  libdispatch.dylib               0x1a54537ac         _dispatch_main_queue_callback_4CF
18  CoreFoundation                  0x33ab8b11c         <redacted>
19  CoreFoundation                  0x33ab85120         <redacted>
20  CoreFoundation                  0x33ab8421c         CFRunLoopRunSpecific
21  GraphicsServices                0x376f31784         GSEventRunModal
22  UIKitCore                       0x331431fe0         <redacted>
23  UIKitCore                       0x331437854         UIApplicationMain
24  bunq                            0x201292534         main (main.m:17)
25  libdyld.dylib                   0x349f616b0         <redacted>

Hi @AndreasVerhoeven and @lmmenge. Thank you so much for raising it and providing great insight, we made a new release to fix this issue by adding more write access protection. https://github.com/mixpanel/mixpanel-iphone/releases/tag/v3.6.4. I'm closing this issue now, please feel free to reopen it if the problem still exists.