CODEWITHSUNDEEP
javaandroidsocketsnetwork-programmingtcp
Kevik
Kevik

Reputation: 9351

Why is this short sample of sockets code producing corrupted files?

I am trying to do something like in this first example of code below that is non-blocking but it gives corrupted files. The second example works fine but it is blocking, if you don't close the socket of output stream from the client side (other side), which is the other device this sever is connected to, then it will block and thread execution will not go any farther than that.

What is wrong with the first piece of code that causes it to produce corrupted files every time?

First example code, good idea but produces corrupted files;

 while(totalBytesRead < fileSizeFromClient){
            int bytesRemaining = fileSizeFromClient - totalBytesRead;
            int bytesRead = bufferedInputStream.read(buffer, totalBytesRead, bytesRemaining);
                            bufferedOutputStream.write(buffer, totalBytesRead, bytesRemaining);

            if(bytesRead == -1){
                break;
             }else{
                totalBytesRead += bytesRead;
            }
        }

Second example, code gets stuck; blocks. So I can't use this, as you have to kill the socket from the client side to get code execution to continue beyond these lines of code. But it produces perfect uncorrupted files.

  while((count = bufferedInputStream.read(buffer)) > 0){
        bufferedOutputStream.write(buffer, 0, count); 
 }

Upvotes: 0

Views: 119

Answers (2)

Joni
Joni

Reputation: 111219

This line writes all of the "remaining" bytes, even if only one byte was read, and also potentially writes something if read returned -1 signaling the end of input:

bufferedOutputStream.write(buffer, totalBytesRead, bytesRemaining);

To write only the bytes that were read, pass the number of bytes that were read as the third parameter.

    int bytesRead = bufferedInputStream.read(buffer, totalBytesRead, bytesRemaining);

    if (bytesRead == -1) {
        break;
     } else {
        bufferedOutputStream.write(buffer, totalBytesRead, bytesRead);
        totalBytesRead += bytesRead;
    }

There is no reason why the second code should block and this one not, though.

Also note that this way you are creating an in-memory copy of the data that you are transferring, which may occupy a lot of memory or make the program fail with ArrayIndexOutOfBoundsException if the buffer is not big enough. If that was not your intention you should use 0 instead of totalBytesRead as the array offset when reading and writing, and limit the number of bytes read to the size of the buffer. After these transformations the code would look like this:

    int bytesRead = bufferedInputStream.read(buffer);

    if (bytesRead == -1) {
        break;
     } else {
        bufferedOutputStream.write(buffer, 0, bytesRead);
        totalBytesRead += bytesRead;
    }

As you can see, it is almost identical to your second example, which you say "blocks."

Upvotes: 0

user207421
user207421

Reputation: 310860

  1. The test for bytesRead == -1 should precede the write, not follow it.
  2. The length argument for write() should be bytesRead.
  3. The offset arguments for both read() and write() should be zero.

Upvotes: 1

Related Questions