Ben Copsey

More care with ASIDownloadCache locks

Fix lock regression in ASIHTTPRequest
@@ -69,10 +69,12 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -69,10 +69,12 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
69 for (NSString *directory in directories) { 69 for (NSString *directory in directories) {
70 BOOL exists = [[NSFileManager defaultManager] fileExistsAtPath:directory isDirectory:&isDirectory]; 70 BOOL exists = [[NSFileManager defaultManager] fileExistsAtPath:directory isDirectory:&isDirectory];
71 if (exists && !isDirectory) { 71 if (exists && !isDirectory) {
  72 + [[self accessLock] unlock];
72 [NSException raise:@"FileExistsAtCachePath" format:@"Cannot create a directory for the cache at '%@', because a file already exists",directory]; 73 [NSException raise:@"FileExistsAtCachePath" format:@"Cannot create a directory for the cache at '%@', because a file already exists",directory];
73 } else if (!exists) { 74 } else if (!exists) {
74 [[NSFileManager defaultManager] createDirectoryAtPath:directory attributes:nil]; 75 [[NSFileManager defaultManager] createDirectoryAtPath:directory attributes:nil];
75 if (![[NSFileManager defaultManager] fileExistsAtPath:directory]) { 76 if (![[NSFileManager defaultManager] fileExistsAtPath:directory]) {
  77 + [[self accessLock] unlock];
76 [NSException raise:@"FailedToCreateCacheDirectory" format:@"Failed to create a directory for the cache at '%@'",directory]; 78 [NSException raise:@"FailedToCreateCacheDirectory" format:@"Failed to create a directory for the cache at '%@'",directory];
77 } 79 }
78 } 80 }
@@ -138,21 +140,26 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -138,21 +140,26 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
138 140
139 - (NSDictionary *)cachedHeadersForRequest:(ASIHTTPRequest *)request 141 - (NSDictionary *)cachedHeadersForRequest:(ASIHTTPRequest *)request
140 { 142 {
  143 + [[self accessLock] lock];
141 if (![self storagePath]) { 144 if (![self storagePath]) {
  145 + [[self accessLock] unlock];
142 return nil; 146 return nil;
143 } 147 }
144 // Look in the session store 148 // Look in the session store
145 NSString *path = [[self storagePath] stringByAppendingPathComponent:sessionCacheFolder]; 149 NSString *path = [[self storagePath] stringByAppendingPathComponent:sessionCacheFolder];
146 NSString *dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]]; 150 NSString *dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]];
147 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) { 151 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) {
  152 + [[self accessLock] unlock];
148 return [NSDictionary dictionaryWithContentsOfFile:dataPath]; 153 return [NSDictionary dictionaryWithContentsOfFile:dataPath];
149 } 154 }
150 // Look in the permanent store 155 // Look in the permanent store
151 path = [[self storagePath] stringByAppendingPathComponent:permanentCacheFolder]; 156 path = [[self storagePath] stringByAppendingPathComponent:permanentCacheFolder];
152 dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]]; 157 dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]];
153 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) { 158 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) {
  159 + [[self accessLock] unlock];
154 return [NSDictionary dictionaryWithContentsOfFile:dataPath]; 160 return [NSDictionary dictionaryWithContentsOfFile:dataPath];
155 } 161 }
  162 + [[self accessLock] unlock];
156 return nil; 163 return nil;
157 } 164 }
158 165
@@ -167,52 +174,66 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -167,52 +174,66 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
167 174
168 - (NSString *)pathToCachedResponseDataForRequest:(ASIHTTPRequest *)request 175 - (NSString *)pathToCachedResponseDataForRequest:(ASIHTTPRequest *)request
169 { 176 {
  177 + [[self accessLock] lock];
170 if (![self storagePath]) { 178 if (![self storagePath]) {
  179 + [[self accessLock] unlock];
171 return nil; 180 return nil;
172 } 181 }
173 // Look in the session store 182 // Look in the session store
174 NSString *path = [[self storagePath] stringByAppendingPathComponent:sessionCacheFolder]; 183 NSString *path = [[self storagePath] stringByAppendingPathComponent:sessionCacheFolder];
175 NSString *dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cacheddata"]]; 184 NSString *dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cacheddata"]];
176 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) { 185 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) {
  186 + [[self accessLock] unlock];
177 return dataPath; 187 return dataPath;
178 } 188 }
179 // Look in the permanent store 189 // Look in the permanent store
180 path = [[self storagePath] stringByAppendingPathComponent:permanentCacheFolder]; 190 path = [[self storagePath] stringByAppendingPathComponent:permanentCacheFolder];
181 dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cacheddata"]]; 191 dataPath = [path stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cacheddata"]];
182 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) { 192 if ([[NSFileManager defaultManager] fileExistsAtPath:dataPath]) {
  193 + [[self accessLock] unlock];
183 return dataPath; 194 return dataPath;
184 } 195 }
  196 + [[self accessLock] unlock];
