lloydsargent / BlackRaccoon

IOS FTP Client Code
102 stars 37 forks source link

Incomplete uploads with no error #17

Closed lnafziger closed 11 years ago

lnafziger commented 11 years ago

I have the same problem as issues #14 and #9:

When uploading files, they are often only partially uploaded, however I get a requestCompleted: delegate callback instead of a requestFailed: delegate callback.

From what you have said in the other two issues, this can probably be fixed by passing smaller "chunks" in requestDataToSend:. However when sending the entire file this should at the very least, result in the requestFailed: delegate method being called when it does not upload the entire file, but should probably be fixed so that it works, especially since this is the example code that you give for us to use.

lloydsargent commented 11 years ago

I agree, it should return "requestFailed".

Unfortunately, I do not have the time this week to look into the issue. If you would like to do so and send me a patch, I would be happy to accept it.

That is the nature of opensource code. If you like, there is still WhiteRaccoon which I believe is now ARC compatible.

On Mar 30, 2013, at 15:09 , Louis notifications@github.com wrote:

I have the same problem as issues #14 and #9:

When uploading files, they are often only partially uploaded, however I get a requestCompleted: delegate callback instead of a requestFailed: delegate callback.

From what you have said in the other two issues, this can probably be fixed by passing smaller "chunks" in requestDataToSend:. However when sending the entire file this should at the very least, result in the requestFailed: delegate method being called when it does not upload the entire file, but should probably be fixed so that it works, especially since this is the example code that you give for us to use.

— Reply to this email directly or view it on GitHub.

lnafziger commented 11 years ago

I looked into it some, but without knowing the code base better couldn't see an obvious cause of the problem.

I did compare BlackRaccoon with WhiteRaccoon to see if there was something that stood out that I could modify here, but you are using a different method than they are.

Unfortunately, WhiteRaccoon does not provide a method of showing the percent complete like BlackRaccoon does so isn't an option for me. I may need to switch to something else completely though. In any case, fixing the underlying problem when you have time (and also correcting it so that similar problems actually generate an error) would be good for others!

lloydsargent commented 11 years ago

Let's start over. Obviously something is going astray. Let's see if we can figure this out.

First, have you tried using the BlackRaccoon demo program to upload a file?

Second, what does your code look like? I'm kind of working in the dark - I don't know what your code looks like which makes it very difficult to determine where the error lies/

Third, when you say "uploading" is that transferring FROM the iPad to TO the iPad. I just want to be assured we're not mixing things up.

Fourth, when you say you "couldn't see an obvious cause" does that mean you set breakpoints? Did you use a packet sniffer to see what was going on?

Fifth, are you communicating to a local system or a server located on the web?

I agree that BlackRaccoon may have some holes, but also be aware that FTP servers can introduce some oddities of their own. I'm willing to work with you, but I need more information.

On Apr 1, 2013, at 12:06 , Louis notifications@github.com wrote:

I looked into it some, but without knowing the code base better couldn't see an obvious cause of the problem.

I did compare BlackRaccoon with WhiteRaccoon to see if there was something that stood out that I could modify here, but you are using a different method than they are.

Unfortunately, WhiteRaccoon does not provide a method of showing the percent complete like BlackRaccoon does so isn't an option for me. I may need to switch to something else completely though. In any case, fixing the underlying problem when you have time (and also correcting it so that similar problems actually generate an error) would be good for others!

— Reply to this email directly or view it on GitHub.

lnafziger commented 11 years ago

Great, thank you for working with me on this. I'm not a networking expert, so I have limited means when it comes to trouble-shooting these types of issues but am willing to help where I can, and can hopefully provide useful information if pointed in the correct direction.

Another big thing is that this is an intermittent issue, so it is very hard to troubleshoot. If it were repeatable, then I could have more meaningful information for you. Since this is the third issue posted to Github with this problem though, something must be amiss.

I am uploading three files in a row. A very small xml file (the story title), a photo that can range in size, but most are taken on the iPad and are around 2 MB in size, and an audio file that can vary from 20k - 50MB. I have seen BOTH the xml file and the photo file get clipped, but have not paid a lot of attention to the audio file yet. I assume that we are having the same problems with that file but it has been over-shadowed by the most visible ones at this point.

