1

I am working on a .NET application where the server sends JPG compressed images from a webcam to the client via TCP/IP. For serialization/deserialization I am using the BinaryFormatter class. When testing between my Desktop computer (client/Windows 10) and my Laptop (server/Windows 10), everything works fine in the long run. When using a LattePanda (server/Windows 7), I get the following error after approximately 3-5 minutes runtime (I send/receive 30 frames per second):

An unhandled exception of type 'System.Runtime.Serialization.SerializationException' occurred in mscorlib.dll

Additional information: The input stream is not a valid binary format. The starting contents (in bytes) are: 00-00-00-01-00-00-00-FF-FF-FF-FF-01-00-00-00-00-00 ...

Here's a snippet from the server code:

    private void distribute(Camera camera, Mat frame) {
        if(clientSockets!=null) {
            if(clientSockets.Count > 0) {
                if(camera.Streaming) {
                    // compress and encapsulate raw image with jpg algorithm
                    CameraImage packet = new CameraImage(camera.Id, frame, camera.CodecInfo, camera.EncoderParams);
                    packet.SystemId = Program.Controller.Identity.Id;
                    packet.SequenceNumber = curSeqNum;
                    byte[] content;
                    using(MemoryStream ms = new MemoryStream()) {
                        BinaryFormatter bf = new BinaryFormatter();
                        bf.Serialize(ms, packet);
                        content = ms.ToArray();
                    }
                    byte[] payload = new byte[content.Length+4];
                    // prefix with packet length
                    Array.Copy(BitConverter.GetBytes(content.Length), 0, payload, 0, 4);
                    // append payload after length header
                    Array.Copy(content, 0, payload, 4, content.Length);
                    // distribute to connected clients
                    this.distribute(payload);
                }
            }
        }
    }

    private void distribute(byte[] bytes) {
        if(Program.Launched) {
            lock(syncobj) {
                // distribute to connected clients
                for(int i=clientSockets.Count-1; i>=0; i--) {
                    try {
                        clientSockets[i].Send(bytes, bytes.Length, SocketFlags.None);
                    } catch(SocketException) {
                        clientSockets.RemoveAt(i);
                    }
                }
            }
        }
    }

Here's a snippet from the client code:

    private void receive() {
        try {
            while(running) {
                if((available = clientSocket.Receive(buffer, 4, SocketFlags.None)) > 0) {
                    // receive bytes from tcp stream
                    int offset = 0;
                    int bytesToRead = BitConverter.ToInt32(buffer, 0);
                    byte[] data = new byte[bytesToRead];
                    while(bytesToRead > 0) {
                        int bytesReceived = clientSocket.Receive(data, offset, bytesToRead, SocketFlags.None);
                        offset += bytesReceived;
                        bytesToRead -= bytesReceived;
                    }
                    // deserialize byte array to network packet
                    NetworkPacket packet = null;
                    using(MemoryStream ms = new MemoryStream(data)) {
                        BinaryFormatter bf = new BinaryFormatter();
                        packet = (NetworkPacket)bf.Deserialize(ms);
                    }
                    // deliver network packet to listeners
                    if(packet!=null) {
                        this.onNetworkPacketReceived?.Invoke(packet);
                    }
                    // update network statistics
                    NetworkStatistics.getInstance().TotalBytesRtpIn += data.Length;
                }
            }
        } catch(SocketException ex) {
            onNetworkClientDisconnected?.Invoke(agent.SystemId);
        } catch(ObjectDisposedException ex) {
            onNetworkClientDisconnected?.Invoke(agent.SystemId);
        } catch(ThreadAbortException ex) {
            // allows your thread to terminate gracefully
            if(ex!=null) Thread.ResetAbort();
        }
    }

Any ideas why I get the SerializationException only on one machine and not on the other? Different mscorlib.dll libraries installed? How to check the version of the relevant (?) libraries?

