C# TcpClient not sending or reading 100% of data?

Question

Hey all. I'm writing a simple client/server application (just for the experience, networking is fairly new to me) where the client sends the server data and the server outputs it to a textbox. It all works fine, except for one small detail... It seems sometimes a connection is made, but the data isn't being sent or read (can't work out which) and thus nothing is being outputted in the textbox. Every time a connection is made a counter is incremented, same thing when a data block is received. When you compare the two, the number of connections is correct but the data counter is usually lower, sometimes by as much as half. Anyway, if anyone can give me some advice or point me in the right direction, it would be greatly appreciated!

Here's the code if you require it:

(SERVER_CODE)

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace Server
{
    public partial class Form1 : Form
    {
        public int Connections = 0;
        public int blocks = 0;
        public int threads = 0;
        public Thread MasterThread;
        public TcpListener Master;
        public volatile bool Run;

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_Load(object sender, EventArgs e)
        {

        }

        public void StartMaster()
        {
            Master = new TcpListener(IPAddress.Any, 1986);
            Master.Start();
            MasterThread = new Thread(new ThreadStart(RunMaster));
            MasterThread.Start();
        }

        public void RunMaster()
        {
            threads++;
            label6.Text = String.Format("{0}", threads);

            while (Run)
            {
                TcpClient client = Master.AcceptTcpClient();
                Connections++;
                label4.Text = String.Format("{0}", Connections);
                Thread ClientThread = new Thread(new ParameterizedThreadStart(RunClient));
                ClientThread.Start(client);
            }

            Master.Stop();

            threads--;
            label6.Text = String.Format("{0}", threads);
        }

        public void RunClient(object tcpClient)
        {
            TcpClient client = (TcpClient)tcpClient;
            byte[] buffer = new byte[4096];
            int byteCount = 0;
            NetworkStream stream = client.GetStream();
            threads++;
            label6.Text = String.Format("{0}", threads);

            while (Run)
            {
                try
                {
                    byteCount = stream.Read(buffer, 0, 4096);
                }
                catch
                {
                    //Connections--;
                    break;
                }

                if (byteCount == 0)
                {
                    //Connections--;
                    break;
                }

                blocks++;
                label5.Text = String.Format("{0}", blocks);

                textBox1.AppendText(Encoding.ASCII.GetString(buffer, 0, byteCount) + "\r\n");
            }

            client.Close();

            threads--;
            label6.Text = String.Format("{0}", threads);
        }

        private void button1_Click(object sender, EventArgs e)
        {
            Run = true;
            StartMaster();
        }

        private void button2_Click(object sender, EventArgs e)
        {
            Run = false;
        }
    }
}

(CLIENT_CODE)

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace Client
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            IPEndPoint endPoint = new IPEndPoint(IPAddress.Parse("127.0.0.1"), 1986);
            TcpClient client = new TcpClient();
            try
            {
                client.Connect(endPoint);
            }
            catch
            {
                MessageBox.Show("Connect Error");
            }

            NetworkStream stream = client.GetStream();
            byte[] data = Encoding.ASCII.GetBytes(textBox1.Text);
            stream.Write(data, 0, data.Length);
            stream.Flush();
            client.Close();
        }
    }
}

Thank-you,
Tristan!.

Answer 1

Well, to start with you're crippling your own diagnostics with this:

catch
{
    //Connections--;
    break;
}

Why are you swallowing exceptions without any logging etc? Maybe an exception is being thrown, and you have no way of knowing. Ideally you should catch specific exceptions, and when you do catch an exception at least log what's going on.

At the other end of the spectrum, Wireshark should help you to work out whether the data is being sent or not.

Answer 2

I haven't had a thorough look at your code yet, but after a quick glance, you access variables from multiple threads without proper locking. A statement like x++; has to read the value of x, increment it, and write it back. Now if you have two threads doing this, you might run into this situation:

x = 0

Thread 1           Thread 2
------------------------
Read (0)
                   Read (0)
Increment (1)
                   Increment (1)
Write (1)
                   Write (1)

=> x = 1 instead of 2

If you need to access variables from multiple threads, ALWAYS synchronize unless you know exactly what you're doing. For example, create and use a synchronization object like this:

int threads = 0;
object threadSync = new object();

...

lock (threadSync) {
    threads++;
}

Then only one thread may access the variable at a time and values are incremented correctly.

Edit: Another problem is that you access visible controls from a different thread than the one that created them. Early .NET versions allowed this, but the newer don't. If you need to update status messages, you need to look at the control's InvokeRequired property and if set to true, use Control.Invoke(...) to call a method that sets the property.