1

Here is my TCP listener that reads the data received:

    private void Update()
    {
        //Console.WriteLine("Call");
        if (!serverStarted)
        {
            return;
        }

        foreach (ServerClient c in clients.ToList())
        {
            // Is the client still connected?
            if (!IsConnected(c.tcp))
            {
                c.tcp.Close();
                disconnectList.Add(c);
                Console.WriteLine(c.connectionId + " has disconnected.");
                CharacterLogout(c.connectionId);
                continue;
                //Console.WriteLine("Check for connection?\n");
            }
            else
            {
                // Check for message from Client.

                NetworkStream s = c.tcp.GetStream();
                if (s.DataAvailable)
                {
                    StreamReader reader = new StreamReader(s, true);
                    string data = reader.ReadLine();

                    if (data != null)
                    {
                        OnIncomingData(c, data);
                    }
                }
                //continue;
            }
        }

        for (int i = 0; i < disconnectList.Count - 1; i++)
        {
            clients.Remove(disconnectList[i]);
            disconnectList.RemoveAt(i);
        }


    }

    private bool IsConnected(TcpClient c)
    {
        try
        {
            if (c != null && c.Client != null && c.Client.Connected)
            {
                if (c.Client.Poll(0, SelectMode.SelectRead))
                {
                    return !(c.Client.Receive(new byte[1], SocketFlags.Peek) == 0);
                }

                return true;
            }
            else
            {
                return false;
            }
        }
        catch
        {
            return false;
        }
    }

    private void StartListening()
    {
        server.BeginAcceptTcpClient(OnConnection, server);
    }

Normally this is working fine. For example when i send one request per 2, 3 second or in bigger interval of time everything is working fine.

However when i send requests for example 2, 3 requests per 1 second or even less in time, in the beginning it works fine and after a while it just cut's the data received.

If i send requests very often i receive the problem. Here is example of the data received.

Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"257.8289","position.z":"251.5683","position.y":"2.798551"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"257.8895","position.z":"251.6447","position.y":"2.796823"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"257.9507","position.z":"251.7218","position.y":"2.795079"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.0536","position.z":"251.8516","position.y":"2.792141"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.2184","position.z":"252.0594","position.y":"2.793153"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.2699","position.z":"252.1243","position.y":"2.795335"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.3738","position.z":"252.2554","position.y":"2.799742"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.5936","position.z":"252.5325","position.y":"2.80906"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.7353","position.z":"252.7112","position.y":"2.815068"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.7883","position.z":"252.778","position.y":"2.817239"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"258.9598","position.z":"252.9942","position.y":"2.809892"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.0203","position.z":"253.0705","position.y":"2.806783"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.0894","position.z":"253.1577","position.y":"2.803234"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.2439","position.z":"253.3524","position.y":"2.795195"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.4146","position.z":"253.5677","position.y":"2.784776"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.5829","position.z":"253.7799","position.y":"2.772557"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.7452","position.z":"253.9845","position.y":"2.762063"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"259.8754","position.z":"254.1487","position.y":"2.760789"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"260.0437","position.z":"254.3609","position.y":"2.759143"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"260.2104","position.z":"254.5711","position.y":"2.757511"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"260.5002","position.z":"254.8992","position.y":"2.756335"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"260.9822","position.z":"255.1375","position.y":"2.765062"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"261.5056","position.z":"255.2772","position.y":"2.787397"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"261.5968","position.z":"255.3015","position.y":"2.791161"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"261.7819","position.z":"255.3509","position.y":"2.797171"},"connectionId":1}
Character position Updated
{"header":"1x008","data":{"characterId":"1","position.x":"261.8941","position.z":"255.3809","position.y":"2.799694"},"connectionId":1}
Character position Updated
ion.z":"255.5631","position.y":"2.815049"},"connectionId":1}

As you can see in the last line the JSON is broken. The data is reduced without reason and it brakes my code.

I can confirm that the client send valid JSON string for the last line that appears cutted on the server/listener side.

I think it may be something releated with buffer size or something but i'm very new at this TCP communications.

This is the send function from the client:

public void Send(string header, Dictionary<string, string> data)
{

    if (stream.CanRead)
    {
        socketReady = true;
    }

    if (!socketReady)
    {
        return;
    }
    JsonData SendData = new JsonData();
    SendData.header = "1x" + header;
    foreach (var item in data)
    {
        SendData.data.Add(item.Key.ToString(), item.Value.ToString());
    }
    SendData.connectionId = connectionId;

    string json = JsonConvert.SerializeObject(SendData);
    var howManyBytes = json.Length * sizeof(Char);
    writer.WriteLine(json);
    writer.Flush();

    Debug.Log("Client World:" + json);
}

Where is my mistake and how can i fix it ?

