Why is my C# TCP client receiving garbage sometimes?

Question

If the server sends 2 items of data at almost the same time, the client receives the 2nd item of data. It will have about 75% to receive garbage.

I'm pretty sure the problem is in the C# client:

1. I've printed the buffer in the server - everything is correct
2. Used Wireshark to check the buffer - everything is correct
3. And used Python to receive the data instead of using C# - seems to be correct

I tried:

1. Clear the buffer when receiving - nothing happened
2. Adding a delay before the receive functions - it worked. But I'm not sure how long I should wait. And because of the probability of getting the correct data, it might not be the correct way to fix this. Also, as a game I want to receive data as quickly as it can.

This is the code about network in the client:

(This client uses GODOT - I've deleted parts of the code that isn't about receiving data)

public partial class Network : Node
{
    public bool error = false; // Happened Error.

    private Socket socket = null; // The Socket File.
    private Change change = new Change(); // The Changer To Change Packages And Bytes.
    private byte[] buffer = new byte[64]; // The Buffer.
    private List<Tuple<object, PackageType>> packageGet = new List<Tuple<object, PackageType>>(); // The Packages Got And Not Handled.

    public override void _ExitTree()
    {
        close();
    }

        public override void _Process(double delta) // 
        {
            if (!updated && packageGet.Count > 0)
            {
                handleEvent();
            }
        }

    /**
     * @brief Connect To The Server.
     * @param ipParameter The IP Address.
     * @return The Result.
     */
    public bool connect(string ipParameter)
    {
        close();
        socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

        try
        {
            if (!socket.ConnectAsync(ipParameter, 10000).Wait(ConfigPanel.connectTime))
            {
                return false;
            }
        }
        catch
        {
            return false;
        }

        startRead();
        
        return true;
    }

    /**
     * @brief Close The Socket.
     */
    public void close()
    {
        if (socket != null && socket.Connected)
        {
            socket.Shutdown(SocketShutdown.Both);
            socket.Close();
        }
    }

    /**
     * @breif Start To Read Data From Server.
     */
    private void startRead()
    {
        Array.Clear(buffer, 0, buffer.Length);
        socket.BeginReceive(buffer, 0, buffer.Length, SocketFlags.None, read, null);
    }

    /**
     * @breif Read Data Got (Callback From startRead()).
     * @param resultParameter The Result.
     */
    private void read(IAsyncResult resultParameter)
    {
        try
        {
            if (!socket.Connected || socket == null)
            {
                return;
            }

            int length = socket.EndReceive(resultParameter);

            if (length == 0)
            {
                close();
// Error
                return;
            }

            PackageType type = PackageType.None;
            object package = change.toPackage(buffer, ref type);
            packageGet.Add(new Tuple<object, PackageType>(package, type));
            startRead();
        }
        catch
        {
// Error
        }
    }

    /**
     * @brief Handle The Events.
     */
    private void handleEvent()
    {
                // Just Handle
    }
}

Sorry for the bad indentation :(, Thanks for helping.

Answer 1

TCP is a streaming protocol, not a message protocol; when calling Read/Receive/etc:

• the number of bytes must be respected - everything beyond that is (as you say): garbage
• your message can be fragmented over any number of receive calls: all that is guaranteed is that each receive will get at least one byte, or an EOF

as such, you need to a: respect length, and b: implement some kind of framing protocol; usually this means something like "the first 4 bytes of any message are the length, in little-endian int32, of the message that follows", so: you'd buffer at least 4 bytes (possibly over 4 receive calls, although in reality: usually 1), parse the length, then buffer at least that many bytes (possibly over many receive calls), and then parse the message. If you have over-read (i.e. in the "at least", you consumed more): then you need to retain that backlog too.

Honestly, it is easier to use the "pipelines" model, which simplifies all this buffer management; there's a 3-and-a-bit series of mine here on that topic, although it is quite a paradigm shift.

Answer 2

Please take my answer with caution as i am a beginner myself, and my answer might be wrong. My answer is maybe not what you expect, and maybe cou cant even use it with your approach, but i still feel like leaving it here, as it might help you maybe?

I recently did a chat application using tcp, but i receive messages a bit different: Instead of your socket.BeginReceive(buffer, 0, buffer.Length, SocketFlags.None, read, null);

I used var amountBytes = await _clientSocket.ReceiveAsync(bytes); Like already said, tcp doesnt guarantee to send something in chucks you like to. So maybe its wise to put a identifyer at the end of each complete Datamodel, so you can use it like this:

string decodedMessage = Encoding.UTF8.GetString(bytes, 0, amountBytes);
                    buffer += decodedMessage;

                    //If we already received a complete message model
                    if (buffer.Contains(_endOfMessageIdentifyer))
                    {
                        //get the index of the start of the identifyer 
                        int index = decodedMessage.IndexOf(_endOfMessageIdentifyer);
                        //Cut everything before the identifyer out
                        string completeDatamodel = decodedMessage.Substring(0, index);
                        //Cut everything after the identifyer out
                        string remainingData = decodedMessage.Substring(index + _endOfMessageIdentifyer.Length);

                        buffer = remainingData;

With this approach, you are safe to always have exact one datamodel which you can process.