Crash after canceling S3 GetObject request in S3TransferManager
colesbury opened this issue · 7 comments
This is in version 1.71.
The crash usually occurs at S3GetObjectResponse (connection:didReceiveData:)
Use case:
S3TransferManager* tm = [[S3TransferManager alloc] init]
S3GetObjectRequest* get = ...
S3TransferOperation* op = [tm download:get]
after a little while
[op cancel]
Depending on how (un)lucky you get, you may see the crash.
The problem is at S3GetObjectOperation_Internal.m (line 262). The didReceiveResponse method forgets to cancel the underlying getRequest. Instead, it cleans up and finishes and deallocates, but S3GetObjectOperation_Internal is still set as the delegate on the S3GetObjectRequest. When the S3GetObjectResponse receives data, it the zombie S3GetObjectOperation_Internal can cause a crash.
FIX:
Add [self.getRequest cancel]; to S3GetObjectOperation_Internal around line 262.
Seeing this crash too - even when passing the request to an S3TransferManager. Calling pause or cancel on the downloads will trigger this.
I'm seeing this also in crash reports. I raised this on AWS SDK forum a couple of months ago and got no satisfaction. I guess I'm a little hesitant to modify Amazon's code.
Could this also be an issue in S3PutOperation_Internal
? We are getting an issue where it calls request:didFailWithError:
but then later calls request:didCompleteWithResponse:
on the same request as far as I can tell.
@chadwilken FWIW I would not spend any time on the old S3TransferManager. I have spent a lot of time tracing back the problem with the delegate methods and the problem runs deep in to the way it is architected. I am using the low level APIs for PutObject because it is the only method that I can verify to work consistently. Even MultipartUpload in the old iOS SDK has issues. I am hopeful that the completely redesigned iOS SDK that is in preview right now will be a big improvement, but until it is verified, I am sticking with the low-level APIs and in some cases, running the requests through my own server where I can use the Java methods instead...
@middleseatman would you be willing to share your code?
I can share the code to do the upload, but it is pretty basic. It just uses put object, retrying x times in a while loop if it fails.
-
(bool) uploadData:(NSData_)data withKey:(NSString_)key toBucket:(NSString*)bucket
{
S3PutObjectRequest * request = [[S3PutObjectRequest alloc] initWithKey:key inBucket:bucket];
[request setData:data];
S3PutObjectResponse *response = nil;
int attempts = 0;
do {
response = [[AmazonClientManager s3] putObject:request];
attempts++;
} while (response.error && attempts<retries);return response.error == nil;
}
The more complicated part that I can’t share right now is that we also use Core Data to track state for uploads and downloads. That ensures that the object will always eventually get transferred regardless of what happens with the network or the system. We also monitor changes in the network to trigger retries once a connection returns. Finally, our servers get a post request when the upload is successful and they double check the object metadata to be sure that everything matches.
On Jul 18, 2014, at 2:19 PM, Chad Wilken notifications@github.com wrote:
@middleseatman would you be willing to share your code?
—
Reply to this email directly or view it on GitHub.
Thanks for reaching out.
I fixed this issue using a fix I found on Stack Overflow. I hope you’ll incorporate the fix in a release!
I’m remote on a very slow internet connection — otherwise I would’ve found the link and send it to you. But I’m attaching the fixed file. Just do a diff with your release version to see what was done.
Best,
Winston.
Winston.
On Jul 18, 2014, at 11:19 AM, Chad Wilken notifications@github.com wrote:
@middleseatman would you be willing to share your code?
—
Reply to this email directly or view it on GitHub.