TReKiE / msnp-sharp

Automatically exported from code.google.com/p/msnp-sharp
0 stars 0 forks source link

Thread unsafe #9

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Intensive use, sending and receiving messages

What is the expected output? What do you see instead?
Expected: Do not throw any exception
I get an exception when the library iterates over a Synchronized ArrayList.

What version of the product are you using? On what operating system?
Revision 334 from subversion

Please provide any additional information below.

You should wrap iterations over Synchronized ArrayList with locks like
this, according to documentation:
http://msdn.microsoft.com/en-us/library/3azh197k.aspx

lock (tsConversations.SyncRoot)
{
  foreach (Conversation conversation in tsConversations)
  {
    if (conversation.Switchboard == sender)
    {
      tsConversations.Remove(conversation);
      return;
    }
  }
}

This problem happens in the constructor and methods
Switchboard_SessionClosed and GetMSNSLPHandler of class Messenger

Original issue reported on code.google.com by ignacioa...@gmail.com on 17 Jul 2008 at 8:38

Attachments:

GoogleCodeExporter commented 9 years ago
woooo, what a professional report! Thanks.

Original comment by freezing...@gmail.com on 18 Jul 2008 at 5:30

GoogleCodeExporter commented 9 years ago
Already fixed, just check out r371 or the latest version.

Original comment by freezing...@gmail.com on 19 Jul 2008 at 3:57

GoogleCodeExporter commented 9 years ago
Great! Thanks for your quick response.

Original comment by ignacioa...@gmail.com on 19 Jul 2008 at 2:44

GoogleCodeExporter commented 9 years ago
Hi! I realized that you reverted the modification you've made. Microsoft's
documentation states that:

"Enumerating through a collection is intrinsically not a thread-safe procedure. 
Even
when a collection is synchronized, other threads can still modify the 
collection,
which causes the enumerator to throw an exception. To guarantee thread safety 
during
enumeration, you can either lock the collection during the entire enumeration or
catch the exceptions resulting from changes made by other threads."

So, even in the case you use ArrayList.Synchronized, you should add locks when
iterating over a collection to avoid getting an exception that says that the
collection had been modified during the iteration.

Original comment by ignacioa...@gmail.com on 21 Jul 2008 at 7:44

GoogleCodeExporter commented 9 years ago
Ok, fixed.

Original comment by hepha...@gmail.com on 22 Jul 2008 at 12:30

GoogleCodeExporter commented 9 years ago

Original comment by hepha...@gmail.com on 23 Jan 2009 at 5:40