Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios] Adds thread safe access to MGLNetworkConfiguration events dictionary #15113

Merged
merged 4 commits into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions platform/darwin/src/MGLNetworkConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ @interface MGLNetworkConfiguration ()
@property (strong) NSURLSessionConfiguration *sessionConfig;
@property (nonatomic, strong) NSMutableDictionary<NSString *, NSDictionary*> *events;
@property (nonatomic, weak) id<MGLNetworkConfigurationMetricsDelegate> metricsDelegate;
@property (nonatomic) dispatch_queue_t eventsQueue;

@end

Expand All @@ -20,6 +21,7 @@ - (instancetype)init {
if (self = [super init]) {
self.sessionConfiguration = nil;
_events = [NSMutableDictionary dictionary];
_eventsQueue = dispatch_queue_create("com.mapbox.network-configuration", DISPATCH_QUEUE_CONCURRENT);
}

return self;
Expand Down Expand Up @@ -65,8 +67,9 @@ - (NSURLSessionConfiguration *)defaultSessionConfiguration {
}

- (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];
}
}

Expand All @@ -82,10 +85,13 @@ - (void)sendEventForURLResponse:(NSURLResponse *)response withAction:(NSString *
{
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];
});
}
}

Expand All @@ -94,7 +100,7 @@ - (void)sendEventForURLResponse:(NSURLResponse *)response withAction:(NSString *
- (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];
Expand Down Expand Up @@ -129,4 +135,28 @@ - (NSDictionary *)eventAttributesForURL:(NSURLResponse *)response withAction:(NS
};
}

#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
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
* Fixed an issue where the two-finger tilt gesture would continue after lifting one finger. ([#14969](https://github.com/mapbox/mapbox-gl-native/pull/14969))
* Added `MGLMapView.compassView.visibility` and `MGLOrnamentVisibility` to allow configuration of compass visibility behavior. ([#15055](https://github.com/mapbox/mapbox-gl-native/pull/15055))
* Fixed a bug where using the pinch gesture could result in an incorrect map center coordinate. ([#15097](https://github.com/mapbox/mapbox-gl-native/pull/15097))
* Fixed a crash during network access. ([#15113](https://github.com/mapbox/mapbox-gl-native/pull/15113))

julianrex marked this conversation as resolved.
Show resolved Hide resolved
## 5.1.0 - June 19, 2019

Expand Down
4 changes: 4 additions & 0 deletions platform/ios/ios.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,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 */; };
Expand Down Expand Up @@ -1198,6 +1199,7 @@
CA5E5042209BDC5F001A8A81 /* MGLTestUtility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = MGLTestUtility.h; path = ../../darwin/test/MGLTestUtility.h; sourceTree = "<group>"; };
CA65C4F721E9BB080068B0D4 /* MGLCluster.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MGLCluster.h; sourceTree = "<group>"; };
CA6914B420E67F50002DB0EE /* MGLAnnotationViewIntegrationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = MGLAnnotationViewIntegrationTests.m; path = "Annotation Tests/MGLAnnotationViewIntegrationTests.m"; sourceTree = "<group>"; };
CA86FF0D22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLNetworkConfigurationTests.m; sourceTree = "<group>"; };
CA88DC2F21C85D900059ED5A /* MGLStyleURLIntegrationTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MGLStyleURLIntegrationTest.m; sourceTree = "<group>"; };
CA8FBC0821A47BB100D1203C /* MGLRendererConfigurationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLRendererConfigurationTests.mm; path = ../../darwin/test/MGLRendererConfigurationTests.mm; sourceTree = "<group>"; };
CAD9D0A922A86D6F001B25EE /* MGLResourceTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = MGLResourceTests.mm; path = ../../darwin/test/MGLResourceTests.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2071,6 +2073,7 @@
357579811D502AD4000B822E /* Styling */,
409F43FB1E9E77D10048729D /* Swift Integration */,
4031ACFD1E9FD26900A3EA26 /* Test Helpers */,
CA86FF0D22D8D5A0009EB14A /* MGLNetworkConfigurationTests.m */,
);
name = "SDK Tests";
path = test;
Expand Down Expand Up @@ -3250,6 +3253,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 */,
Expand Down
43 changes: 43 additions & 0 deletions platform/ios/test/MGLNetworkConfigurationTests.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#import <Mapbox/Mapbox.h>
#import <XCTest/XCTest.h>
#import "MGLNetworkConfiguration_Private.h"

@interface MGLNetworkConfigurationTests : XCTestCase
@end

@implementation MGLNetworkConfigurationTests

friedbunny marked this conversation as resolved.
Show resolved Hide resolved
// 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