Venelin
  • 2,905
  • 7
  • 53
  • 117
  • Take a look at [Broken TCP messages](https://stackoverflow.com/q/7257139/3744182) and [Can some data be separated in a Tcp connection?](https://stackoverflow.com/q/24783218/3744182), as well as the article [Message Framing](https://blog.stephencleary.com/2009/04/message-framing.html) by Stephen Cleary. – dbc Nov 19 '17 at 19:43
  • Do you mean that on every message send from the client the client should send the size of the message in bytes and then when the server receives that message he will know the size of the message and if it's not equal he will put this part of the message until the message completes to the size ? – Venelin Nov 19 '17 at 19:57
  • Yes, that is the solution recommended in those answers and the linked article. – dbc Nov 19 '17 at 21:05
  • I have added my `Send` function to my question. Is this how i get the size of the sending string ? By doing `var howManyBytes = json.Length * sizeof(Char);` ? Can you give me example how should my send function looks like so it sets a prefix with the size in bytes of my sending string ? – Venelin Nov 19 '17 at 21:09
  • Are you running that `Update` code in a loop? meaning: will a single socket get to `new StreamReader(s, true)` twice? if so: the problem is almost certainly that you lost some data in the data that the `StreamReader` had buffered, and that you discarded - i.e. it read *all* of one line, and *part* of the next – Marc Gravell Nov 19 '17 at 21:15
  • Take a look at https://blog.stephencleary.com/2009/04/sample-code-length-prefix-message.html which shows one such example. What you need to include is the number of **bytes** not the number of **characters**, so your best bet would be to serialize to a `MemoryStream` rather than a string as shown [here](https://stackoverflow.com/a/22689976) then use message framing for the byte array returned by [`MemoryStream.ToArray()`](https://msdn.microsoft.com/en-us/library/system.io.memorystream.toarray(v=vs.110).aspx). On the receiving end use the message length to read the correct # of bytes. – dbc Nov 19 '17 at 21:16
  • @MarcGravell yes i'm running `Update` code inside loop like this `while (true) { serverInstance.Update(); }` – Venelin Nov 19 '17 at 21:18
  • @TonyStark well, then there's your answer... – Marc Gravell Nov 19 '17 at 21:21
  • @MarcGravell i don't get it. What you suggest to change? Can you make a complete answer so i can accept it – Venelin Nov 19 '17 at 21:22
  • @TonyStark I'm preparing an answer to explain – Marc Gravell Nov 19 '17 at 21:22
  • @dbc can you please check this question: https://stackoverflow.com/questions/47382549/how-to-create-tcp-message-framing-for-the-stream Thanks – Venelin Nov 19 '17 at 22:13

1 Answers1

1

A StreamReader contains two internal buffers - one of characters that have been decoded but not yet consumed, and one of bytes that have been received but not yet decoded (perhaps incomplete characters for multi-byte encodings). When it needs data, it first checks the buffers, and if it doesn't have what it needs, then it reads "some" data from the stream until it can complete the current request (ReadLine() or whatever). In some cases with sparse messages that might mean it reads a single message in its entirety, which can incorrectly suggest that your code is working. However, when multiple messages come close together, it can over-read from the socket - receiving all of one message and part of another. This means that it is very important to retain the StreamReader per socket - otherwise when you drop it on the floor, you lose whatever data was consumed from the socket but not read in the previous ReadLine(). This would have exactly the result you see. So: as a simple fix: instead of storing sockets, store a tuple of the sockets with their StreamReader - and only ever create one reader per socket.

Alternatively, you could write better "framing" code and manually buffer data (somewhere) until your frame is complete (end of line in your case). Then process the back-buffer, being careful to retain any additional bytes left after the frame. If you're not familiar with network code, the first option is probably simpler. The second option is better, though - it avoids blocking when a partial frame is received. Currently a single client could kill your server by sending some text without a line-feed and just keeping the socket open (maybe dribbling in an extra character every 30 seconds). This would cause your server to stop at ReadLine() forever - or until your server OOMs because if incoming text. Heck, send a huge block of text without line feeds!


This could be as simple as (using C# vCurrent)

List<(Socket socket, StreamReader reader)> sockets = ...

...


// (adding)
var newSocket = ...
var reader = new StreamReader(...); // on newSocket
sockets.Add((newSocket, reader));
// ...

foreach(var pair in sockets) {
    // ...
    var socket = pair.socket;
    var line = pair.reader.ReadLine();
    if(line != null) {...}
    // ...
}
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • Thank you very much for your detailed answer!Can you show me how to tore a tuple of the sockets *with their StreamReader ? I mean can you add it to you answer ? – Venelin Nov 19 '17 at 21:34
  • I'll mark your answer as an answer but i'll got with message Framing. Thank you once again. Is this a good guide showing good message framing: https://youtu.be/J9ae1abjbKM?list=PL-sMjTtTQ_8d-7weAFuK2Yf_jfv9iH3AQ ? – Venelin Nov 19 '17 at 21:47
  • @TonyStark something else to watch out for: you are currently checking `.DataAvailable` on the socket - which tells you how much is buffered locally on the socket - but you *could* have multiple lines already buffered in the reader - so *not* processing just because `.DataAvailable == 0` could lead to missed messages (especially missed *final* messages). AFAIK, there's no easy way to see what is buffered in a `StreamReader`. This makes it even more compelling to simply *not use `StreamReader` at all*, and deal with the frames manually – Marc Gravell Nov 19 '17 at 21:47
  • Yes i think i'll go with message framing. Can you check the guide i provided in the link aboce and say what do you think about it ? Is it good to go with that approach ? – Venelin Nov 19 '17 at 21:48
  • @TonyStark frankly, I'm not going to watch a half hour video - I'm a text-focused person, but from what I could see: it is doing some crazy weird stuff with `ArrayList` as the back-buffer, which is pretty inefficient - and it looked like it was also using `BinaryFormatter` which made me very nervous. This stuff isn't crazy hard, though - ultimately it is `ReadSomeDataAndAppendToBackBuffer(); while(BackBufferHasEntireFrame()) { ProcessFrameFromBackBuffer(); }` - the tricky bit is staying in bytes until you have an entire frame – Marc Gravell Nov 19 '17 at 21:52
  • Can you please check this question: https://stackoverflow.com/questions/47382549/how-to-create-tcp-message-framing-for-the-stream Thanks – Venelin Nov 19 '17 at 22:13