salocinx
  • 3,715
  • 8
  • 61
  • 110
  • 4
    Any chance `clientSocket.Receive(buffer, 4, SocketFlags.None))` returns a value `< 4` but `> 0`? Also `BinaryFormatter` is intended to be used only for IPC between processes on the same machine, not for persisted storage nor network communication. It is a very brittle protocol. I highly recommend you switch to a different one like `ProtoBuf-net` which is designed to handle differences between machines. – Scott Chamberlain Jun 13 '17 at 20:50
  • It's better to not even search for a source of this error but just switch out of BinaryFormatter. It is not good for this task. – Evk Jun 13 '17 at 20:55
  • 3
    My money would be on exactly what @Scott says - that first read is **not** safe. I'm also a strong fan of the idea of removing BinaryFormatter, and an admittedly biased advocate of the suggested library.... cough – Marc Gravell Jun 13 '17 at 21:00
  • You can convert to 64string : Convert.ToBase64String(byte[]) Convert.FromBase64String(string). So use ReadAllbytes from memory stream. – jdweng Jun 13 '17 at 21:01
  • The badness of `BinaryFormatter` aside, are you properly implementing [message framing](https://stackoverflow.com/q/7257139)? See https://blog.stephencleary.com/2009/04/message-framing.html for details. – dbc Jun 13 '17 at 21:01
  • @jdweng that would ... not help; that's the same data, but now it takes more space. If you're dealing with raw sockets, raw bytes are just fine – Marc Gravell Jun 13 '17 at 21:02
  • (For those not in the know, Marc Gravell is the author of [ProtoBuf-net](https://github.com/mgravell/protobuf-net)) – Scott Chamberlain Jun 13 '17 at 21:02
  • 1
    @dbc with the exception of the dubious first read: yes, they are - they have a length prefix for the payload. It isn't implemented especially efficiently, but: fix that first read and it should work fine in theory. – Marc Gravell Jun 13 '17 at 21:03
  • Thanks for the numerous feedback so far. Okay I will try to get rid of this first read and test again. I already had a look into ProtoBuf-net. But since my project is quite large and lots of other objects are interchanged between C/S that are also used to store as settings in an XML file (using [NonSerialized] and [XmlIgnore] to decide what is shared on the network and written to the XML file), I am bit scared to replace the BinaryFormatter at this stage. I will report back as soon as I fixed the first read. – salocinx Jun 13 '17 at 21:34
  • 1
    @salocinx well, if you ever change your mind on that... I'm always willing to help folks out. – Marc Gravell Jun 13 '17 at 21:45
  • MarcGravell: The problem seems to be solved, I will post an answer with my new approach. Thanks a lot for your offering, I will use ProtoBuf-net in my next project for sure :) ScottChamberlain: Indeed, the problem was the first unsafe read. It only happened on the LattePanda, probably because it's a lot slower than the Laptop I used as reference. Perhaps the LattePanda was not able to deliver the data fast enough, resulting in some smaller fragments/chunks on the receiving side. Somewhen ending in a read that was smaller than the size header of 4 bytes. – salocinx Jun 13 '17 at 22:06

2 Answers2

1

Here is a tweaked version of your answer. Right now if clientSocket.Available < 4 and running == true you have a empty while(true) { } loop. This is going to take up 100% of one cpu core doing no useful work.

Instead of looping doing nothing till you have 4 bytes in the system I/O buffer use the same kind of loop you did for the payload for your message and load it to your byte array for the header. (I have also simplified the loop of reading the payload data to the loop I unusually use.)

private void receive() {
    try {
        while(running) {
            int offset = 0;
            byte[] header = new byte[4];

            // receive header bytes from tcp stream
            while (offset < header.Length) {
                offset += clientSocket.Receive(header, offset, header.Length - offset, SocketFlags.None);
            }
                int bytesToRead = BitConverter.ToInt32(header, 0);
                // receive body bytes from tcp stream
                offset = 0;
                byte[] data = new byte[bytesToRead];
                while(offset < data.Length) {
                    offset += clientSocket.Receive(data, offset, data.Length - offset, SocketFlags.None);
                }
                // deserialize byte array to network packet
                NetworkPacket packet = null;
                using(MemoryStream ms = new MemoryStream(data)) {
                    BinaryFormatter bf = new BinaryFormatter();
                    packet = (NetworkPacket)bf.Deserialize(ms);
                }
                // deliver network packet to listeners
                if(packet!=null) {
                    this.onNetworkPacketReceived?.Invoke(packet);
                }
                // update network statistics 
                NetworkStatistics.getInstance().TotalBytesRtpIn += data.Length;
            }
        }
    } catch(SocketException ex) {
        onNetworkClientDisconnected?.Invoke(agent.SystemId);
    } catch(ObjectDisposedException ex) {
        onNetworkClientDisconnected?.Invoke(agent.SystemId);
    } catch(ThreadAbortException ex) {
        // allows your thread to terminate gracefully
        if(ex!=null) Thread.ResetAbort();
    }
}

However, if you switched from the System.Net.Socket class to the System.Net.TcpClient class you could simplify your code a lot. First, if you don't need TotalBytesRtpIn to be updating you can stop sending the header, it is not needed for the deserialization as BinaryFormatter already has it's length stored as a internal field of the payload. Then all you need to do is get the NetworkStream from the TcpClient and process the packets as they come in.

private TcpClient _client; // Set this wherever you had your original Socket set up.

private void receive() {
    try {
        using(var stream = _client.GetStream()) {
            BinaryFormatter bf = new BinaryFormatter();
            while(running) {


#region This part is not needed if you are only going to deserialize the stream and not update TotalBytesRtpIn, make sure the server stops sending the header too!
                    int offset = 0;
                    byte[] header = new byte[4];

                    // receive header bytes from tcp stream
                    while (offset < header.Length) {
                        offset += stream.Read(header, offset, header.Length - offset);
                    }
                    int bytesToRead = BitConverter.ToInt32(header, 0);
#endregion
                    packet = (NetworkPacket)bf.Deserialize(stream);

                    // deliver network packet to listeners
                    if(packet!=null) {
                        this.onNetworkPacketReceived?.Invoke(packet);
                    }
                    // update network statistics
                    NetworkStatistics.getInstance().TotalBytesRtpIn += bytesToRead;
                }
            }
        }
    //These may need to get changed.
    } catch(SocketException ex) {
        onNetworkClientDisconnected?.Invoke(agent.SystemId);
    } catch(ObjectDisposedException ex) {
        onNetworkClientDisconnected?.Invoke(agent.SystemId);
    } catch(ThreadAbortException ex) {
        // allows your thread to terminate gracefully
        if(ex!=null) Thread.ResetAbort();
    }
}
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • Thank you so much! Your approach is much better and the overall CPU load dropped from 59% to 30% on my i7 870 :-) ! I will try the TCPClient alternative as soon as I get some spare time. – salocinx Jun 15 '17 at 12:56
0