185 return nil; 197 return nil;
186 } 198 }
187 199
188 - (void)removeCachedDataForRequest:(ASIHTTPRequest *)request 200 - (void)removeCachedDataForRequest:(ASIHTTPRequest *)request
189 { 201 {
  202 + [[self accessLock] lock];
190 if (![self storagePath]) { 203 if (![self storagePath]) {
  204 + [[self accessLock] unlock];
191 return; 205 return;
192 } 206 }
193 NSString *cachedHeadersPath = [[self storagePath] stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]]; 207 NSString *cachedHeadersPath = [[self storagePath] stringByAppendingPathComponent:[[[self class] keyForRequest:request] stringByAppendingPathExtension:@"cachedheaders"]];
194 if (!cachedHeadersPath) { 208 if (!cachedHeadersPath) {
  209 + [[self accessLock] unlock];
195 return; 210 return;
196 } 211 }
197 NSString *dataPath = [self pathToCachedResponseDataForRequest:request]; 212 NSString *dataPath = [self pathToCachedResponseDataForRequest:request];
198 if (!dataPath) { 213 if (!dataPath) {
  214 + [[self accessLock] unlock];
199 return; 215 return;
200 } 216 }
201 [[NSFileManager defaultManager] removeItemAtPath:cachedHeadersPath error:NULL]; 217 [[NSFileManager defaultManager] removeItemAtPath:cachedHeadersPath error:NULL];
202 [[NSFileManager defaultManager] removeItemAtPath:dataPath error:NULL]; 218 [[NSFileManager defaultManager] removeItemAtPath:dataPath error:NULL];
  219 + [[self accessLock] unlock];
203 } 220 }
204 221
205 - (BOOL)isCachedDataCurrentForRequest:(ASIHTTPRequest *)request 222 - (BOOL)isCachedDataCurrentForRequest:(ASIHTTPRequest *)request
206 { 223 {
  224 + [[self accessLock] lock];
207 if (![self storagePath]) { 225 if (![self storagePath]) {
  226 + [[self accessLock] unlock];
208 return NO; 227 return NO;
209 } 228 }
210 NSDictionary *cachedHeaders = [self cachedHeadersForRequest:request]; 229 NSDictionary *cachedHeaders = [self cachedHeadersForRequest:request];
211 if (!cachedHeaders) { 230 if (!cachedHeaders) {
  231 + [[self accessLock] unlock];
212 return NO; 232 return NO;
213 } 233 }
214 NSString *dataPath = [self pathToCachedResponseDataForRequest:request]; 234 NSString *dataPath = [self pathToCachedResponseDataForRequest:request];
215 if (!dataPath) { 235 if (!dataPath) {
  236 + [[self accessLock] unlock];
216 return NO; 237 return NO;
217 } 238 }
218 239
@@ -222,6 +243,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -222,6 +243,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
222 NSString *expires = [cachedHeaders objectForKey:@"Expires"]; 243 NSString *expires = [cachedHeaders objectForKey:@"Expires"];
223 if (expires) { 244 if (expires) {
224 if ([[ASIHTTPRequest dateFromRFC1123String:expires] timeIntervalSinceNow] < 0) { 245 if ([[ASIHTTPRequest dateFromRFC1123String:expires] timeIntervalSinceNow] < 0) {
  246 + [[self accessLock] unlock];
225 return NO; 247 return NO;
226 } 248 }
227 } 249 }
@@ -242,6 +264,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -242,6 +264,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
242 #endif 264 #endif
243 265
244 if ([expiryDate timeIntervalSinceNow] < 0) { 266 if ([expiryDate timeIntervalSinceNow] < 0) {
  267 + [[self accessLock] unlock];
245 return NO; 268 return NO;
246 } 269 }
247 } 270 }
@@ -255,10 +278,12 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -255,10 +278,12 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
255 NSArray *headersToCompare = [NSArray arrayWithObjects:@"Etag",@"Last-Modified",nil]; 278 NSArray *headersToCompare = [NSArray arrayWithObjects:@"Etag",@"Last-Modified",nil];
256 for (NSString *header in headersToCompare) { 279 for (NSString *header in headersToCompare) {
257 if (![[[request responseHeaders] objectForKey:header] isEqualToString:[cachedHeaders objectForKey:header]]) { 280 if (![[[request responseHeaders] objectForKey:header] isEqualToString:[cachedHeaders objectForKey:header]]) {
  281 + [[self accessLock] unlock];
258 return NO; 282 return NO;
259 } 283 }
260 } 284 }
261 } 285 }
  286 + [[self accessLock] unlock];
