0

I am having an issue where an object with a byte[20] is being passed into a BlockingCollection on one thread and another thread returning the object with a byte[0] using BlockingCollection.Take(). I think this is a threading issue but I do not know where or why this is happening considering that BlockingCollection is a concurrent collection.

Sometimes on thread2, myclass2.mybytes equals byte[0]. Any information on how to fix this is greatly appreciated.

[EDIT] The original code. I removed the above code that seemed to run just fine so I took the time to go through my original code and post it.

MessageBuffer.cs

public class MessageBuffer : BlockingCollection<Message>
{
}

In the class that has Listener() and ReceivedMessageHandler(object messageProcessor)

private MessageBuffer RecievedMessageBuffer;

On Thread1

    private void Listener()
    {
        while (this.IsListening)
        {
            try
            {
                Message message = Message.ReadMessage(this.Stream, this);
                if (message != null)
                {
                    this.RecievedMessageBuffer.Add(message);
                }
            }
            catch (IOException ex)
            {
                if (!this.Client.Connected)
                {
                    this.OnDisconnected();
                }
                else
                {
                    Logger.LogException(ex.ToString());
                    this.OnDisconnected();
                }
            }
            catch (Exception ex)
            {
                Logger.LogException(ex.ToString());
                this.OnDisconnected();
            }
        }
    }

Message.ReadMessage(NetworkStream stream, iTcpConnectClient client)

    public static Message ReadMessage(NetworkStream stream, iTcpConnectClient client)
    {
        int ClassType = -1;
        Message message = null;

        try
        {
            ClassType = stream.ReadByte();
            if (ClassType == -1)
            {
                return null;
            }

            if (!Message.IDTOCLASS.ContainsKey((byte)ClassType))
            {
                throw new IOException("Class type not found");
            }

            message = Message.GetNewMessage((byte)ClassType);
            message.Client = client;
            message.ReadData(stream);

            if (message.Buffer.Length < message.MessageSize + Message.HeaderSize)
            {
                return null;
            }

        }
        catch (IOException ex)
        {
            Logger.LogException(ex.ToString());
            throw ex;
        }
        catch (Exception ex)
        {
            Logger.LogException(ex.ToString());
            //throw ex;
        }

        return message;
    }

On Thread2

    private void ReceivedMessageHandler(object messageProcessor)
    {
        if (messageProcessor != null)
        {
            while (this.IsListening)
            {
                Message message = this.RecievedMessageBuffer.Take();
                message.Reconstruct();
                message.HandleMessage(messageProcessor);
            }
        }
        else
        {
            while (this.IsListening)
            {
                Message message = this.RecievedMessageBuffer.Take();
                message.Reconstruct();
                message.HandleMessage();
            }
        }
    }

PlayerStateMessage.cs

public class PlayerStateMessage : Message
{
    public GameObject PlayerState;

    public override int MessageSize
    {
        get { return 12; }
    }

    public PlayerStateMessage()
        : base()
    {
        this.PlayerState = new GameObject();
    }

    public PlayerStateMessage(GameObject playerState)
    {
        this.PlayerState = playerState;
    }

    public override void Reconstruct()
    {
        this.PlayerState.Poisiton = this.GetVector2FromBuffer(0);
        this.PlayerState.Rotation = this.GetFloatFromBuffer(8);
        base.Reconstruct();
    }

    public override void Deconstruct()
    {
        this.CreateBuffer();
        this.AddToBuffer(this.PlayerState.Poisiton, 0);
        this.AddToBuffer(this.PlayerState.Rotation, 8);
        base.Deconstruct();
    }

    public override void HandleMessage(object messageProcessor)
    {
        ((MessageProcessor)messageProcessor).ProcessPlayerStateMessage(this);
    }
}

Message.GetVector2FromBuffer(int bufferlocation) This is where the exception is thrown because this.Buffer is byte[0] when it should be byte[20].

    public Vector2 GetVector2FromBuffer(int bufferlocation)
    {
        return new Vector2(
            BitConverter.ToSingle(this.Buffer, Message.HeaderSize + bufferlocation),
            BitConverter.ToSingle(this.Buffer, Message.HeaderSize + bufferlocation + 4));
    }
MJLaukala
  • 171
  • 1
  • 3
  • 13
  • 1
    Just a comment on style, it's C# convention to have class names capitalized. – Tudor Jul 10 '12 at 10:01
  • `myclass2.mybytes equals byte[0]` - do you mean that it has 0 length? Can you post the code after the `Take`? – Tudor Jul 10 '12 at 10:02
  • So, does thread1 keep expanding the `classes` collection infinitely? – Jodrell Jul 10 '12 at 10:57
  • @Tudor after the take, I have, int i = BitConverter.ToInt32(Buffer, 0); int i2 = BitConverter.ToInt32(Buffer, 4); int i3 = BitConverter.ToInt32(Buffer, 8); and every now and then I get a IndexOutOfRangeException because the length of the array is 0 when it should be 12. – MJLaukala Jul 10 '12 at 11:02
  • @Tudor this is just example code of what I am doing. My original code is spread over many different functions but this example shows the basis of how my code is running. – MJLaukala Jul 10 '12 at 11:05
  • The threadsafety of the blockingcollection only concerns getting classes in and out threadsafe. Are you sure you are not modifying a class somewhere else that is also inside the blockingcollection. Maybe even because of a bug that you have a reference still to a class that was already put into the collection? – IvoTops Jul 10 '12 at 11:07
  • @Jodrell no, this is just example code. I have a TcpClient that receives a "message". If the message buffer, which is a byte[], has a length of zero, it is not added to the classes collection. – MJLaukala Jul 10 '12 at 11:11
  • To fix the problem requires an example where the problem occurs. Otherwise all we have is conjecture and speculation. – Jodrell Jul 10 '12 at 11:14
  • @IvoTops The class1 object is created based on certain conditions that are met. Even after it is created, it is tested to make sure that the bytes are there. if it passes all the tests, it is added to the classes collection and nothing else is done with it until it is pulled out of the collection by thread2 – MJLaukala Jul 10 '12 at 11:21
  • @Jodrell You are right. I did a test with the example code that I provided and it never threw the exception. My original code is very complex and spread over several functions. I'll have to take the time to break it all down and post it. – MJLaukala Jul 10 '12 at 17:31
  • You'r code doesn't look like it will compile. `Class class1` should be `class class1`. But following PascalCase, it would be `class MyClass`. – Cole Tobin Jul 10 '12 at 19:50
  • I have posted all of the relevant code. I hope it helps you help me solve this very frustrating issue. I'm sure it's something stupid that I have overlooked. – MJLaukala Jul 10 '12 at 19:51
  • @coleJohnson Yeah, that was just example code. If I tried to compile it exactly like that, it would not have compiled. Anyway, I took the time to go through the original code and post the relative stuff. – MJLaukala Jul 10 '12 at 19:55
  • Cross posted on gamedev.SE: http://gamedev.stackexchange.com/questions/32068/blockingcollection-having-issues-with-byte-arrays – CodesInChaos Jul 11 '12 at 11:32

1 Answers1

0

So this was a hard problem to solve. As far as I know, I was just receiving random bytes so I changed up my "Message" quite a bit. there is now a header buffer and a data buffer. The entire message is encapsulated with a beginning marker and an ending marker while the header and data buffers are each encapsulated by different markers. this has allowed me to tell when I have received bad data and can discard the message. If the message does get discarded, on the next message read, instead of just checking if the first 4 bytes received is the opening marker, it will read byte by byte until the last 4 bytes read are equal to the marker.

MJLaukala
  • 171
  • 1
  • 3
  • 13