As Scott suggested, I replaced the unsafe line that reads the header of the messages with a safer approach:

    private void receive() {
        try {
            while(running) {
                if(clientSocket.Available>=4) {
                    // receive header bytes from tcp stream
                    byte[] header = new byte[4];
                    clientSocket.Receive(header, 4, SocketFlags.None);
                    int bytesToRead = BitConverter.ToInt32(header, 0);
                    // receive body bytes from tcp stream
                    int offset = 0;
                    byte[] data = new byte[bytesToRead];
                    while(bytesToRead > 0) {
                        int bytesReceived = clientSocket.Receive(data, offset, bytesToRead, SocketFlags.None);
                        offset += bytesReceived;
                        bytesToRead -= bytesReceived;
                    }
                    // deserialize byte array to network packet
                    NetworkPacket packet = null;
                    using(MemoryStream ms = new MemoryStream(data)) {
                        BinaryFormatter bf = new BinaryFormatter();
                        packet = (NetworkPacket)bf.Deserialize(ms);
                    }
                    // deliver network packet to listeners
                    if(packet!=null) {
                        this.onNetworkPacketReceived?.Invoke(packet);
                    }
                    // update network statistics
                    NetworkStatistics.getInstance().TotalBytesRtpIn += data.Length;
                }
            }
        } catch(SocketException ex) {
            onNetworkClientDisconnected?.Invoke(agent.SystemId);
        } catch(ObjectDisposedException ex) {
            onNetworkClientDisconnected?.Invoke(agent.SystemId);
        } catch(ThreadAbortException ex) {
            // allows your thread to terminate gracefully
            if(ex!=null) Thread.ResetAbort();
        }
    }

Now it's running flawlessly :) Thanks a lot for your help!

salocinx
  • 3,715
  • 8
  • 61
  • 110
  • 1
    Instead of `if(clientSocket.Available>=4)` just do the same loop you did with `while(bytesToRead > 0) {` lower in the code and just make `bytesToRead` be 4. Your current method will have a tight loop that uses up 100% CPU while you are waiting for packets to arrive. – Scott Chamberlain Jun 14 '17 at 14:53
  • Thanks for your comment. The receive() function is running in a thread and works fine. I am not sure what do you exactly mean. Could you post an answer with your suggested modification? I will then test it and delete my own answer in order to accept yours. Thank you! – salocinx Jun 14 '17 at 22:11
  • 1
    Posted, I also included a alternate simpler implementation if you are willing to use a `TcpClient` instead of a `Socket` – Scott Chamberlain Jun 14 '17 at 22:39