Ben Copsey

Refactor to fix lots of bugs, especially locking issues

@@ -50,6 +50,11 @@ @@ -50,6 +50,11 @@
50 50
51 - (void)buildPostBody 51 - (void)buildPostBody
52 { 52 {
  53 + if (!postData && ! fileData) {
  54 + [super buildPostBody];
  55 + return;
  56 + }
  57 +
53 NSMutableData *body = [[[NSMutableData alloc] init] autorelease]; 58 NSMutableData *body = [[[NSMutableData alloc] init] autorelease];
54 59
55 // Set your own boundary string only if really obsessive. We don't bother to check if post data contains the boundary, since it's pretty unlikely that it does. 60 // Set your own boundary string only if really obsessive. We don't bother to check if post data contains the boundary, since it's pretty unlikely that it does.
@@ -10,6 +10,7 @@ @@ -10,6 +10,7 @@
10 // Portions are based on the ImageClient example from Apple: 10 // Portions are based on the ImageClient example from Apple:
11 // See: http://developer.apple.com/samplecode/ImageClient/listing37.html 11 // See: http://developer.apple.com/samplecode/ImageClient/listing37.html
12 12
  13 +#import <Cocoa/Cocoa.h>
13 14
14 @interface ASIHTTPRequest : NSOperation { 15 @interface ASIHTTPRequest : NSOperation {
15 16
@@ -93,18 +94,18 @@ @@ -93,18 +94,18 @@
93 int responseStatusCode; 94 int responseStatusCode;
94 95
95 //Size of the response 96 //Size of the response
96 - unsigned int contentLength; 97 + unsigned long long contentLength;
97 98
98 //Size of the POST payload 99 //Size of the POST payload
99 - unsigned int postLength; 100 + unsigned long long postLength;
100 101
101 //The total amount of downloaded data 102 //The total amount of downloaded data
102 - unsigned int totalBytesRead; 103 + unsigned long long totalBytesRead;
103 104
104 //Last amount of data read (used for incrementing progress) 105 //Last amount of data read (used for incrementing progress)
105 - unsigned int lastBytesRead; 106 + unsigned long long lastBytesRead;
106 //Last amount of data sent (used for incrementing progress) 107 //Last amount of data sent (used for incrementing progress)
107 - unsigned int lastBytesSent; 108 + unsigned long long lastBytesSent;
108 109
109 //Realm for authentication when credentials are required 110 //Realm for authentication when credentials are required
110 NSString *authenticationRealm; 111 NSString *authenticationRealm;
@@ -112,6 +113,9 @@ @@ -112,6 +113,9 @@
112 //This lock will block the request until the delegate supplies authentication info 113 //This lock will block the request until the delegate supplies authentication info
113 NSConditionLock *authenticationLock; 114 NSConditionLock *authenticationLock;
114 115
  116 + //This lock prevents the operation from being cancelled at an inopportune moment
  117 + NSLock *cancelledLock;
  118 +
115 //Called on the delegate when the request completes successfully 119 //Called on the delegate when the request completes successfully
116 SEL didFinishSelector; 120 SEL didFinishSelector;
117 121
@@ -128,7 +132,7 @@ @@ -128,7 +132,7 @@
128 NSAutoreleasePool *pool; 132 NSAutoreleasePool *pool;
129 133
130 // Will be YES when a HEAD request will handle the content-length before this request starts 134 // Will be YES when a HEAD request will handle the content-length before this request starts
131 - BOOL useCachedContentLength; 135 + BOOL shouldResetProgressIndicators;
132 136
133 // Used by HEAD requests when showAccurateProgress is YES to preset the content-length for this request 137 // Used by HEAD requests when showAccurateProgress is YES to preset the content-length for this request
134 ASIHTTPRequest *mainRequest; 138 ASIHTTPRequest *mainRequest;
@@ -177,9 +181,9 @@ @@ -177,9 +181,9 @@
177 181
178 // Called on main thread to update progress delegates 182 // Called on main thread to update progress delegates
179 - (void)updateProgressIndicators; 183 - (void)updateProgressIndicators;
180 -- (void)resetUploadProgress:(NSNumber *)max; 184 +- (void)resetUploadProgress:(unsigned long long)value;
181 - (void)updateUploadProgress; 185 - (void)updateUploadProgress;
182 -- (void)resetDownloadProgress:(NSNumber *)max; 186 +- (void)resetDownloadProgress:(unsigned long long)value;
183 - (void)updateDownloadProgress; 187 - (void)updateDownloadProgress;
184 188
185 // Called when authorisation is needed, as we only find out we don't have permission to something when the upload is complete 189 // Called when authorisation is needed, as we only find out we don't have permission to something when the upload is complete
@@ -279,10 +283,10 @@ @@ -279,10 +283,10 @@
279 @property (assign) NSTimeInterval timeOutSeconds; 283 @property (assign) NSTimeInterval timeOutSeconds;
280 @property (retain) NSString *requestMethod; 284 @property (retain) NSString *requestMethod;
281 @property (retain,setter=setPostBody:) NSData *postBody; 285 @property (retain,setter=setPostBody:) NSData *postBody;
282 -@property (assign) unsigned int contentLength; 286 +@property (assign) unsigned long long contentLength;
283 -@property (assign) unsigned int postLength; 287 +@property (assign) unsigned long long postLength;
284 -@property (assign) BOOL useCachedContentLength; 288 +@property (assign) BOOL shouldResetProgressIndicators;
285 @property (retain) ASIHTTPRequest *mainRequest; 289 @property (retain) ASIHTTPRequest *mainRequest;
286 @property (assign) BOOL showAccurateProgress; 290 @property (assign) BOOL showAccurateProgress;
287 -@property (assign,readonly) unsigned int totalBytesRead; 291 +@property (assign,readonly) unsigned long long totalBytesRead;
288 @end 292 @end
This diff is collapsed. Click to expand it.
@@ -109,8 +109,9 @@ @@ -109,8 +109,9 @@
109 ASIHTTPRequest *request = [[[ASIHTTPRequest alloc] initWithURL:url] autorelease]; 109 ASIHTTPRequest *request = [[[ASIHTTPRequest alloc] initWithURL:url] autorelease];
110 [request setDownloadProgressDelegate:self]; 110 [request setDownloadProgressDelegate:self];
111 [request start]; 111 [request start];
  112 +
112 BOOL success = (progress == 1); 113 BOOL success = (progress == 1);
113 - STAssertTrue(success,@"Failed to properly increment download progress"); 114 + STAssertTrue(success,@"Failed to properly increment download progress %f != 1.0",progress);
114 } 115 }
115 116
116 117
@@ -121,8 +122,9 @@ @@ -121,8 +122,9 @@
121 [request setPostBody:[NSMutableData dataWithLength:1024*32]]; 122 [request setPostBody:[NSMutableData dataWithLength:1024*32]];
122 [request setUploadProgressDelegate:self]; 123 [request setUploadProgressDelegate:self];
123 [request start]; 124 [request start];
  125 +
124 BOOL success = (progress == 1); 126 BOOL success = (progress == 1);
125 - STAssertTrue(success,@"Failed to properly increment upload progress"); 127 + STAssertTrue(success,@"Failed to properly increment upload progress %f != 1.0",progress);
126 } 128 }
127 129
128 130
@@ -6,7 +6,7 @@ @@ -6,7 +6,7 @@
6 // Copyright 2008 All-Seeing Interactive. All rights reserved. 6 // Copyright 2008 All-Seeing Interactive. All rights reserved.
7 // 7 //
8 8
9 - 9 +#import <Cocoa/Cocoa.h>
10 10
11 @interface ASINetworkQueue : NSOperationQueue { 11 @interface ASINetworkQueue : NSOperationQueue {
12 12
@@ -26,19 +26,19 @@ @@ -26,19 +26,19 @@
26 id uploadProgressDelegate; 26 id uploadProgressDelegate;
27 27
28 // Total amount uploaded so far for all requests in this queue 28 // Total amount uploaded so far for all requests in this queue
29 - unsigned int uploadProgressBytes; 29 + unsigned long long uploadProgressBytes;
30 30
31 // Total amount to be uploaded for all requests in this queue - requests add to this figure as they work out how much data they have to transmit 31 // Total amount to be uploaded for all requests in this queue - requests add to this figure as they work out how much data they have to transmit
32 - unsigned int uploadProgressTotalBytes; 32 + unsigned long long uploadProgressTotalBytes;
33 33
34 // Download progress indicator, probably an NSProgressIndicator or UIProgressView 34 // Download progress indicator, probably an NSProgressIndicator or UIProgressView
35 id downloadProgressDelegate; 35 id downloadProgressDelegate;
36 36
37 // Total amount downloaded so far for all requests in this queue 37 // Total amount downloaded so far for all requests in this queue
38 - unsigned int downloadProgressBytes; 38 + unsigned long long downloadProgressBytes;
39 39
40 // Total amount to be downloaded for all requests in this queue - requests add to this figure as they receive Content-Length headers 40 // Total amount to be downloaded for all requests in this queue - requests add to this figure as they receive Content-Length headers
41 - unsigned int downloadProgressTotalBytes; 41 + unsigned long long downloadProgressTotalBytes;
42 42
43 // When YES, the queue will cancel all requests when a request fails. Default is YES 43 // When YES, the queue will cancel all requests when a request fails. Default is YES
44 BOOL shouldCancelAllRequestsOnFailure; 44 BOOL shouldCancelAllRequestsOnFailure;
@@ -61,16 +61,19 @@ @@ -61,16 +61,19 @@
61 - (void)addHEADOperation:(NSOperation *)operation; 61 - (void)addHEADOperation:(NSOperation *)operation;
62 62
63 // Called at the start of a request to add on the size of this upload to the total 63 // Called at the start of a request to add on the size of this upload to the total
64 -- (void)incrementUploadSizeBy:(int)bytes; 64 +- (void)incrementUploadSizeBy:(unsigned long long)bytes;
65 65
66 // Called during a request when data is written to the upload stream to increment the progress indicator 66 // Called during a request when data is written to the upload stream to increment the progress indicator
67 -- (void)incrementUploadProgressBy:(int)bytes; 67 +- (void)incrementUploadProgressBy:(unsigned long long)bytes;
68 68
69 // Called at the start of a request to add on the size of this download to the total 69 // Called at the start of a request to add on the size of this download to the total
70 -- (void)incrementDownloadSizeBy:(int)bytes; 70 +- (void)incrementDownloadSizeBy:(unsigned long long)bytes;
71 71
72 // Called during a request when data is received to increment the progress indicator 72 // Called during a request when data is received to increment the progress indicator
73 -- (void)incrementDownloadProgressBy:(int)bytes; 73 +- (void)incrementDownloadProgressBy:(unsigned long long)bytes;
  74 +
  75 +// Called during a request when authorisation fails to cancel any progress so far
  76 +- (void)decrementUploadProgressBy:(unsigned long long)bytes;
74 77
75 // All ASINetworkQueues are paused when created so that total size can be calculated before the queue starts 78 // All ASINetworkQueues are paused when created so that total size can be calculated before the queue starts
76 // This method will start the queue 79 // This method will start the queue
@@ -34,6 +34,7 @@ @@ -34,6 +34,7 @@
34 34
35 showAccurateProgress = NO; 35 showAccurateProgress = NO;
36 36
  37 + [self setMaxConcurrentOperationCount:4];
37 [self setSuspended:YES]; 38 [self setSuspended:YES];
38 39
39 return self; 40 return self;
@@ -59,9 +60,6 @@ @@ -59,9 +60,6 @@
59 uploadProgressTotalBytes = 0; 60 uploadProgressTotalBytes = 0;
60 downloadProgressBytes = 0; 61 downloadProgressBytes = 0;
61 downloadProgressTotalBytes = 0; 62 downloadProgressTotalBytes = 0;
62 - [self setUploadProgressDelegate:nil];  
63 - [self setDownloadProgressDelegate:nil];  
64 - [self setDelegate:nil];  
65 [super cancelAllOperations]; 63 [super cancelAllOperations];
66 } 64 }
67 65
@@ -103,6 +101,8 @@ @@ -103,6 +101,8 @@
103 if ([operation isKindOfClass:[ASIHTTPRequest class]]) { 101 if ([operation isKindOfClass:[ASIHTTPRequest class]]) {
104 102
105 ASIHTTPRequest *request = (ASIHTTPRequest *)operation; 103 ASIHTTPRequest *request = (ASIHTTPRequest *)operation;
  104 + [request setRequestMethod:@"HEAD"];
  105 + [request setQueuePriority:10];
106 [request setShowAccurateProgress:YES]; 106 [request setShowAccurateProgress:YES];
107 if (uploadProgressDelegate) { 107 if (uploadProgressDelegate) {
108 [request setUploadProgressDelegate:self]; 108 [request setUploadProgressDelegate:self];
@@ -133,12 +133,11 @@ @@ -133,12 +133,11 @@
133 //If this is a GET request and we want accurate progress, perform a HEAD request first to get the content-length 133 //If this is a GET request and we want accurate progress, perform a HEAD request first to get the content-length
134 if ([[request requestMethod] isEqualToString:@"GET"]) { 134 if ([[request requestMethod] isEqualToString:@"GET"]) {
135 ASIHTTPRequest *HEADRequest = [[[ASIHTTPRequest alloc] initWithURL:[request url]] autorelease]; 135 ASIHTTPRequest *HEADRequest = [[[ASIHTTPRequest alloc] initWithURL:[request url]] autorelease];
136 - [HEADRequest setRequestMethod:@"HEAD"];  
137 - [HEADRequest setQueuePriority:10];  
138 [HEADRequest setMainRequest:request]; 136 [HEADRequest setMainRequest:request];
139 [self addHEADOperation:HEADRequest]; 137 [self addHEADOperation:HEADRequest];
140 138
141 - [request setUseCachedContentLength:YES]; 139 + //Tell the request not to reset the progress indicator when it gets a content-length, as we will get the length from the HEAD request
  140 + [request setShouldResetProgressIndicators:NO];
142 [request addDependency:HEADRequest]; 141 [request addDependency:HEADRequest];
143 142
144 //If we want to track uploading for this request accurately, we need to add the size of the post content to the total 143 //If we want to track uploading for this request accurately, we need to add the size of the post content to the total
@@ -150,6 +149,9 @@ @@ -150,6 +149,9 @@
150 [request setShowAccurateProgress:showAccurateProgress]; 149 [request setShowAccurateProgress:showAccurateProgress];
151 150
152 if (uploadProgressDelegate) { 151 if (uploadProgressDelegate) {
  152 +
  153 + //For uploads requests, we always work out the total upload size before the queue starts, so we tell the request not to reset the progress indicator when starting each request
  154 + [request setShouldResetProgressIndicators:NO];
153 [request setUploadProgressDelegate:self]; 155 [request setUploadProgressDelegate:self];
154 } else { 156 } else {
155 [request setUploadProgressDelegate:NULL]; 157 [request setUploadProgressDelegate:NULL];
@@ -173,7 +175,7 @@ @@ -173,7 +175,7 @@
173 if (requestDidFailSelector) { 175 if (requestDidFailSelector) {
174 [delegate performSelector:requestDidFailSelector withObject:request]; 176 [delegate performSelector:requestDidFailSelector withObject:request];
175 } 177 }
176 - if (shouldCancelAllRequestsOnFailure) { 178 + if (shouldCancelAllRequestsOnFailure && requestsCount > 0) {
177 [self cancelAllOperations]; 179 [self cancelAllOperations];
178 } 180 }
179 } 181 }
@@ -191,7 +193,7 @@ @@ -191,7 +193,7 @@
191 } 193 }
192 } 194 }
193 195
194 -- (void)incrementUploadSizeBy:(int)bytes 196 +- (void)incrementUploadSizeBy:(unsigned long long)bytes
195 { 197 {
196 if (!uploadProgressDelegate) { 198 if (!uploadProgressDelegate) {
197 return; 199 return;
@@ -200,7 +202,19 @@ @@ -200,7 +202,19 @@
200 [self incrementUploadProgressBy:0]; 202 [self incrementUploadProgressBy:0];
201 } 203 }
202 204
203 -- (void)incrementUploadProgressBy:(int)bytes 205 +- (void)decrementUploadProgressBy:(unsigned long long)bytes
  206 +{
  207 + if (!uploadProgressDelegate || uploadProgressTotalBytes == 0) {
  208 + return;
  209 + }
  210 + uploadProgressBytes -= bytes;
  211 +
  212 + double progress = (uploadProgressBytes*1.0)/(uploadProgressTotalBytes*1.0);
  213 + [ASIHTTPRequest setProgress:progress forProgressIndicator:uploadProgressDelegate];
  214 +}
  215 +
  216 +
  217 +- (void)incrementUploadProgressBy:(unsigned long long)bytes
204 { 218 {
205 if (!uploadProgressDelegate || uploadProgressTotalBytes == 0) { 219 if (!uploadProgressDelegate || uploadProgressTotalBytes == 0) {
206 return; 220 return;
@@ -209,24 +223,26 @@ @@ -209,24 +223,26 @@
209 223
210 double progress = (uploadProgressBytes*1.0)/(uploadProgressTotalBytes*1.0); 224 double progress = (uploadProgressBytes*1.0)/(uploadProgressTotalBytes*1.0);
211 [ASIHTTPRequest setProgress:progress forProgressIndicator:uploadProgressDelegate]; 225 [ASIHTTPRequest setProgress:progress forProgressIndicator:uploadProgressDelegate];
  226 +
212 } 227 }
213 228
214 -- (void)incrementDownloadSizeBy:(int)bytes 229 +- (void)incrementDownloadSizeBy:(unsigned long long)bytes
215 { 230 {
216 if (!downloadProgressDelegate) { 231 if (!downloadProgressDelegate) {
217 return; 232 return;
218 } 233 }
219 downloadProgressTotalBytes += bytes; 234 downloadProgressTotalBytes += bytes;
220 [self incrementDownloadProgressBy:0]; 235 [self incrementDownloadProgressBy:0];
  236 + NSLog(@"download size is now: %qu",downloadProgressTotalBytes);
221 } 237 }
222 238
223 -- (void)incrementDownloadProgressBy:(int)bytes 239 +- (void)incrementDownloadProgressBy:(unsigned long long)bytes
224 { 240 {
225 if (!downloadProgressDelegate || downloadProgressTotalBytes == 0) { 241 if (!downloadProgressDelegate || downloadProgressTotalBytes == 0) {
226 return; 242 return;
227 } 243 }
228 downloadProgressBytes += bytes; 244 downloadProgressBytes += bytes;
229 - //NSLog(@"%hu/%hu",downloadProgressBytes,downloadProgressTotalBytes); 245 + //NSLog(@"%qu/%qu",downloadProgressBytes,downloadProgressTotalBytes);
230 double progress = (downloadProgressBytes*1.0)/(downloadProgressTotalBytes*1.0); 246 double progress = (downloadProgressBytes*1.0)/(downloadProgressTotalBytes*1.0);
231 [ASIHTTPRequest setProgress:progress forProgressIndicator:downloadProgressDelegate]; 247 [ASIHTTPRequest setProgress:progress forProgressIndicator:downloadProgressDelegate];
232 } 248 }
@@ -201,8 +201,6 @@ @@ -201,8 +201,6 @@
201 201
202 - (void)requestFailedCancellingOthers:(ASIHTTPRequest *)request 202 - (void)requestFailedCancellingOthers:(ASIHTTPRequest *)request
203 { 203 {
204 - BOOL success = (request == requestThatShouldFail);  
205 - STAssertTrue(success,@"Wrong request failed");  
206 complete = YES; 204 complete = YES;
207 } 205 }
208 206