From abb1ace09c5e1ea701a76184a0b9d1a9c72d327e Mon Sep 17 00:00:00 2001 From: Julian Rex Date: Mon, 15 Jul 2019 21:11:17 -0400 Subject: Cherry picks #15113 - Adds thread safe access to MGLNetworkConfiguration events dictionary --- platform/darwin/src/MGLNetworkConfiguration.m | 42 +++++++++++++++++++---- platform/ios/CHANGELOG.md | 6 ++++ platform/ios/ios.xcodeproj/project.pbxproj | 4 +++ platform/ios/test/MGLNetworkConfigurationTests.m | 43 ++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 platform/ios/test/MGLNetworkConfigurationTests.m diff --git a/platform/darwin/src/MGLNetworkConfiguration.m b/platform/darwin/src/MGLNetworkConfiguration.m index 40993c97b5..0e5046e7a3 100644 --- a/platform/darwin/src/MGLNetworkConfiguration.m +++ b/platform/darwin/src/MGLNetworkConfiguration.m @@ -11,6 +11,7 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; @property (strong) NSURLSessionConfiguration *sessionConfig; @property (nonatomic, strong) NSMutableDictionary *events; @property (nonatomic, weak) id metricsDelegate; +@property (nonatomic) dispatch_queue_t eventsQueue; @end @@ -20,6 +21,7 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; if (self = [super init]) { self.sessionConfiguration = nil; _events = [NSMutableDictionary dictionary]; + _eventsQueue = dispatch_queue_create("com.mapbox.network-configuration", DISPATCH_QUEUE_CONCURRENT); } return self; @@ -65,8 +67,9 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; } - (void)startDownloadEvent:(NSString *)urlString type:(NSString *)resourceType { - if (urlString && ![self.events objectForKey:urlString]) { - [self.events setObject:@{ MGLStartTime: [NSDate date], MGLResourceType: resourceType } forKey:urlString]; + if (urlString && ![self eventDictionaryForKey:urlString]) { + NSDate *startDate = [NSDate date]; + [self setEventDictionary:@{ MGLStartTime: startDate, MGLResourceType: resourceType } forKey:urlString]; } } @@ -82,10 +85,13 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; { if ([response isKindOfClass:[NSURLResponse class]]) { NSString *urlString = response.URL.relativePath; - if (urlString && [self.events objectForKey:urlString]) { + if (urlString && [self eventDictionaryForKey:urlString]) { NSDictionary *eventAttributes = [self eventAttributesForURL:response withAction:action]; - [self.metricsDelegate networkConfiguration:self didGenerateMetricEvent:eventAttributes]; - [self.events removeObjectForKey:urlString]; + [self removeEventDictionaryForKey:urlString]; + + dispatch_async(dispatch_get_main_queue(), ^{ + [self.metricsDelegate networkConfiguration:self didGenerateMetricEvent:eventAttributes]; + }); } } @@ -94,7 +100,7 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; - (NSDictionary *)eventAttributesForURL:(NSURLResponse *)response withAction:(NSString *)action { NSString *urlString = response.URL.relativePath; - NSDictionary *parameters = [self.events objectForKey:urlString]; + NSDictionary *parameters = [self eventDictionaryForKey:urlString]; NSDate *startDate = [parameters objectForKey:MGLStartTime]; NSDate *endDate = [NSDate date]; NSTimeInterval elapsedTime = [endDate timeIntervalSinceDate:startDate]; @@ -129,4 +135,28 @@ NSString * const kMGLDownloadPerformanceEvent = @"mobile.performance_trace"; }; } +#pragma mark - Events dictionary access + +- (nullable NSDictionary*)eventDictionaryForKey:(nonnull NSString*)key { + __block NSDictionary *dictionary; + + dispatch_sync(self.eventsQueue, ^{ + dictionary = [self.events objectForKey:key]; + }); + + return dictionary; +} + +- (void)setEventDictionary:(nonnull NSDictionary*)dictionary forKey:(nonnull NSString*)key { + dispatch_barrier_async(self.eventsQueue, ^{ + [self.events setObject:dictionary forKey:key]; + }); +} + +- (void)removeEventDictionaryForKey:(nonnull NSString*)key { + dispatch_barrier_async(self.eventsQueue, ^{ + [self.events removeObjectForKey:key]; + }); +} + @end diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 1dd7e7d946..979aa09acf 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -2,6 +2,12 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started. +## 5.1.2 + +### Other changes + +* Fixed a crash during network access. ([#15113](https://github.com/mapbox/mapbox-gl-native/pull/15113)) + ## 5.1.1 - July 18, 2019 * Fixed a bug in telemetry collection. ([#15077](https://github.com/mapbox/mapbox-gl-native/pull/15077)) diff --git a/platform/ios/ios.xcodeproj/project.pbxproj b/platform/ios/ios.xcodeproj/project.pbxproj index cbeeebd7bc..807fb988f8 100644 --- a/platform/ios/ios.xcodeproj/project.pbxproj +++ b/platform/ios/ios.xcodeproj/project.pbxproj @@ -506,6 +506,7 @@ CA6914B520E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */; }; CA7766832229C10E0008DE9E /* MGLCompactCalloutView.m in Sources */ = {isa = PBXBuildFile; fileRef = DA8848451CBAFB9800AB86E3 /* MGLCompactCalloutView.m */; }; CA7766842229C11A0008DE9E /* SMCalloutView.m in Sources */ = {isa = PBXBuildFile; fileRef = DA88488A1CBB037E00AB86E3 /* SMCalloutView.m */; }; + CA86FF0E22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CA86FF0D22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m */; }; CA88DC3021C85D900059ED5A /* MGLStyleURLIntegrationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = CA88DC2F21C85D900059ED5A /* MGLStyleURLIntegrationTest.m */; }; CA8FBC0921A47BB100D1203C /* MGLRendererConfigurationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CA8FBC0821A47BB100D1203C /* MGLRendererConfigurationTests.mm */; }; CAA69DA4206DCD0E007279CD /* Mapbox.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DA4A26961CB6E795000B7809 /* Mapbox.framework */; }; @@ -1179,6 +1180,7 @@ CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = ""; }; CA65C4F721E9BB080068B0D4 /* MGLCluster.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCluster.h; sourceTree = ""; }; CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationViewIntegrationTests.m; path = "Annotation Tests/MGLAnnotationViewIntegrationTests.m"; sourceTree = ""; }; + CA86FF0D22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLNetworkConfigurationTests.m; sourceTree = ""; }; CA88DC2F21C85D900059ED5A /* MGLStyleURLIntegrationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleURLIntegrationTest.m; sourceTree = ""; }; CA8FBC0821A47BB100D1203C /* MGLRendererConfigurationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLRendererConfigurationTests.mm; path = ../../darwin/test/MGLRendererConfigurationTests.mm; sourceTree = ""; }; CAD9D0A922A86D6F001B25EE /* MGLResourceTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLResourceTests.mm; path = ../../darwin/test/MGLResourceTests.mm; sourceTree = ""; }; @@ -2043,6 +2045,7 @@ 357579811D502AD4000B822E /* Styling */, 409F43FB1E9E77D10048729D /* Swift Integration */, 4031ACFD1E9FD26900A3EA26 /* Test Helpers */, + CA86FF0D22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m */, ); name = "SDK Tests"; path = test; @@ -3216,6 +3219,7 @@ DA2DBBCE1D51E80400D38FF9 /* MGLStyleLayerTests.m in Sources */, DA35A2C61CCA9F8300E826B2 /* MGLCompassDirectionFormatterTests.m in Sources */, DAE7DEC21E245455007505A6 /* MGLNSStringAdditionsTests.m in Sources */, + CA86FF0E22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m in Sources */, 4085AF091D933DEA00F11B22 /* MGLTileSetTests.mm in Sources */, DAEDC4341D603417000224FF /* MGLAttributionInfoTests.m in Sources */, 1F7454A91ED08AB400021D39 /* MGLLightTest.mm in Sources */, diff --git a/platform/ios/test/MGLNetworkConfigurationTests.m b/platform/ios/test/MGLNetworkConfigurationTests.m new file mode 100644 index 0000000000..bfb63f57af --- /dev/null +++ b/platform/ios/test/MGLNetworkConfigurationTests.m @@ -0,0 +1,43 @@ +#import +#import +#import "MGLNetworkConfiguration_Private.h" + +@interface MGLNetworkConfigurationTests : XCTestCase +@end + +@implementation MGLNetworkConfigurationTests + +// Regression test for https://github.com/mapbox/mapbox-gl-native/issues/14982 +- (void)testAccessingEventsFromMultipleThreads { + MGLNetworkConfiguration *configuration = [[MGLNetworkConfiguration alloc] init]; + + // Concurrent + dispatch_queue_t queue = dispatch_queue_create("com.mapbox.testAccessingEventsFromMultipleThreads", DISPATCH_QUEUE_CONCURRENT); + + NSUInteger numberOfConcurrentBlocks = 20; + + XCTestExpectation *expectation = [self expectationWithDescription:@"wait-for-threads"]; + expectation.expectedFulfillmentCount = numberOfConcurrentBlocks; + + for (NSUInteger i = 0; i < numberOfConcurrentBlocks; i++) { + + NSString *event = [NSString stringWithFormat:@"test://event-%ld", i]; + NSString *resourceType = @"test"; + + dispatch_async(queue, ^{ + [configuration startDownloadEvent:event type:resourceType]; + + NSURL *url = [NSURL URLWithString:event]; + NSURLResponse *response = [[NSURLResponse alloc] initWithURL:url MIMEType:nil expectedContentLength:0 textEncodingName:nil]; + + [configuration stopDownloadEventForResponse:response]; + + dispatch_async(dispatch_get_main_queue(), ^{ + [expectation fulfill]; + }); + }); + } + + [self waitForExpectations:@[expectation] timeout:10.0]; +} +@end -- cgit v1.2.1