Best Way to write large data over network stream of TcpClient

Question

We have a requirement to upload large firmware files to printers to upgrade the firmware of the device. The printer device is in the same network as my server, and the size of firmware that we are trying to upload is approximately to 200 - 500 MB. The approach that we have chosen is to load the firmware (.bin file) into Memory stream and write it in chunks over the network using TcpClient.

Based on the response from the network stream, we are displaying the status of firmware upgrade to the client. Following is the code snippet that we have used for firmware upgrade. I want to know if it is the best approach, as wrong one may harm the device.

EDIT:

class MyClass
{
    int port = 9100;
    string _deviceip;
    byte[] m_ReadBuffer = null;
    TcpClient _tcpclient;
    NetworkStream m_NetworkStream;
    static string CRLF = "\r\n";

    public event EventHandler<DeviceStatus> onReceiveUpdate;

    public async Task<bool> UploadFirmware(Stream _stream)
    {
        bool success = false;
        try
        {
            _tcpclient = new TcpClient();
            _tcpclient.Connect(_deviceip, port);

            _stream.Seek(0, SeekOrigin.Begin);
            m_NetworkStream = _tcpclient.GetStream();
            byte[] buffer = new byte[1024];
            m_ReadBuffer = new byte[1024];
            int readcount = 0;
            m_NetworkStream.BeginRead(m_ReadBuffer, 0, m_ReadBuffer.Length,
                                     new AsyncCallback(EndReceive), null);
            await Task.Run(() =>
            {
                while ((readcount = _stream.Read(buffer, 0, buffer.Length)) > 0)
                {
                    m_NetworkStream.Write(buffer, 0, readcount);
                    m_NetworkStream.Flush();
                }
            });
            success = true;
        }
        catch (Exception ex)
        {
            upgradeStatus = false;
        }
        return success;
    }

     private void EndReceive(IAsyncResult ar)
     {
         try
         {
             int nBytes;
             nBytes = m_NetworkStream.EndRead(ar);
             if (nBytes > 0)
             {
                 string res = Encoding.UTF8.GetString(m_ReadBuffer, 0, nBytes);
                 DeviceStatus status = new DeviceStatus();

                 string[] readlines = res.Split(new string[] { CRLF }, 
                              StringSplitOptions.RemoveEmptyEntries);
                 foreach (string readline in readlines)
                 {
                     if (readline.StartsWith("CODE"))
                     {
                         //read readline string here
                         break;
                     }
                 }
             }

            if (m_NetworkStream.CanRead)
            {
                do
                {
                    m_NetworkStream.BeginRead(m_ReadBuffer, 0, m_ReadBuffer.Length, new 
                                           AsyncCallback(EndReceive), null);
                } while (m_NetworkStream.DataAvailable);
            }
         }
         catch (ObjectDisposedException ods)
         {
             return;
         }
         catch (System.IO.IOException ex)
         {
         }
     }
}

Any help will be really appreciated.

Answer 1

Your code is basically fine with a few issues:

1. m_NetworkStream.Flush(); AFAIK this does nothing. If it did something it would harm throughput. So delete that.
2. _stream.Seek(0, SeekOrigin.Begin); seeking is the callers concern, remove that. This is a layering violation.
3. Use bigger bigger buffers. Determine the right size experimentally. I usually start at 64KB for bulk transfers. This makes the IOs less chatty.
4. Turn on nagling which helps with bulk transfers because it saves you from spurious small packets.
5. You can replace the entire read-write-loop with Stream.Copy.
6. The way you report exceptions to the callers hides a lot of information. Just let the exception bubble out. Don't return a bool.
7. Use using for all resource to ensure they are cleaned up in the error case.
8. nBytes = m_NetworkStream.EndRead(ar); here, you assume that a single read will return all data that will be coming. You might receive just the first byte, though. Probably, you should use StreamReader.ReadLine in a loop until you know you are done.
9. catch (System.IO.IOException ex) { } What is that about? If firmware updates are such a critical thing suppressing errors appears very dangerous. How else can you find out about bugs?
10. I would convert the reading code to use await.