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

Fixed crashes due to race conditions with NSMutableDictionary access in AFHTTPRequestSerializer #3526

Merged
merged 3 commits into from
Jun 6, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 24 additions & 6 deletions AFNetworking/AFURLRequestSerialization.m
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ - (NSMutableURLRequest *)requestByFinalizingMultipartFormData;
@interface AFHTTPRequestSerializer ()
@property (readwrite, nonatomic, strong) NSMutableSet *mutableObservedChangedKeyPaths;
@property (readwrite, nonatomic, strong) NSMutableDictionary *mutableHTTPRequestHeaders;
@property (readwrite, nonatomic, strong) dispatch_queue_t requestHeaderModificationQueue;
@property (readwrite, nonatomic, assign) AFHTTPRequestQueryStringSerializationStyle queryStringSerializationStyle;
@property (readwrite, nonatomic, copy) AFQueryStringSerializationBlock queryStringSerialization;
@end
Expand All @@ -208,6 +209,7 @@ - (instancetype)init {
self.stringEncoding = NSUTF8StringEncoding;

self.mutableHTTPRequestHeaders = [NSMutableDictionary dictionary];
self.requestHeaderModificationQueue = dispatch_queue_create("requestHeaderModificationQueue", DISPATCH_QUEUE_CONCURRENT);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a SERIAL queue?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I see that dispatch_barrier_async is used with CONCURRENT queues.


// Accept-Language HTTP Header; see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.4
NSMutableArray *acceptLanguagesComponents = [NSMutableArray array];
Expand Down Expand Up @@ -306,17 +308,27 @@ - (void)setTimeoutInterval:(NSTimeInterval)timeoutInterval {
#pragma mark -

- (NSDictionary *)HTTPRequestHeaders {
return [NSDictionary dictionaryWithDictionary:self.mutableHTTPRequestHeaders];
NSDictionary __block *value;
dispatch_sync(self.requestHeaderModificationQueue, ^{
value = [NSDictionary dictionaryWithDictionary:self.mutableHTTPRequestHeaders];
});
return value;
}

- (void)setValue:(NSString *)value
forHTTPHeaderField:(NSString *)field
{
[self.mutableHTTPRequestHeaders setValue:value forKey:field];
dispatch_barrier_async(self.requestHeaderModificationQueue, ^{
[self.mutableHTTPRequestHeaders setValue:value forKey:field];
});
}

- (NSString *)valueForHTTPHeaderField:(NSString *)field {
return [self.mutableHTTPRequestHeaders valueForKey:field];
NSString __block *value;
dispatch_sync(self.requestHeaderModificationQueue, ^{
value = [self.mutableHTTPRequestHeaders valueForKey:field];
});
return value;
}

- (void)setAuthorizationHeaderFieldWithUsername:(NSString *)username
Expand All @@ -328,7 +340,9 @@ - (void)setAuthorizationHeaderFieldWithUsername:(NSString *)username
}

- (void)clearAuthorizationHeader {
[self.mutableHTTPRequestHeaders removeObjectForKey:@"Authorization"];
dispatch_barrier_async(self.requestHeaderModificationQueue, ^{
[self.mutableHTTPRequestHeaders removeObjectForKey:@"Authorization"];
});
}

#pragma mark -
Expand Down Expand Up @@ -560,15 +574,19 @@ - (instancetype)initWithCoder:(NSCoder *)decoder {
}

- (void)encodeWithCoder:(NSCoder *)coder {
[coder encodeObject:self.mutableHTTPRequestHeaders forKey:NSStringFromSelector(@selector(mutableHTTPRequestHeaders))];
dispatch_sync(self.requestHeaderModificationQueue, ^{
[coder encodeObject:self.mutableHTTPRequestHeaders forKey:NSStringFromSelector(@selector(mutableHTTPRequestHeaders))];
});
[coder encodeInteger:self.queryStringSerializationStyle forKey:NSStringFromSelector(@selector(queryStringSerializationStyle))];
}

#pragma mark - NSCopying

- (instancetype)copyWithZone:(NSZone *)zone {
AFHTTPRequestSerializer *serializer = [[[self class] allocWithZone:zone] init];
serializer.mutableHTTPRequestHeaders = [self.mutableHTTPRequestHeaders mutableCopyWithZone:zone];
dispatch_sync(self.requestHeaderModificationQueue, ^{
serializer.mutableHTTPRequestHeaders = [self.mutableHTTPRequestHeaders mutableCopyWithZone:zone];
});
serializer.queryStringSerializationStyle = self.queryStringSerializationStyle;
serializer.queryStringSerialization = self.queryStringSerialization;

Expand Down
22 changes: 22 additions & 0 deletions Tests/Tests/AFHTTPRequestSerializationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,28 @@ - (void)testThatHTTPHeaderValueCanBeRemoved {
XCTAssertFalse([serializer.HTTPRequestHeaders.allKeys containsObject:headerField]);
}

- (void)testThatHTTPHeaderValueCanBeSetToReferenceCountedStringFromMultipleThreadsWithoutCrashing {
@autoreleasepool {
int dispatchTarget = 1000;
__block int completionCount = 0;
for(int i=0; i<dispatchTarget; i++) {
NSString *nonStaticNonTaggedPointerString = [NSString stringWithFormat:@"%@", [NSDate dateWithTimeIntervalSince1970:i]];
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul);
dispatch_async(queue, ^{

[self.requestSerializer setValue:nonStaticNonTaggedPointerString forHTTPHeaderField:@"FrequentlyUpdatedHeaderField"];

dispatch_sync(dispatch_get_main_queue(), ^{
completionCount++;
});
});
}
while (completionCount < dispatchTarget) {
[[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
}
} // Test succeeds if it does not EXC_BAD_ACCESS when cleaning up the @autoreleasepool
}

#pragma mark - Helper Methods

- (void)testQueryStringFromParameters {
Expand Down