Problem with sockets and OutOfMemory error

Question

I have a huge problem. Trying to create an app that has to have two parts: server and client side. Those two parts have to communicate somehow and exchange objects. I have decides to use Sockets because i'm not familiar with WCF, and i can test both parts on same computer (just put them to listen at 127.0.0.1 address).

Now, when i try to send some "custom" serializable object from client i got "OutOfMemory" exception at server side! I read about Sockets, ways to send/receive objects, i have tried some code i found on net but no luck! I have no idea what's wrong with my code. Here's my code:

This is test class defined in code of both sides:

[Serializable]
    class MyClass
    {
        public string msg="default";
    }

Client-side sending code (works fine):

private void cmdSendData_Click(object sender, System.EventArgs e)
    {
        try
        {
            MyClass test = new MyClass();

            NetworkStream ns = new NetworkStream(m_socWorker); //m_socWorker is socket
            BinaryWriter bw = new BinaryWriter(ns);

            MemoryStream ms = new MemoryStream();
            BinaryFormatter bf = new BinaryFormatter();
            bf.Serialize(ms, test);

            bw.Write(ms.ToArray());
            MessageBox.Show("Length is: " + ms.ToArray().Length); //length is 152!
            ns.Close();
        }
        catch(System.Net.Sockets.SocketException se)
        {
            MessageBox.Show (se.Message );
        }
    }

Server-side code (the one that cause problems):

public  void OnDataReceived(IAsyncResult asyn)
    {
        try
        {
            CSocketPacket theSockId = (CSocketPacket)asyn.AsyncState ;

            NetworkStream ns = new NetworkStream(m_socWorker);
            byte[] buffer = new byte[1024];
            ns.Read(buffer, 0, buffer.Length);

            BinaryFormatter bin = new BinaryFormatter();
            MemoryStream mem = new MemoryStream(buffer.Length);

            mem.Write(buffer, 0, buffer.Length);
            mem.Seek(0, SeekOrigin.Begin);
            MyClass tst = (MyClass)bin.Deserialize(mem); //ERROR IS THROWN HERE!

            MessageBox.Show(tst.msg);

            theSockId.thisSocket.EndReceive(asyn);

            WaitForData(m_socWorker);
        }
        catch (ObjectDisposedException )
        {
            System.Diagnostics.Debugger.Log(0,"1","\nOnDataReceived: Socket has been closed\n");
        }
        catch(SocketException se)
        {
            MessageBox.Show (se.Message );
        }
    }

So, i got exception when i try to deserialize. Have no idea what's wrong.

I have threatened my code "if you continue causing problems i'll report you to StackOverflow guys" so here i'm :)

Did you look at your data? What's in mem? What's the value for buffer.length? etc. — Richard Brightwell, Jul 02 '11 at 18:17
@Richard re the last - a brutally enforced 1024; that doesn't reflect the actual data length, though :) — Marc Gravell, Jul 02 '11 at 18:23
@Marc Doh! Yup. I didn't read the code very closely, but was trying to point out some basic debugging techniques and info which would likely point to the source of his problem. — Richard Brightwell, Jul 02 '11 at 18:27

score 1 · Answer 1 · answered Jul 02 '11 at 18:17

There is some very odd code there that:

assumes we read 1024 bytes without checking
copies the 1024 buffer
assumes the serialized data is 1024 bytes, no more no less
deserializes from that

IMO there is your error; you should be reading the correct number of bytes from the stream (usually in a loop). Generally, you would be looping, checking the return value from Read until either we have read the amount of data we wanted, or we get EOF (return <= 0).

Or better; use serializers that do this for you... For example, protobuf-net has SerializeWithLengthPrefix and DeserializeWithLengthPrefix that handle all the length issues for you.

Since you mention "custom" serialization - if you are implementing ISerializable it is also possible that the problem is in there - but we can't see that without code. Besides, the current buffer/stream is so broken (sorry, but it is) that I doubt it is getting that far anyway.

Adam Robinson · Answer 2 · 2011-07-02T18:30:52.303

First, while I'm not certain if this is the cause of your issue directly, you have a serious issue with your reading logic.

You create a 1024 byte buffer and read into it without checking to see how much was actually read; if the incoming buffer only has 56 bytes, you'll only read 56 bytes (unless you use a blocking read on the socket itself, which could time out). Likewise, your buffer could have 2000 bytes in it, which means you'd have 976 bytes left in the buffer that won't get processed until you receive more data. That could be an entire object, and the client could be waiting on a response to it before it sends any more.
You then take that buffer and copy it again into a MemoryStream. Rather than doing this, just take the overload of the MemoryStream constructor that takes a buffer. You won't be copying the data that way.
You call EndReceive after you've processed the incoming data; while this may not actually cause an error, it's not in the spirit of the Begin/End old-style async pattern. You should call EndXXX at the beginning of your callback to get the result.

I realize that this is not a direct answer to your question, but you really need to reconsider your decision not to use WCF.

I was in the same boat as you a couple of months ago; I had not used WCF before, and I hadn't bothered to look at how things work in it. It was a very large black box to me, and I had done socket-based communication on other platforms, so it was a known quantity. Looking back, my choice to take the plunge into WCF was the best decision I could have made. Once you've wrapped your head around some of the concepts (bindings, contracts, and how to use the various attributes), development of the service is simple and you don't have to spend your time writing plumbing.

NetTcpBinding provides a TCP-based binding that can support long-lived connections and sessions (which is how I'm using it), and even takes care of keep-alive messages to keep the connection open via Reliable Sessions. It's as simple as turning on a flag. If you need something more interopable (meaning cross-platform), you can write your own binding that does this and keep your code as-is.

Look at some of the TCP WCF examples; it won't take you terribly long to get something up and running, and once you've reached that point, modification is as simple as adding a function to your interface, then a corresponding function on your service class.

Well, i have never programmed sockets, rarely used streams so that whole thing was confusion to me :\ — guest86, Jul 02 '11 at 19:16
@guest86: If you haven't programmed sockets, then that is even more of a signal to use WCF. — Adam Robinson, Jul 02 '11 at 19:33

Problem with sockets and OutOfMemory error

2 Answers2

Linked