mixpanel / mixpanel-iphone

Official iOS (Objective-C) Tracking Library for Mixpanel Analytics
http://mixpanel.com
Apache License 2.0
1.04k stars 567 forks source link

Concurrency Issue with TimedEvents #926

Closed timbo24 closed 3 years ago

timbo24 commented 3 years ago

Integration Method: Carthage Xcode Version: 12.3 Library Version: 3.6.2 Platform: iOS Language: Objective-C Description: Race condition when calling eventElapsedTime Expected Behavior: Not crashing

We're seeing crashes due to the timedEvents property inside of Mixpanel.m not being thread safe. In all other instances, outside of initialization, accesses to timedEvents is wrapped and dispatched to the serialQueue but not in eventElapsedTime.

Suggested change:

NSNumber *startTime = self.timedEvents[event];

->

__block NSNumber *startTime;

dispatch_sync(self.serialQueue, ^{
    startTime = self.timedEvents[event];
});
timbo24 commented 3 years ago

PR open https://github.com/mixpanel/mixpanel-iphone/pull/927

zihejia commented 3 years ago

Thank you so much @timbo24 , your PR has been approved and merged, it will be included in the next release!

zihejia commented 3 years ago

We have released it! https://github.com/mixpanel/mixpanel-iphone/releases/tag/v3.7.1 Thanks again! I'm closing this issue for now.

timbo24 commented 3 years ago

Thank you!