bretddog
bretddog

Reputation: 5519

TcpClient read OutOfMemoryException

I have problem with intermittent OutOfMemoryException, on the line

buffer = new byte[metaDataSize];

(Under //Read the command's Meta data.)

Does this mean I try to read the complete message while only part of it has been received? In case, what is a reliable way to handle that? Btw, I need variable length messages, as most are very short, while occasional messages are very large. Should I attach the complete message size in front of the message? Still, how can I know how much the stream contains before trying to read from it? (as it seems the read sometimes fails when trying to read a specific length, like I currently do)

    public static Command Read(NetworkStream ns)
    {
        try
        {
                //Read the command's Type.
                byte[] buffer = new byte[4];
                int readBytes = ns.Read(buffer, 0, 4);
                if (readBytes == 0)
                    return null;
                CommandType cmdType = (CommandType)(BitConverter.ToInt32(buffer, 0));

                //Read cmdID
                buffer = new byte[4];
                readBytes = ns.Read(buffer, 0, 4);
                if (readBytes == 0)
                    return null;
                int cmdID = BitConverter.ToInt32(buffer, 0);

                //Read MetaDataType
                buffer = new byte[4];
                readBytes = ns.Read(buffer, 0, 4);
                if (readBytes == 0)
                    return null;
                var metaType = (MetaTypeEnum)(BitConverter.ToInt32(buffer, 0));

                //Read the command's MetaData size.
                buffer = new byte[4];
                readBytes = ns.Read(buffer, 0, 4);
                if (readBytes == 0)
                    return null;
                int metaDataSize = BitConverter.ToInt32(buffer, 0);

                //Read the command's Meta data.
                object cmdMetaData = null;
                if (metaDataSize > 0)
                {
                    buffer = new byte[metaDataSize];

                    int read = 0, offset = 0, toRead = metaDataSize;
                    //While 
                    while (toRead > 0 && (read = ns.Read(buffer, offset, toRead)) > 0)
                    {
                        toRead -= read;
                        offset += read;
                    }
                    if (toRead > 0) throw new EndOfStreamException();

                    // readBytes = ns.Read(buffer, 0, metaDataSize);
                    //if (readBytes == 0)
                    //    return null;
                    // readBytes should be metaDataSize, should we check? 

                    BinaryFormatter bf = new BinaryFormatter();
                    MemoryStream ms = new MemoryStream(buffer);
                    ms.Position = 0;
                    cmdMetaData = bf.Deserialize(ms);
                    ms.Close();
                }
                //Build and return Command
                Command cmd = new Command(cmdType, cmdID, metaType, cmdMetaData);

                return cmd;
        }
        catch (Exception)
        {

            throw;
        }

    }

WRITE method:

    public static void Write(NetworkStream ns, Command cmd)
    {
        try
        { 

            if (!ns.CanWrite)
                return;

            //Type [4]
            // Type is an enum, of fixed 4 byte length. So we can just write it.
            byte[] buffer = new byte[4];
            buffer = BitConverter.GetBytes((int)cmd.CommandType);
            ns.Write(buffer, 0, 4);
            ns.Flush();

            // Write CmdID, fixed length [4]
            buffer = new byte[4];                    // using same buffer
            buffer = BitConverter.GetBytes(cmd.CmdID);
            ns.Write(buffer, 0, 4);
            ns.Flush();

            //MetaDataType [4]
            buffer = new byte[4];
            buffer = BitConverter.GetBytes((int)cmd.MetaDataType);
            ns.Write(buffer, 0, 4);
            ns.Flush();

            //MetaData (object) [4,len]
            if (cmd.MetaData != null)
            {
                BinaryFormatter bf = new BinaryFormatter();
                MemoryStream ms = new MemoryStream();
                bf.Serialize(ms, cmd.MetaData);

                ms.Seek(0, SeekOrigin.Begin);

                byte[] metaBuffer = ms.ToArray();
                ms.Close();

                buffer = new byte[4];
                buffer = BitConverter.GetBytes(metaBuffer.Length);
                ns.Write(buffer, 0, 4);
                ns.Flush();

                ns.Write(metaBuffer, 0, metaBuffer.Length);
                ns.Flush();

                if (cmd.MetaDataType != MetaTypeEnum.s_Tick)
                    Console.WriteLine(cmd.MetaDataType.ToString() + " Meta: " + metaBuffer.Length);
            }
            else
            {
                //Write 0 length MetaDataSize
                buffer = new byte[4];
                buffer = BitConverter.GetBytes(0);
                ns.Write(buffer, 0, 4);
                ns.Flush();
            }

        }
        catch (Exception)
        {

            throw;
        }
    }

VB.NET:

Private tcp As New TcpClient 
Private messenger As InMessenger    
Private ns As NetworkStream 

Public Sub New(ByVal messenger As InMessenger)
    Me.messenger = messenger
End Sub

Public Sub Connect(ByVal ip As String, ByVal port As Integer)

    Try
        tcp = New TcpClient


        Debug.Print("Connecting to " & ip & " " & port)

        'Connect with a 5sec timeout
        Dim res = tcp.BeginConnect(ip, port, Nothing, Nothing)
        Dim success = res.AsyncWaitHandle.WaitOne(5000, True)

        If Not success Then
            tcp.Close()

        Else
            If tcp.Connected Then
                ns = New NetworkStream(tcp.Client)

                Dim bw As New System.ComponentModel.BackgroundWorker
                AddHandler bw.DoWork, AddressOf DoRead
                bw.RunWorkerAsync()

            End If
        End If


    Catch ex As Exception
        Trac.Exception("Connection Attempt Exception", ex.ToString)
        CloseConnection()
    End Try
End Sub


Private Sub DoRead()

    Try
        While Me.tcp.Connected

            ' read continuously : 
            Dim cmd = CommandCoder.Read(ns)

            If cmd IsNot Nothing Then
                HandleCommand(cmd)
            Else
                Trac.TraceError("Socket.DoRead", "cmd is Nothing")
                CloseConnection()
                Exit While
            End If

            If tcp.Client Is Nothing Then
                Trac.TraceError("Socket.DoRead", "tcp.client = nothing")
                Exit While
            End If
        End While
    Catch ex As Exception
        Trac.Exception("Socket.DoRead Exception", ex.ToString())
        CloseConnection()
        EventBus.RaiseErrorDisconnect()
    End Try

End Sub

EDIT:

I put in some WriteLine's, and found that some packages sent are recognized with wrong size on receiver side. So the metaDataSize which should be 9544 for a certain message, is read as 5439488, or similar incorrect value. I assume on few occations this number is so large it causes the OutOfMemoryException.

Seems Douglas' answer may be on the mark(?), I will test. For info: The server (sender) program is built as "Any CPU", ran on Windows 7 x64 pc. While the Client (receiver) is built as x86, and (during this test) ran on XP. But must also be coded to work on other windows x86 or x64.

Upvotes: 1

Views: 1876

Answers (2)

Douglas
Douglas

Reputation: 54887

You need to pay attention to the endianness of your architecture, particularly since the behaviour of BitConverter is architecture-dependant. As it stands, your code probably fails when you are transmitting data between architectures of different endianness. Imagine, for example, a message whose size is 241 bytes. The sender – who we shall assume to be big-endian – would indicate this size by sending a byte sequence of [0,0,0,241]. This will be correctly interpreted as 241 on a big-endian receiver, but as 4,043,309,056 (equal to 241×2563) on a little-endian one. If you try to allocate a byte array that large, you would most likely get an OutOfMemoryException.

Assuming that your incoming stream is always big-endian, you handle this by adapting your code to reverse the array when your architecture is little-endian:

buffer = new byte[4];
readBytes = ns.Read(buffer, 0, 4);
if (readBytes == 0)
    return null;
if (BitConverter.IsLittleEndian)
    Array.Reverse(buffer);

Edit: Replying to comment:

You need to correct for endianness whenever you’re going to use the BitConverter.ToInt32 method to convert a 4-byte sequence into an integer. You don’t need to correct for endianness when using the BinaryFormatter, since it handles endianness transparently.

I assume that endianness depends on your machine’s physical architecture, but I’ve never studied the specifics. To be safe, you should never assume a particular endianness, irrespective of whether you’re running on x86 or x64.

If you’re responsible for the server code as well, you also need to correct for endianness there. For example, to send the cmdID value from the server:

int cmdID = 22;  // for the example

byte[] buffer = BitConverter.GetBytes(cmdID);
if (BitConverter.IsLittleEndian)
    Array.Reverse(buffer);
ns.Write(buffer, 0, 4);

Upvotes: 2

CodesInChaos
CodesInChaos

Reputation: 108800

You're talking about packets, but that's not a concept exposed by TCP. TCP exposes a stream of bytes, nothing more. It doesn't care how many Send calls there were. It can split one Send call into multiple reads, and merge multiple sends, or a mix of these.

The return value of Read tells you how many bytes were read. If this value is larger than 0, but smaller than the length you passed to Read, you got less bytes then you passed it. Your code assumes that either 0 or length bytes were read. This is an invalid assumption.

Your code also suffers from endian issues, but I think both of your systems were little endian, so it's unlikely that this is causing your current problems.


If you don't care about blocking(Your existing code already blocks in the loop, so this is no additional issue) you can simply use a BinaryReader on your stream.

It has helper methods like ReadInt32 that automatically take care of partial reads, and it uses fixed endianness(always little).

buffer = new byte[4];
readBytes = ns.Read(buffer, 0, 4);
if (readBytes == 0)
    return null;
int cmdID = BitConverter.ToInt32(buffer, 0);

becomes:

int cmdId = reader.ReadInt32();

It will throw an EndOfStreamException if it unexpectedly encounters the end of the stream, instead of returning null.

Upvotes: 2

Related Questions