Reputation: 13335
The below works fine when there is data returned. It is valid however for an empty stream to be returned at which point the below hangs on stream.read(). How can I refactor it to cater for both cases?
using(TcpClient client = new TcpClient(_server, _port))
using(NetworkStream stream = client.GetStream())
{
stream.Write(data, 0, data.Length);
byte[] myReadBuffer = new byte[1024];
int numberOfBytesRead = 0;
var message = new List<Byte> ();
do{
numberOfBytesRead = stream.Read(myReadBuffer, numberOfBytesRead, myReadBuffer.Length);
message.AddRange(myReadBuffer.Take(numberOfBytesRead));
}
while(stream.DataAvailable);
return message.ToArray();
}
Upvotes: 1
Views: 1283
Reputation: 171236
You can never tell whether more data will be coming or whether the stream is depleted. The remote side must signal you the end of the stream.
By closing or shutting down the socket you will read 0
bytes which is your signal to stop reading.
Or, make the remote side send data signaling the end of the stream.
Your use of DataAvailable
is a bug (99% of such uses are bugs). DataAvailable
is poison. It tells you how much data is ready to be read in a non-blocking way right now. 1us later the answer might be different.
You need to throw this away.
Upvotes: 1
Reputation: 70701
If I understand the code example correctly, all you really want to do is read all of the bytes from the socket (i.e. the TCP connection) and then return them as a byte[]
. If that's the case, then this would be much better:
using(TcpClient client = new TcpClient(_server, _port))
using(NetworkStream stream = client.GetStream())
using(MemoryStream output = new MemoryStream())
{
stream.CopyTo(output);
return output.ToArray();
}
As a general rule, I strongly advise against using the DataAvailable
property. In this case, it's especially inappropriate because all it does is check to see how many bytes can be read from the stream and if it's non-zero, returns true
. An empty stream will never have bytes available, and for that matter you will never detect the end of the stream even when the stream isn't empty (i.e. there's no way to tell when the other end is done sending you data).
If I've misunderstood and you really just want to return the bytes that are immediately available, then I can't provide an answer without more context. The correct way to implement this would be to use asynchronous I/O, with a layer that understands the overall structure of the stream (i.e. where data boundaries are), but the details required to implement that aren't present in your question, so it's not possible to provide specific advice along those lines.
Upvotes: 1