To answer your questions:

  1. I have not used the demo program, although I did use your example code from the Readme.me file on the main page of Github. I modified it slightly, so as to be able to upload multiple files, but I'm just "chaining" them together and they are uploading individually.
  2. I'll post the entirety of the code at the end of this post. (Well, I removed the non-essential UI update code to make it easier for you.)
  3. Yes, I am uploading files FROM the iPad TO the remove server.
  4. Yes, I set breakpoints but unfortunately, even after many tries, it did not happen when a breakpoint was set. By "obvious cause" I meant by looking through that portion of the code, and also by comparing it to WhiteRaccoon which does not seem to have this issue. I did not use a packet sniffer, because I'm not familiar with what I should be looking for if I did.
  5. I am communicating with an pureftp server located on the web.

Let me know what else I can provide!

- (void)upload
{
    NSURL *photoURL = ...
    NSURL *audioURL = ...
    NSURL *storyTitleURL = ...

    NSString *username = ...
    NSString *password = ...

    if (![photoURL checkResourceIsReachableAndReturnError:nil] ||
        ![audioURL checkResourceIsReachableAndReturnError:nil] ||
        ![storyTitleURL checkResourceIsReachableAndReturnError:nil] ||
        [username length] < 1 ||
        [password length] < 1)
    {
        // Alert User
        // ...
        return;
    }

    // Chain the three requests together by filling in the nextRequest.
    // After the first one completes, it will start the next one running in requestCompleted:
    self.audioUploadRequest = [BRRequestUpload initWithDelegate:self];
    self.audioUploadRequest.path = [audioURL lastPathComponent];
    self.audioUploadRequest.hostname = kUploadServerAddress;
    self.audioUploadRequest.username = username;
    self.audioUploadRequest.password = password;

    self.photoUploadRequest = [BRRequestUpload initWithDelegate:self];
    self.photoUploadRequest.path = [photoURL lastPathComponent];
    self.photoUploadRequest.hostname = kUploadServerAddress;
    self.photoUploadRequest.username = username;
    self.photoUploadRequest.password = password;
    self.photoUploadRequest.nextRequest = self.audioUploadRequest;

    self.storyTitleUploadRequest = [BRRequestUpload initWithDelegate:self];
    self.storyTitleUploadRequest.path = [storyTitleURL lastPathComponent];
    self.storyTitleUploadRequest.hostname = kUploadServerAddress;
    self.storyTitleUploadRequest.username = username;
    self.storyTitleUploadRequest.password = password;
    self.storyTitleUploadRequest.nextRequest = self.photoUploadRequest;

    self.uploadRequest = self.storyTitleUploadRequest;
    [self.uploadRequest start];
}

#pragma mark - Black Raccoon - BRRequestDelegate methods

#pragma mark - Black Raccoon (ftp) delegate methods

- (BOOL)shouldOverwriteFileWithRequest:(BRRequest *)request
{
    return YES;
}

- (long)requestDataSendSize:(BRRequestUpload *)request
{
    if (request == self.photoUploadRequest)
    {
        NSDictionary *info = [[NSFileManager defaultManager] attributesOfItemAtPath:[self.talkingPhoto.photoURL path] error:nil];
        return [info fileSize];
    }

    if (request == self.audioUploadRequest)
    {
        NSDictionary *info = [[NSFileManager defaultManager] attributesOfItemAtPath:[self.talkingPhoto.audioURL path] error:nil];
        return [info fileSize];
    }

    if (request == self.storyTitleUploadRequest)
    {
        NSDictionary *info = [[NSFileManager defaultManager] attributesOfItemAtPath:[self.talkingPhoto.storyTitleURL path] error:nil];
        return [info fileSize];
    }

    return 0.0;
}

