Hello,
I'm working on a Network Application which holds all the connected clients in a Dictionary (Collection) and when Clients connect or disconnect this collection needs to be Modified as per requirement. All works fine with small work load e.g if a single client connect or disconnects or may be 2.
But when a bunch of Clients say 30 disconnect altogether then I get Excetpion Collection Modified, Enumeration Operation may not execute come thing like this. I know this is all due to Thread Safety where multiple disconnection occurs. I have used lock and thought its enough but I'm getting this error.
Here is my Code:
public
void Disconnected(ClientProxy sender, EventArgs e){
try{
lock (clientProxyDictionary){
ClientProxy disconnectedProxy = sender; string disconnectedProxyName = disconnectedProxy.UserName; string disconnectedProxyIP = disconnectedProxy.ClientIPAddress; if (privateChatWindows.ContainsKey(disconnectedProxyName)){
FrmPrivateChat frmPrivateChat;privateChatWindows.TryGetValue(disconnectedProxyName,
out frmPrivateChat); this.Invoke(new EnableDisableControlsDlgt(frmPrivateChat.EnableDisableControls), false);}
clientProxyDictionary.Remove(disconnectedProxy.UserName);
TextMessage textMessage = new TextMessage();textMessage.IsBold =
false;textMessage.IsItalic =
false;textMessage.IsPrivate =
true;textMessage.IsUnderline =
false;textMessage.SenderName =
"";textMessage.TextMessageType =
TextMessageType.UserDisconnected; byte[] initializationVector;textMessage.ChatMessage = encryptorDecryptor.Encrypt(disconnectedProxyName +
"`#`" + disconnectedProxyIP, out initializationVector, secKey);textMessage.ReceiverName =
"";textMessage.InitializationVector = initializationVector;
Dictionary<string, ClientProxy>.ValueCollection clientProxies = clientProxyDictionary.Values; foreach (ClientProxy clientProxy in clientProxies){
clientProxy.SendMessage(textMessage);
}
textMessage =
null; this.Invoke(new RemovePeerDlgt(RemovePeer), disconnectedProxyName); this.Invoke(new AppendNotificationDlgt(AppendNotification), disconnectedProxyName + " " + ProtectChat.Properties.Resources.HasLeft);}
}
catch(Exception ex){
MessageBox.Show(ex.Message + "\n" + ex.Source + "\n" + ex.StackTrace);}
}
I really hope to get a solution or hint for my problem from you people.
Best Regards,
Rizwan Ahmed.
Thread Safety and Collection Problem
Ramanuj
Michael Taylor - 9/14/06
Frank Paul
The problem is being caused when you enter your foreach loop. The builtin collections (and in general all collections) will raise an exception if you begin enumerating a collection and the collection contents change. What normally happens is that when GetEnumerator is called the returned object contains a reference to the collection to enumerate and the current version # of the collection. The version # of a collection is incremented each time the collection changes (add, insert, remove). Each time MoveNext is called the version # stored in the enumerator is compared to the current version # of the collection. If they don't match then an exception occurs. This is a safety feature because bad things can happen if a list changes while it is enumerating.
The solution is relatively straightforward in a MT world. The best option is to copy the collection to be enumerated before you enumerate it. This works well when the list is small. Note that assigning the collection to a variable is not copying it. During the copy operation the list must be locked. For example the collection classes normally have a CopyTo method which copies the collection contents to an array. This is a good method to use in this case.
ClientProxy[] proxies = new ClientProxy[clientProxyDictionary.Values.Count];
clientProxyDictionary.Values.CopyTo(proxies, 0);
foreach(ClientProxy proxy in proxies)
{
};
The problem with the above code is that it is not thread-safe during the copy operation. Between the time the array is created and it is populated the collection could change. Therefore you need to lock the collection first and then release it after.
lock(...)
{
ClientProxy[] proxies = new ClientProxy[clientProxyDictionary.Values.Count];
clientProxyDictionary.Values.CopyTo(proxies, 0);
};
foreach(ClientProxy proxy in proxies)
{
};
Once the collection is copied the copy is thread safe (although the objects contained in it are not). For large lists this might eat up too much memory. In that case the alternative is to lock the collection, enumerate the contents and then unlock the collection. In your case I would recommend against this strategy though because you are making method calls that could potentially take a while. Therefore you would lock your collections longer than you'd need to which will be bad for performance.
Collections contain the SyncRoot property for locking but I honestly never use it and I'm not sure if it is completely accurate because the collection could still change. I would recommend that you either lock the collection instance itself or use a separate variable to lock the collection. Personally I use ReaderWriterLock for these situations even though many people consider them to be poorly implemented. The benefit is that in this case you are simply reading the collection (I hope) so you can take a read lock. Others can also get a read lock to interact with the collection. However if anyone tries to write they'll have to wait. This gives you some better performance (except the cost of the lock itself which is high) provided you mainly read from the collection. Alternatively there have been enhanced RW locks published in MSDN Magazine and online if you are interested in a better alternative.
Michael Taylor - 9/11/06
MyerTheFlyer
"The problem is being caused when you enter your foreach loop. The builtin collections (and in general all collections) will raise an exception if you begin enumerating a collection and the collection contents change."
I'm alredy locking the collection at the entrance of this mothod and releasing it at the end. What I have studied about lock keyword in MSDN is that we use it around some critical code block which must be accessed from one thread at a time only. So If I'm using lock around a all that code then why its being grabbled from the other thread
Any help would be a great favour and appreciated.
Best Regards,
Rizwan Ahmed