Server/Client. The Client uses a lot of memory

Question

This is my first question and I hope you can help me.

I have 2 programs, a Server and a Client and my problem is in the Client.

After 2 or 3 days of running, it uses more than 300MB of RAM (I can tell this by seeing the TaskManager) and never releases it! Also, I have to say that this Client receives data every second from GPS devices. I have analyzed my Client with the ANTS Memory Profiler and I noticed that I create an object several times and they are never destroyed.

Here is my code:

private static TcpClient _client;
private readonly ManualResetEvent _receiveDone = new ManualResetEvent(false);

public void Receive()
{
    if (_client.Connected)
    {
        _receiveDone.Reset();
        ObjectState state = new ObjectState(); 
        state.WorkSocket = _client.Client;
        state.Data = new byte[_client.ReceiveBufferSize];
        _client.Client.BeginReceive(state.Data, 0, Convert.ToInt32(_client.ReceiveBufferSize), 0, ReceiveCallback, state);

        if (!_receiveDone.WaitOne(20000)) 
        {
            //after 20 seconds WITHOUT RECEIVING DATA, do some code to test if the connection is alive                        
        }
    }
}

void ReceiveCallback(IAsyncResult ar)
{
    ObjectState state = (ObjectState)ar.AsyncState;
    Socket client = state.WorkSocket;
    if (client.Connected)
    {
        int bytesRead = client.EndReceive(ar);
        if (bytesRead > 0)
        {
            string response = Encoding.ASCII.GetString(state.Data, 0, bytesRead);
            doProcess(response);
            client.BeginReceive(state.Data, 0, Convert.ToInt32(_client.ReceiveBufferSize), 0, ReceiveCallback, state);
            _receiveDone.Set();
        }
        else
        {
        //Disconnection
        }
    }
}

public class ObjectState
{
    public Socket WorkSocket;
    public byte[] Data;
}

The ANTS Memory Profiler tells me I have thousands of live instances of byte[]. (because I always create new instances of ObjectState)

First thing I wanted to do: Dispose all the ObjectState that I create after I call BeginReceive, but I only get the first message.

Then I wanted to stop using the ObjectState... How? This is my modified code:

private _data byte[];

public void Receive()
{
    if (_client.Connected)
    {
        _receiveDone.Reset();
        _data = new byte[_client.ReceiveBufferSize];
        _client.Client.BeginReceive(_data, 0, Convert.ToInt32(_client.ReceiveBufferSize), 0, ReceiveCallback, null);

        if (!_receiveDone.WaitOne(20000)) 
        {
            //after 20 seconds of inactivity do some code to test if the connectio is alive                        
        }
    }
}

void ReceiveCallback(IAsyncResult ar)
{
    Socket client = _cliente.Client;
    if (client.Connected)
    {
        int bytesRead = client.EndReceive(ar);
        if (bytesRead > 0)
        {
            string response = Encoding.ASCII.GetString(_data, 0, bytesRead);
            doProcess(response);
            client.BeginReceive(_data, 0, Convert.ToInt32(_client.ReceiveBufferSize), 0, ReceiveCallback, null);
            _receiveDone.Set();
        }
        else
        {
            //Disconnection
        }
    }
}

What's wrong with this? I only get the first message, so this is not good. Then if I remove the _receiveDone.Set it gets all the messages, BUT the _receiveDone.WaitOne(2000) always is executed every 20 seconds, no matter if I actually am receiving data, and this is not good either.

So my question is, what can I do to reduce the using of so much RAM? I hope this explains well what my problem is.

EDIT: I uploaded these images, I hope they can be helpful too.

Answer 1

I think the primary problem is that you're depending on BeginReceive to return 0 bytes when there's nothing available. But that's not how it works. BeginReceive will block until there is data available, or until the client disconnects.

In the first case, the ReceiveCallback passes the state object to client.BeginReceive. Now you have an ObjectState instance out there waiting for data from the client. The _receiveDone event is set, so your Receive method exits but the client connection is still open and you have a pending asynchronous read.

The next time Receive is called, it will create another ObjectState instance and issue another asynchronous read request.

The second case is pretty scary. Your _data array is at class scope and gets reallocated every time. So it's possible that an asynchronous read can fill the buffer you pass it, but then the data is lost when you get to ReceiveCallback.

Your code only does the timeout check for the first receive. After that, things can hang indefinitely. I would suggest something more like:

private _data byte[];
private bool _receiving = false;

public void StartReceiving()
{
    if (_receiving)
    {
        // ERROR: Already receiving
        throw new InvalidOperationException();
    }
    _receiving = true;
    Receive();
}

public void Receive()
{
    if (_client.Connected)
    {
        _data = new byte[_client.ReceiveBufferSize];
        var ir _client.Client.BeginReceive(_data, 0, Convert.ToInt32(_client.ReceiveBufferSize), 0, ReceiveCallback, null);

        if (!_ir.WaitOne(20000)) 
        {
            //after 20 seconds of inactivity do some code to test if the connectio is alive                        
        }
    }
}

void ReceiveCallback(IAsyncResult ar)
{
    Socket client = _cliente.Client;
    int bytesRead;
    try
    {
        bytesRead = client.EndReceive(ar);
        if (bytesRead > 0)
        {
            string response = Encoding.ASCII.GetString(_data, 0, bytesRead);
            doProcess(response);
            // Calling Receive here ensures that the timeout check happens
            // with every read.
            Receive();
        }
        else
        {
            _receiving = false;
        }
    }
    catch
    {
        // Catch SocketException and others here
    }
}

Note that you don't need the _receiveDone event. You can check it with the IAsyncResult.WaitHandle.

Answer 2

if (client.Connected)
{
    int bytesRead = client.EndReceive(ar);
    // etc..

Calling EndReceive is not optional, you have to call it or you'll leak resources. Use try/catch to catch an ObjectDisposedException.