Thread Safety??? [modified]
-
Thanks for reply, Yes dataStream is an object of NetworkStream. I can understand the mechanism of Queing the messages but creating 100 threads for 50 Clients(50 on client end and 50 on server end) will seriously do a rape with CPU;P. One easiest way to solve this problem is to put Thread.Sleep(50); above each call of this method. Which will also not consume CPU but its cheap and will hit performance of application. Best Regards,
And queue objects with message and connection details? A man said to the universe: "Sir I exist!" "However," replied the Universe, "The fact has not created in me A sense of obligation." -- Stephen Crane
-
The situation you have matches what you would use a ReaderWriterLock[^] for. A reader/writer lock allows only one writer at a time, but several readers at a time. The only concern I would have after that is if you can only allow one reader at a time. In that case, I would stick with the lock you have, but apply the lock to readers as well. In the code you have above, it looks like you obtain a lock to write to the stream, but not to read. If a call to the read method happens in the middle of your write method, there's nothing in that read method checking if there's a lock or not. So it's entirely possible reads are still happening during your writes. You have to use the lock on both sides.
First of all yes! I'm maintainging locks on both read and write that is not problem with my case. The problem in my case is when A piece of code calls SendMessage(TextMessage textMessage) { // Doing work here } then an other piece of code calls this Method with a new object of TextMessage Send(TextMessage textMessage) { // Doing work here } the second class overwrite the previous object of TextMessage Class and then the some part of data written to the stream is from the last object and one from the new object. Its different from the scenerio you are pointing that stream is misbehaved. Stream is already locked and that is safe the only thing is grabbled is TextMessage Objec that is passed to the method. Do you think I'm right? If so then please suggest something. I'm really stucked at this problem. Best Regards, Rizwan
-
And queue objects with message and connection details? A man said to the universe: "Sir I exist!" "However," replied the Universe, "The fact has not created in me A sense of obligation." -- Stephen Crane
Did not get it :~
-
First of all yes! I'm maintainging locks on both read and write that is not problem with my case. The problem in my case is when A piece of code calls SendMessage(TextMessage textMessage) { // Doing work here } then an other piece of code calls this Method with a new object of TextMessage Send(TextMessage textMessage) { // Doing work here } the second class overwrite the previous object of TextMessage Class and then the some part of data written to the stream is from the last object and one from the new object. Its different from the scenerio you are pointing that stream is misbehaved. Stream is already locked and that is safe the only thing is grabbled is TextMessage Objec that is passed to the method. Do you think I'm right? If so then please suggest something. I'm really stucked at this problem. Best Regards, Rizwan
There's not much information for me to go off of here. It looks like you have two different methods that write a TextMessage out to a stream. Are these methods in the same class or in different classes? Do they work with the same stream and obtain the same lock? Do you have two instances of the class that are talking to the same underlying network stream? I guess what I'm looking for here is if you have a singleton that has a monopoly over this network stream. Everything that goes to that stream therefore has to go through the same object (or through static methods of the same class). Then it's down to a syncronization issue on the methods reading and writing to that stream.
-
There's not much information for me to go off of here. It looks like you have two different methods that write a TextMessage out to a stream. Are these methods in the same class or in different classes? Do they work with the same stream and obtain the same lock? Do you have two instances of the class that are talking to the same underlying network stream? I guess what I'm looking for here is if you have a singleton that has a monopoly over this network stream. Everything that goes to that stream therefore has to go through the same object (or through static methods of the same class). Then it's down to a syncronization issue on the methods reading and writing to that stream.
Forget about all the things and focus only on this code:
public void SendMessage(TextMessage messageToSend) { try { lock (this.dataStream) { byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes serializedBytes = null; int messageSize = encMessageBytes.Length; byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; messageSizeBuffer = BitConverter.GetBytes(messageSize); this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message this.dataStream.Flush(); messageSizeBuffer = null; encMessageBytes = null; } } catch { }Now try to follow me: Code bock 'A' Makes a call SendMessage(textMessage1); and SendMessage is in the middle of its work and a Code Block 'B' also call this Method SendMessage(textMessage2); the second call overwrite the textMessage1 with textMessage2:doh: I think this is the problem May be I'm wrong:(( I dont know what to do:sigh: Thanks for your help. If you have something more in mind please help me. I really need this so urgent. Best Regards, Rizwan -
Forget about all the things and focus only on this code:
public void SendMessage(TextMessage messageToSend) { try { lock (this.dataStream) { byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes serializedBytes = null; int messageSize = encMessageBytes.Length; byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; messageSizeBuffer = BitConverter.GetBytes(messageSize); this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message this.dataStream.Flush(); messageSizeBuffer = null; encMessageBytes = null; } } catch { }Now try to follow me: Code bock 'A' Makes a call SendMessage(textMessage1); and SendMessage is in the middle of its work and a Code Block 'B' also call this Method SendMessage(textMessage2); the second call overwrite the textMessage1 with textMessage2:doh: I think this is the problem May be I'm wrong:(( I dont know what to do:sigh: Thanks for your help. If you have something more in mind please help me. I really need this so urgent. Best Regards, RizwanWell, you have a lock, which is fine. The lock will only let one thread through this code at a time. This is guaranteed. What's not guaranteed is what I'm asking about. This method is not static, so it must be an instance method. I assume that dataStream is also not static. My question is about where dataStream goes. Does it wrap around an existing stream that other objects can also wrap around? If you have a stream that is shared between two or more objects with this SendMessage method, then the lock is useless. That's why I'm asking if this is a singleton or not.
-
Thanks for reply, Yes dataStream is an object of NetworkStream. I can understand the mechanism of Queing the messages but creating 100 threads for 50 Clients(50 on client end and 50 on server end) will seriously do a rape with CPU;P. One easiest way to solve this problem is to put Thread.Sleep(50); above each call of this method. Which will also not consume CPU but its cheap and will hit performance of application. Best Regards,
Thread.Sleep(50) is VERY WRONG synchronization. In fact it's no syncrhonization at all, it just decrease bug frequency, making them even more difficult to debug. Beside you only need 2 thread and a queue. 1 reader thread (or num CPU/2) 1 writer thread (or num CPU/2) and then a queue of { Stream dest; byte[] data }
-
Hello Devs, I'm developing a Chat application and I'm facing a problem. I have the following Method which is called from multiple threads. And when I start sending too much messages in speed (which invokes this method so fast) then client is disconnected. Let me show you the method and then continue my discussion:
public void SendMessage(TextMessage messageToSend) { try { lock (this.dataStream) { byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes serializedBytes = null; int messageSize = encMessageBytes.Length; byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; messageSizeBuffer = BitConverter.GetBytes(messageSize); this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message this.dataStream.Flush(); messageSizeBuffer = null; encMessageBytes = null; } } catch (Exception ex) { string str = ex.Message + "\n" + ex.StackTrace; Disconnect(); if (Disconnected != null) { Disconnected(this, EventArgs.Empty); } } }In my opnion, the cause of this problem is multi threaded environment or it overflows data when i start sending to fast. for example if it is already working to send data on NetworkStream then a new call to this method is made which passes a new object of TextMessage to it which over writes the existing one so it is all messed up. Note: Actual exception I get on the receiving end when length of data is not one which is supposed to be. See the code for Receiving:int messageSize = BitConverter.ToInt32(messageDataLengthBuffer, 0); MemoryStream memstrm = new MemoryStream(); readByteCount = 0; while (messageSize > 0) { readByteCount = dataStream.Read(this.mSorry guy to put you in problem. And thanks a lot for your help(Really appreciate it:) I got the problem on receiving end. Please try to follow me: lock (this.dataStream) // Lock the NetworkStream { byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes serializedBytes = null; int messageSize = encMessageBytes.Length; // Know the Length of encrypted bytes byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; // Make a buffer of 4 bytes to store an integer value messageSizeBuffer = BitConverter.GetBytes(messageSize); // Convert the messageSize into bytes this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message this.dataStream.Flush(); messageSizeBuffer = null; encMessageBytes = null; what I'm doing in the above code is 1) I lock the NetworkStream 2) Convert the TextMessage's object into bytes (So that I can write them on NetworkStream) 3) Encrypt the Bytes 4) Take the Length of the encrypted bytes and store it into a 4 byte's array 5) Before I send the original message, I write the size of message as a 4 byte array.So that the receiver may know that incoming message includes XXX bytes and i have to read only this much data (not more than that). :mad: But when I send messages too fast then the reading end reads data of more than one message (let me show you how) Receiving End: int messageSize = BitConverter.ToInt32(messageDataLengthBuffer, 0); MemoryStream memstrm = new MemoryStream(); readByteCount = 0; while (messageSize > 0) { readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length); if (readByteCount < 1) { Disconnect(); return; } memstrm.Write(this.messageDataBuffer, 0, readByteCount); messageSize -= readByteCount; } memstrm.Position = 0; byte[] decryptedData = encryptorDecryptor.Decrypt(memstrm.ToArray()); receivedMessage = new TextMessage(decryptedData); memstrm.Close(); memstrm = null; 1) I read 4 bytes from the Stream and Convert these 4 bytes to integer so I may know that incoming message has XXX number for bytes and I have to reaod only XXX bytes. 2) The Buffer I'm using is of 1024 which is filled up with this statement readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length); See problem exist at this statement i.e
-
Sorry guy to put you in problem. And thanks a lot for your help(Really appreciate it:) I got the problem on receiving end. Please try to follow me: lock (this.dataStream) // Lock the NetworkStream { byte[] serializedBytes = messageToSend.GetBytes(); // Get bytes from the Message object byte[] encMessageBytes = encryptorDecryptor.Encrypt(serializedBytes); // Encrypt bytes serializedBytes = null; int messageSize = encMessageBytes.Length; // Know the Length of encrypted bytes byte[] messageSizeBuffer = new byte[this.messageDataLengthBufferSize]; // Make a buffer of 4 bytes to store an integer value messageSizeBuffer = BitConverter.GetBytes(messageSize); // Convert the messageSize into bytes this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual message this.dataStream.Flush(); messageSizeBuffer = null; encMessageBytes = null; what I'm doing in the above code is 1) I lock the NetworkStream 2) Convert the TextMessage's object into bytes (So that I can write them on NetworkStream) 3) Encrypt the Bytes 4) Take the Length of the encrypted bytes and store it into a 4 byte's array 5) Before I send the original message, I write the size of message as a 4 byte array.So that the receiver may know that incoming message includes XXX bytes and i have to read only this much data (not more than that). :mad: But when I send messages too fast then the reading end reads data of more than one message (let me show you how) Receiving End: int messageSize = BitConverter.ToInt32(messageDataLengthBuffer, 0); MemoryStream memstrm = new MemoryStream(); readByteCount = 0; while (messageSize > 0) { readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length); if (readByteCount < 1) { Disconnect(); return; } memstrm.Write(this.messageDataBuffer, 0, readByteCount); messageSize -= readByteCount; } memstrm.Position = 0; byte[] decryptedData = encryptorDecryptor.Decrypt(memstrm.ToArray()); receivedMessage = new TextMessage(decryptedData); memstrm.Close(); memstrm = null; 1) I read 4 bytes from the Stream and Convert these 4 bytes to integer so I may know that incoming message has XXX number for bytes and I have to reaod only XXX bytes. 2) The Buffer I'm using is of 1024 which is filled up with this statement readByteCount = dataStream.Read(this.messageDataBuffer, 0, messageDataBuffer.Length); See problem exist at this statement i.e
Hmm, I see. I was assuming that since you wrote the size of the message into the stream:
this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message
this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual messageThat you were also reading the size of the message first and then reading the message. But you're saying you just blindly take in 1024 bytes at a time assuming that the message will be a multiple of that length. In your loop, you should take into account that your message will not be a multiple of 1024. Either look at your
messageSizeto figure out how much you can read (1024 bytes or less), or pad the messages you send to multiples of 1024 bytes.
-
Hmm, I see. I was assuming that since you wrote the size of the message into the stream:
this.dataStream.Write(messageSizeBuffer, 0, this.messageDataLengthBufferSize); // Send the size of the message
this.dataStream.Write(encMessageBytes, 0, messageSize); // Send the actual messageThat you were also reading the size of the message first and then reading the message. But you're saying you just blindly take in 1024 bytes at a time assuming that the message will be a multiple of that length. In your loop, you should take into account that your message will not be a multiple of 1024. Either look at your
messageSizeto figure out how much you can read (1024 bytes or less), or pad the messages you send to multiples of 1024 bytes.
Yes Exactly, I already solved this problem a few hours back but thanks alot for being so concerned from heart to help me. What I did is:
if (messageSize > messageDataBuffer.Length) readByteCount = dataStream.Read(messageDataBuffer, 0, messageDataBuffer.Length); else readByteCount = dataStream.Read(messageDataBuffer, 0, messageSize);:-D All is working perfect now :laugh: Thanks alot again!!! Please add me in you Msn buddies if you feel comfortable :) rizwansharp@hotmail.com Best Regards, Rizwan Ahmed