262 return YES; 287 return YES;
263 } 288 }
264 289
@@ -290,11 +315,13 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -290,11 +315,13 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
290 BOOL isDirectory = NO; 315 BOOL isDirectory = NO;
291 BOOL exists = [[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:&isDirectory]; 316 BOOL exists = [[NSFileManager defaultManager] fileExistsAtPath:path isDirectory:&isDirectory];
292 if (exists && !isDirectory || !exists) { 317 if (exists && !isDirectory || !exists) {
  318 + [[self accessLock] unlock];
293 return; 319 return;
294 } 320 }
295 NSError *error = nil; 321 NSError *error = nil;
296 NSArray *cacheFiles = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:path error:&error]; 322 NSArray *cacheFiles = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:path error:&error];
297 if (error) { 323 if (error) {
  324 + [[self accessLock] unlock];
298 [NSException raise:@"FailedToTraverseCacheDirectory" format:@"Listing cache directory failed at path '%@'",path]; 325 [NSException raise:@"FailedToTraverseCacheDirectory" format:@"Listing cache directory failed at path '%@'",path];
299 } 326 }
300 for (NSString *file in cacheFiles) { 327 for (NSString *file in cacheFiles) {
@@ -302,6 +329,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil; @@ -302,6 +329,7 @@ static NSDateFormatter *rfc1123DateFormatter = nil;
302 if ([extension isEqualToString:@"cacheddata"] || [extension isEqualToString:@"cachedheaders"]) { 329 if ([extension isEqualToString:@"cacheddata"] || [extension isEqualToString:@"cachedheaders"]) {
303 [[NSFileManager defaultManager] removeItemAtPath:[path stringByAppendingPathComponent:file] error:&error]; 330 [[NSFileManager defaultManager] removeItemAtPath:[path stringByAppendingPathComponent:file] error:&error];
304 if (error) { 331 if (error) {
  332 + [[self accessLock] unlock];
305 [NSException raise:@"FailedToRemoveCacheFile" format:@"Failed to remove cached data at path '%@'",path]; 333 [NSException raise:@"FailedToRemoveCacheFile" format:@"Failed to remove cached data at path '%@'",path];
306 } 334 }
307 } 335 }
@@ -23,7 +23,7 @@ @@ -23,7 +23,7 @@
23 23
24 24
25 // Automatically set on build 25 // Automatically set on build
26 -NSString *ASIHTTPRequestVersion = @"v1.6.2-18 2010-05-04"; 26 +NSString *ASIHTTPRequestVersion = @"v1.6.2-19 2010-05-04";
27 27
28 NSString* const NetworkRequestErrorDomain = @"ASIHTTPRequestErrorDomain"; 28 NSString* const NetworkRequestErrorDomain = @"ASIHTTPRequestErrorDomain";
29 29
@@ -656,7 +656,6 @@ static id <ASICacheDelegate> defaultCache = nil; @@ -656,7 +656,6 @@ static id <ASICacheDelegate> defaultCache = nil;
656 // See if we should pull from the cache rather than fetching the data 656 // See if we should pull from the cache rather than fetching the data
657 if ([self cachePolicy] == ASIOnlyLoadIfNotCachedCachePolicy) { 657 if ([self cachePolicy] == ASIOnlyLoadIfNotCachedCachePolicy) {
658 if ([self useDataFromCache]) { 658 if ([self useDataFromCache]) {
659 - [[self cancelledLock] unlock];  
660 return; 659 return;
661 } 660 }
662 } else if ([self cachePolicy] == ASIReloadIfDifferentCachePolicy) { 661 } else if ([self cachePolicy] == ASIReloadIfDifferentCachePolicy) {