From 83ca8a913b4ab90c8953233f7d59fc2924c5594c Mon Sep 17 00:00:00 2001 From: Aliaksandr Bialiauski Date: Tue, 28 Sep 2021 10:44:06 +0300 Subject: [PATCH 1/3] Fixes race conditions in singleton initialization (#315) --- .../MapboxMobileEvents/MMELocationManager.m | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/Sources/MapboxMobileEvents/MMELocationManager.m b/Sources/MapboxMobileEvents/MMELocationManager.m index 4cd59c36..b61e38c3 100644 --- a/Sources/MapboxMobileEvents/MMELocationManager.m +++ b/Sources/MapboxMobileEvents/MMELocationManager.m @@ -21,7 +21,7 @@ @interface MMELocationManager () @property (nonatomic) id application; -@property (nonatomic) CLLocationManager *locationManager; +@property (nonatomic, readonly) CLLocationManager *locationManager; @property (nonatomic, getter=isUpdatingLocation, readwrite) BOOL updatingLocation; @property (nonatomic) NSDate *backgroundLocationServiceTimeoutAllowedDate; @property (nonatomic) NSTimer *backgroundLocationServiceTimeoutTimer; @@ -38,10 +38,12 @@ - (void)dealloc { } - (CLLocationManager *)locationManager { - if (_locationManager == nil) { - _locationManager = [[MMEDependencyManager sharedManager] locationManagerInstance]; + @synchronized (self) { + if (!_locationManager) { + _locationManager = [[MMEDependencyManager sharedManager] locationManagerInstance]; + } + return _locationManager; } - return _locationManager; } - (instancetype)init { @@ -111,13 +113,15 @@ - (NSString *)accuracyAuthorizationString { #endif - (void)setLocationManager:(CLLocationManager *)locationManager { - if (locationManager == nil) { - _locationManager = locationManager; - } else { - id delegate = _locationManager.delegate; - _locationManager.delegate = nil; - _locationManager = locationManager; - _locationManager.delegate = delegate; + @synchronized (self) { + if (locationManager == nil) { + _locationManager = locationManager; + } else { + id delegate = _locationManager.delegate; + _locationManager.delegate = nil; + _locationManager = locationManager; + _locationManager.delegate = delegate; + } } } From 94c4a9fff45ef5acc64498b308217691c13da4f6 Mon Sep 17 00:00:00 2001 From: Aliaksandr Bialiauski Date: Wed, 29 Sep 2021 08:31:23 +0300 Subject: [PATCH 2/3] Make MMEMetricsManager thread safe MMEMetricsManager is accessed from multiple thread from `sendTurnstileEvent` method. --- .../MapboxMobileEvents/MMEMetricsManager.m | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Sources/MapboxMobileEvents/MMEMetricsManager.m b/Sources/MapboxMobileEvents/MMEMetricsManager.m index bc39c75d..9b09e75b 100644 --- a/Sources/MapboxMobileEvents/MMEMetricsManager.m +++ b/Sources/MapboxMobileEvents/MMEMetricsManager.m @@ -25,6 +25,7 @@ - (void)pushEvent:(MMEEvent *)event; @interface MMEMetricsManager () +@property (nonatomic, readonly, strong) NSRecursiveLock *lock; @property (nonatomic) MMEMetrics *metrics; @end @@ -115,12 +116,14 @@ + (BOOL)createFrameworkMetricsEventDir { - (instancetype)init { if (self = [super init]) { [self resetMetrics]; + _lock = [[NSRecursiveLock alloc] init]; } return self; } - (void)updateMetricsFromEventQueue:(NSArray *)eventQueue { if (eventQueue.count > 0) { + [self.lock lock]; self.metrics.eventCountTotal += eventQueue.count; for (MMEEvent *event in eventQueue) { @@ -128,10 +131,12 @@ - (void)updateMetricsFromEventQueue:(NSArray *)eventQueue { eventCount = [NSNumber numberWithInteger:[eventCount integerValue] + 1]; [self.metrics.eventCountPerType setObject:eventCount forKey:event.name]; } + [self.lock unlock]; } } - (void)updateMetricsFromEventCount:(NSUInteger)eventCount request:(nullable NSURLRequest *)request error:(nullable NSError *)error { + [self.lock lock]; if (request.HTTPBody) { [self updateSentBytes:request.HTTPBody.length]; } @@ -165,72 +170,100 @@ - (void)updateMetricsFromEventCount:(NSUInteger)eventCount request:(nullable NSU [self.metrics.failedRequestsDict setObject:failedRequests forKey:MMEEventKeyFailedRequests]; } + [self.lock unlock]; } - (void)updateEventsFailedCount:(NSUInteger)eventCount { + [self.lock lock]; self.metrics.eventCountFailed += eventCount; + [self.lock unlock]; } - (void)updateSentBytes:(NSUInteger)bytes { + [self.lock lock]; if ([[MMEReachability reachabilityForLocalWiFi] isReachableViaWiFi]) { self.metrics.wifiBytesSent += bytes; } else { self.metrics.cellBytesSent += bytes; } + [self.lock unlock]; } - (void)updateReceivedBytes:(NSUInteger)bytes { + [self.lock lock]; if ([[MMEReachability reachabilityForLocalWiFi] isReachableViaWiFi]) { self.metrics.wifiBytesReceived += bytes; } else { self.metrics.cellBytesReceived += bytes; } + [self.lock unlock]; } - (void)incrementAppWakeUpCount { + [self.lock lock]; self.metrics.appWakeups++; + [self.lock unlock]; } - (void)updateConfigurationJSON:(NSDictionary *)configuration { if (configuration) { + [self.lock lock]; self.metrics.configResponseDict = configuration; + [self.lock unlock]; } } - (void)updateCoordinate:(CLLocationCoordinate2D)coordinate { + [self.lock lock]; if (!self.metrics.deviceLat && !self.metrics.deviceLon) { self.metrics.deviceLat = round(coordinate.latitude*1000)/1000; self.metrics.deviceLon = round(coordinate.longitude*1000)/1000; } + [self.lock unlock]; } - (void)resetMetrics { + [self.lock lock]; self.metrics = [MMEMetrics new]; + [self.lock unlock]; } - (void)incrementLocationsInForeground { + [self.lock lock]; self.metrics.locationsInForeground++; + [self.lock unlock]; } - (void)incrementLocationsInBackground { + [self.lock lock]; self.metrics.locationsInBackground++; + [self.lock unlock]; } - (void)incrementLocationsWithApproximateValues { + [self.lock lock]; self.metrics.locationsWithApproximateValues++; + [self.lock unlock]; } - (void)incrementLocationsDroppedBecauseOfHAF { + [self.lock lock]; self.metrics.locationsDroppedBecauseOfHAF++; + [self.lock unlock]; } - (void)incrementLocationsDroppedDueTimeout { + [self.lock lock]; self.metrics.locationsDroppedDueTimeout++; + [self.lock unlock]; } - (void)incrementLocationsConvertedIntoEvents { + [self.lock lock]; self.metrics.locationsConvertedIntoEvents++; + [self.lock unlock]; } - (NSDictionary *)attributes { + [self.lock lock]; MMEMutableMapboxEventAttributes *attributes = [MMEMutableMapboxEventAttributes dictionary]; if (self.metrics.recordingStarted) { attributes[MMEEventDateUTC] = [MMEDate.iso8601DateOnlyFormatter stringFromDate:self.metrics.recordingStarted]; @@ -269,6 +302,7 @@ - (NSDictionary *)attributes { attributes[MMEEventSDKIdentifier] = NSUserDefaults.mme_configuration.mme_legacyUserAgentBase; attributes[MMEEventSDKVersion] = NSUserDefaults.mme_configuration.mme_legacyHostSDKVersion; attributes[MMEEventKeyUserAgent] = NSUserDefaults.mme_configuration.mme_userAgentString; + [self.lock unlock]; return attributes; } @@ -295,7 +329,9 @@ - (MMEEvent *)loadPendingTelemetryMetricsEvent { } - (MMEEvent *)generateTelemetryMetricsEvent { + [self.lock lock]; NSDate *zeroHour = [self.metrics.recordingStarted mme_startOfTomorrow]; + [self.lock unlock]; NSString *metricsDate = [MMEDate.iso8601DateFormatter stringFromDate:NSDate.date]; MMEEvent *telemetryMetrics = [MMEEvent telemetryMetricsEventWithDateString:metricsDate attributes:self.attributes]; From bfef1c84b80076d48fbbc59f01a0f41f686f27d5 Mon Sep 17 00:00:00 2001 From: Aliaksandr Bialiauski Date: Wed, 29 Sep 2021 09:00:03 +0300 Subject: [PATCH 3/3] Make MMEUniqueIdentifier thread-safe --- Sources/MapboxMobileEvents/MMEUniqueIdentifier.m | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/MapboxMobileEvents/MMEUniqueIdentifier.m b/Sources/MapboxMobileEvents/MMEUniqueIdentifier.m index ad135b28..2bfbdbf6 100644 --- a/Sources/MapboxMobileEvents/MMEUniqueIdentifier.m +++ b/Sources/MapboxMobileEvents/MMEUniqueIdentifier.m @@ -2,6 +2,7 @@ @interface MMEUniqueIdentifier () +@property (nonatomic, strong, readonly) NSLock *lock; @property (nonatomic) NSDate *instanceIDRotationDate; @property (nonatomic) NSString *instanceID; @@ -12,11 +13,13 @@ @implementation MMEUniqueIdentifier - (instancetype)initWithTimeInterval:(NSTimeInterval)timeInterval { if (self = [super init]) { _timeInterval = timeInterval; + _lock = [[NSLock alloc] init]; } return self; } - (NSString *)rollingInstanceIdentifer { + [self.lock lock]; if (self.instanceIDRotationDate && [[NSDate date] timeIntervalSinceDate:self.instanceIDRotationDate] >= 0) { _instanceID = nil; } @@ -24,7 +27,9 @@ - (NSString *)rollingInstanceIdentifer { _instanceID = [[NSUUID UUID] UUIDString]; self.instanceIDRotationDate = [[NSDate date] dateByAddingTimeInterval:self.timeInterval]; } - return _instanceID; + NSString *instanceID = _instanceID; + [self.lock unlock]; + return instanceID; } @end