Mutltithread: writing to public lists in many instances, need of lock?

Question

Hi, At first I assume I do need writerlock here but Im not sure (not much experience with that) what if I dont use it.

On the server side, there are client classes for each connected client. Each class contains public list which every other class can write to. Client requests are processed via threadpool workitems.

class client 
{
public List <string> A;

someEventRaisedMethod(param)
{
 client OtherClient=GetClientByID(param) //selects client class by ID sent by msg sender
 OtherCLient.A.Add("blah"); 
}

}

What if two instances reference the same client and both try OtherCLient.A.Add("blah")? Isnt be here some writer lock? It works for me but I encounter some strange issues that I think are due to this.

Thank you!

Answer 1

(update: as always, Eric Lippert has a timely blog entry)

If you don't use a lock, you risk either missing data, state corruption, and probably the odd Exception - but only very occasionally, so very hard to debug.

Absolutely you need to synchronize here. I would expose a lock on the client (so we can span multiple operations):

lock(otherClient.LockObject) {
    otherClient.A.Add("blah");
}

You could make a synchronous Add method on otherClient, but it is often useful to span multiple - perhaps to check Contains and then Add only if missing, etc.

---

Just to clarify 2 points:

• all access to the list (even reads) must also take the lock; otherwise it doesn't work
• the LockObject should be a readonly reference-type

for the second, perhaps:

private readonly object lockObject = new object();
public object LockObject {get {return lockObject;}}

Answer 2

From my point of view you should do the following:

• Isolate the list into a separate class which implements either the IList Interface or only the subset which you require
• Either add locking on a private object in the methods of your list class or use the ReaderWriterSlim implementation - as it is isolated there is only one place needed for changing in one single class

Answer 3

I don't know the C# internals, but I do remember reading awhile back about java example that could cause a thread to endlessly loop if it was reading a collection whilst an insert was being done on the collection (I think it was a hashtable), so make sure if you are using multiple threads that you lock on both read and write. Marc Gravell is correct that you should just create a global lock to handle this since it sounds like you have fairly low volume.

ReaderWriterLockSlim is also a good option if you do alot of reading and only a few write / update actions.