- (NSData *)requestDataToSend:(BRRequestUpload *)request
{
    if (request == self.photoUploadRequest)
    {
        if (!request.totalBytesSent)
        {
            return [NSData dataWithContentsOfURL:self.talkingPhoto.photoURL];
        }
    }

    if (request == self.audioUploadRequest)
    {
        if (!request.totalBytesSent)
        {
            return [NSData dataWithContentsOfURL:self.talkingPhoto.audioURL];
        }
    }

    if (request == self.storyTitleUploadRequest)
    {
        if (!request.totalBytesSent)
        {
            return [NSData dataWithContentsOfURL:self.talkingPhoto.storyTitleURL];
        }
    }

    return nil;
}

- (void)percentCompleted:(BRRequest *)request
{
    // Update the status fields
}

- (void)requestCompleted:(BRRequest *)request
{
    self.uploadRequest = request.nextRequest;
    if (self.uploadRequest)
    {
        [self.uploadRequest start];
    }
    else
    {
        // Update the UI, indicating that we are done uploading
    }
}

- (void)requestFailed:(BRRequest *)request
{
    self.uploadRequest = nil;
    // Update the UI, indicating that the upload has failed.
}
lnafziger commented 11 years ago

By the way, I just re-wrote requestDataToSend: to send smaller chunks (15,000 bytes at a time) and had the same problem on the first file that I uploaded (the image file was truncated):

- (NSData *)requestDataToSend:(BRRequestUpload *)request
{
    if (request == self.photoUploadRequest)
    {
        if (!self.uploadData)
            // NSDataReadingMappedAlways prevents the entire file from being loaded into memory
            self.uploadData = [NSData dataWithContentsOfURL:self.talkingPhoto.photoURL options:NSDataReadingMappedAlways error:nil];

        if (self.uploadData)
            if (request.totalBytesSent < self.uploadData.length)
                return [self.uploadData subdataWithRange:NSMakeRange(request.totalBytesSent, MIN(10000, (self.uploadData.length - request.totalBytesSent)))];

        // Done uploading
        self.uploadData = nil;
        return nil;
    }

    if (request == self.audioUploadRequest)
    {
        if (!self.uploadData)
            self.uploadData = [NSData dataWithContentsOfURL:self.talkingPhoto.audioURL options:NSDataReadingMappedAlways error:nil];

        if (self.uploadData)
            if (request.totalBytesSent < self.uploadData.length)
                return [self.uploadData subdataWithRange:NSMakeRange(request.totalBytesSent, MIN(10000, (self.uploadData.length - request.totalBytesSent)))];

        // Done uploading
        self.uploadData = nil;
        return nil;
    }

    if (request == self.storyTitleUploadRequest)
    {
        if (!self.uploadData)
            self.uploadData = [NSData dataWithContentsOfURL:self.talkingPhoto.storyTitleURL options:NSDataReadingMappedAlways error:nil];

        if (self.uploadData)
            if (request.totalBytesSent < self.uploadData.length)
                return [self.uploadData subdataWithRange:NSMakeRange(request.totalBytesSent, MIN(10000, (self.uploadData.length - request.totalBytesSent)))];

        // Done uploading
        self.uploadData = nil;
        return nil;
    }

    return nil;
}
lnafziger commented 11 years ago

Okay, so I tried another approach, with similar results:

And this time, I added in some NSLog statements to show me the file size and the total number of bytes sent.

The first time, with a successful upload, I got the following for the photo file: requestDataSendSize: 304,845 bytes requestDataToSend: 304,845 bytes The file on the ftp server 304,845 bytes

I then uploaded the same files a second time and got: requestDataSendSize: 304,845 bytes requestDataToSend: 304,845 bytes The file on the ftp server 295,392 bytes

Note that BlackRaccoon reported successfully transferring the entire file, but for some reason it didn't make it to the server, yet reported successful.

This was with the following code:

