3

On the same computer I have 2 console applications. (client sends a file to server)

Before I start I just want to mention that i'm using those 2 async functions :

For reading from NetworkStream:

 public static Task<int> ReadAsync1(this NetworkStream networkStream, byte[] buffer, int offset, int size)
   {
     return Task<int>.Factory.FromAsync(networkStream.BeginRead, networkStream.EndRead, buffer, offset, buffer.Length, networkStream);
   }

For writing to NetworkStream:

   public static Task WriteAsync1(this NetworkStream networkStream, byte[] buffer, int offset, int size)
    {
      return Task.Factory.FromAsync(networkStream.BeginWrite, networkStream.EndWrite, buffer, offset, buffer.Length, networkStream);
    }

My client code :

(let's take a simple file which I know it's size)

enter image description here

My code for sending:

 NetworkStream ns = socketForServer.GetStream();
 ns.WriteAsync1(File.ReadAllBytes(@"c:\r\1.jpg"), 0, 1593899);
 ns.Flush();

My Server code : (reading...)

byte[] d = new byte[1593899];
networkStream.Read(d, 0, d.Length);
File.WriteAllBytes(@"c:\r\2.jpg",d);

Result :

2.jpg is written :

enter image description here

So where is the problem ?

2.jpg is corrupted ! When I try to open the image I see this :

enter image description here

(instead of a the same image as 1.jpg)

So I went to see the "HEX" for what is wrong :

I found out that after a while -all it writes are zeros :

larger iamge

enter image description here

Question

What am I doing wrong ? And how can I fix the code so it will send the whole file ?

Royi Namir
  • 144,742
  • 138
  • 468
  • 792
  • Not sure if that's it, but you seem to be ignoring the `size` parameter to `WriteAsync1`. – 500 - Internal Server Error Dec 24 '14 at 00:43
  • 2
    The `Stream` class already has perfectly good `ReadAsync()` and `WriteAsync()` methods. It is absurd to write whole new ones. As for the question, this is almost certainly being caused by the fact that you assume incorrectly that a single read operation will return all of the data you need. It won't; you have to check the return value of the read operation (which is the count of bytes _actually_ read) and then take the appropriate reaction (e.g. read more data or, once the right number of bytes has been read, go on to actually process the data). – Peter Duniho Dec 24 '14 at 00:46
  • @PeterDuniho Wow. I didn't notice ( it happens) that it already has ReadAsync. but it still yields corrupted image – Royi Namir Dec 24 '14 at 00:55
  • according to msdn the buffer size of Networkstream is 8192 k. If the file size is greater than this then you file a will be corrupted. Use the read method of the stream and keep reading from the stream until read is greater than zero. – user3514987 Dec 24 '14 at 00:59
  • 2
    Yes, of course it yields corrupted data. That bug isn't because you reimplemented the async methods; it's because you're incorrectly assuming that a single read operation gets all of the data and ignoring the byte count value returned by the read operation. – Peter Duniho Dec 24 '14 at 00:59
  • @user3514987: even if the file size is less than 8KB (the default receive buffer for a `Socket`...not sure where you got "8192 k" from), you can still receive less than the entire file in a single receive. The buffer is an upper bound for how much data can be buffered, not a guarantee of how much data _will_ be buffered before it's delivered to a read operation. – Peter Duniho Dec 24 '14 at 01:03

2 Answers2

7

It looks like your client code does not finish reading the entire content: you should have a loop that reads to the end before checking the content:

int remaining = d.Length;
int pos = 0;
while (remaining != 0) {
    int add = networkStream.Read(d, pos, remaining);
    pos += add;
    remaining -= add;
}

Currently, your code reads as much data as the network stream chooses to make initially available to you, and stop without retrieving the rest of the bytes.

Royi Namir
  • 144,742
  • 138
  • 468
  • 792
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • But what if the buffer is exactly as the size of my data ? doesn't it reads as much as the buffer size ? – Royi Namir Dec 24 '14 at 00:48
  • Are you sure ? about `remaining = d.Length` and `networkStream.Read(d` ??one suppose to be the buffer , the other suppose to be size? – Royi Namir Dec 24 '14 at 01:07
  • 1
    @RoyiNamir : If you look at the docs for `Stream.Read`, you'll see that it returns a value "actual bytes read". Only when this returns `0` can you be certain that you have read the entire stream. You should always use the returned value to see how much you have read. Given that the default buffer length for `NetworkStream` is 8192 (IIRC), you're never going to get a single read larger than this value. Never assume you read what you requested. The return value is what you need to look at. – spender Dec 24 '14 at 01:15
  • `Stream.Read` returns "The total number of bytes read into the buffer. This can be less than the number of bytes requested if that many bytes are not currently available, or zero (0) if the end of the stream has been reached." http://msdn.microsoft.com/en-us/library/system.io.stream.read%28v=vs.110%29.aspx – spender Dec 24 '14 at 01:16
  • Just to check : I don't need this attitude with `Write` right ? the problem is only with Read...right? ( I mean - I don't need write in some kind of loop right )? – Royi Namir Dec 24 '14 at 01:27
  • @RoyiNamir You're good on the writing side. You don't need to call `Flush` either, [because it has no effect on `NetweorkStream`s](http://msdn.microsoft.com/en-us/library/system.net.sockets.networkstream.flush(v=vs.110).aspx). – Sergey Kalinichenko Dec 24 '14 at 01:32
3

It's not good idea to read all bytes at once. You should read data by blocks, something like this:

buffer = new byte[4096];
int offset = 0;
do {
  int read = networkStream.Read(buffer, offset, buffer.Length - offset);
  offset += read;
} while (read != 0);
yesenin
  • 199
  • 7