- (NSData *)requestDataToSend:(BRRequestUpload *)request
{
    static unsigned long buffer_size = 15000;

    if (request == self.photoUploadRequest)
    {
        FILE *file = fopen([[self.talkingPhoto.photoURL path] UTF8String], "rb");
        if(file)
        {
            void *data = malloc(buffer_size);  
            if (data)
            {
                fseeko(file, request.totalBytesSent, SEEK_SET);
                size_t bytes_read = fread(data, 1, buffer_size, file);  
                fclose(file);

                // NSData takes ownership and will call free(data) when it's released
                if (bytes_read > 0)
                    return [NSData dataWithBytesNoCopy:data length:bytes_read];

                free(data);
            }
        }
        return nil;
    }

    if (request == self.audioUploadRequest)
    {
        FILE *file = fopen([[self.talkingPhoto.audioURL path] UTF8String], "rb");
        if(file)
        {
            void *data = malloc(buffer_size);  
            if (data)
            {
                fseeko(file, request.totalBytesSent, SEEK_SET);
                size_t bytes_read = fread(data, 1, buffer_size, file);  
                fclose(file);

                // NSData takes ownership and will call free(data) when it's released
                if (bytes_read > 0)
                    return [NSData dataWithBytesNoCopy:data length:bytes_read];

                free(data);
            }
        }
        return nil;
    }

    if (request == self.storyTitleUploadRequest)
    {
        FILE *file = fopen([[self.talkingPhoto.storyTitleURL path] UTF8String], "rb");
        if(file)
        {
            void *data = malloc(buffer_size);  
            if (data)
            {
                fseeko(file, request.totalBytesSent, SEEK_SET);
                size_t bytes_read = fread(data, 1, buffer_size, file);  
                fclose(file);

                // NSData takes ownership and will call free(data) when it's released
                if (bytes_read > 0)
                    return [NSData dataWithBytesNoCopy:data length:bytes_read];

                free(data);
            }
        }
        return nil;
    }

    return nil;
}
lnafziger commented 11 years ago

I also tried NOT using nextRequest but instead just starting the next request when the last one finished, with the same results.

lnafziger commented 11 years ago

One more thing that may help: If I set a breakpoint within the code (say in requestCompleted:) then the problem doesn't appear. Only when I let it run without breakpoints.

lnafziger commented 11 years ago

Another thing, I just tried it with another ftp server (also running Pure-ftp) and had the same issue there. Completely different site/location/configuration/etc though.

lnafziger commented 11 years ago

And lastly, I just used White Raccoon and it does work fine, so there must be some difference between the two that is causing this issue.

lnafziger commented 11 years ago

Okay, I solved the problem, even if I don't understand exactly WHY it solves it:

In BRStreamInfo.m, in openWrite:, I removed the following line:

CFWriteStreamSetProperty(writeStreamRef, kCFStreamPropertyFTPAttemptPersistentConnection, kCFBooleanFalse);

The files now correctly upload, using my original method.

lloydsargent commented 11 years ago

Well that is very odd. I admit I'm puzzled as well. I know what it is doing...

There IS one possibility that I didn't think about. Looking at your code you start one ftp transfer after another. However, note that when I complete I set the value to nil?

That is more than just setting it to nil - it also closes the connection. I wonder if things are getting confused because you never set it to nil?

For example if in the requestCompletedDelegate you had the following:

if (request == self.audioUploadRequest)
{
    self.audioUploadRequest = nil;
}

I think what is happening is that pureFTP is getting confused because its trying to reuse the same socket for a different transfer. I'll go ahead and change this to false just in case. For most people a little slower upload/download won't matter.

Thanks for finding that!

lnafziger commented 11 years ago

Well, keep in mind that my code DOES do this:

self.uploadRequest = request.nextRequest;

which releases the resource for the old request, and is no different than doing this:

self.uploadRequest = nil;
self.uploadRequest = request.nextRequest;

In any case, hopefully this will help some other people in the future!

lloydsargent commented 11 years ago

Sorry, I just skimmed through your code. I've been dealing with C++ all day and a whole 'nother method of memory management. You are correct about the assignment.

FYI, I did a little more research and that line probably never should have been there. What it does is force a recycling of connections which I think is what was messing things up. At least I can see how it could from the big picture view. Also, when the stream is created it is defined as TRUE, so I'm guessing it was just wrong (from whoever's code I cribbed it from).

lloydsargent commented 11 years ago

Okay, it is now up on github.

lnafziger commented 11 years ago

Great, thanks for updating so quickly!

canzhiye commented 11 years ago

You guys are amazing.