views:

126

answers:

4

Hi everyone,

I have a static collections which implements IList interface. This collection is used throughout the application, including adding/removing items.

Due to multithread issue, I wonder what I can do to ensure that the list is modifying one at a time, such as when 1 thread try to add an item, another thread should not do delete item at that time.

I wonder what is difference between lock(this) and lock(privateObject) ? Which one is better in my case?

Thank you.

+4  A: 

The lock(this) will lock on the entire instance while lock(privateObject) will only lock that specific instance variable. The second one is the better choice since locking on the entire instance will prevent other threads from being able to do anything with the object.

From MSDN:

In general, avoid locking on a public type, or instances beyond your code's control. The common constructs lock (this), lock (typeof (MyType)), and lock ("myLock") violate this guideline:

  • lock (this) is a problem if the instance can be accessed publicly.

  • lock (typeof (MyType)) is a problem if MyType is publicly accessible.

  • lock(“myLock”) is a problem since any other code in the process using the same string, will share the same lock.

Best practice is to define a private object to lock on, or a private static object variable to protect data common to all instances.

In this particular case, the collection is static which effectively means there is a single instance but that still doesn't change how the lock(this) and lock(privateObject) would behave.

By using lock(this) even in a static collection you are still locking the entire instance. In this case, as soon as thread A acquires a lock for method Foo() all other threads will have to wait to perform any operation on the collection.

Using lock(privateObject) means that as soon as thread A acquires a lock for method Foo() all other threads can perform any other operation except Foo() without waiting. Only when another thread tries to perform method Foo() will it have to wait until thread A has completed its Foo() operation and released the lock.

Scott Dorman
my collection is static, is it true that they are all just is one instance in my application? so the both lock means the same?
thethanghn
Yes, having a static collection means there is effectively a single instance in the application, but that doesn't change how you lock. I've updated my answer to address this.
Scott Dorman
+1  A: 

Intelligent locking on a collection is not entirely straight forward. Jared Par has a good blog post you should probably read.

ScottS
+1  A: 

The lock keyword is a little confusing. The object expression in the lock statement is really just an identification mechanism for creating critical sections. It is not the subject of the lock nor is it in any way guarenteed to be safe for multithreaded operations just because it is referenced by the statement.

So lock(this) is creating a critical section identified by the class containing the currently executing method whereas lock(privateObject) is identified by an object that is (presumably anyway) private to the class. The former is more risky because a caller of your class could inadvertantly define their own critical sections using a lock statement that uses that class instance as the lock object. That could lead to unintended threading problems including, but not limited to, deadlocks and bottlenecks.

You mentioned that you were concerned with multiple threads modifying the collection at the same time. I should point out that you should be equally concerned with threads reading the collection as well even if they are not modifying it. It is likely that you will need some of the same safe guards in place to protect the collection during reads as you would during writes.

Brian Gideon
A: 

Add a private member to the class that methods lock on.

eg.

public class MyClass : IList
{
    private object syncRoot = new object();

    public void Add(object value)
    {
        lock(this.syncRoot)
        {
           // Add code here
        }
    }

    public void Remove(object value)
    {
        lock(this.syncRoot)
        {
            // Remove code here
        }
    }
}

This will ensure that access to the list is syncronized between threads for the adding and removing cases, while maintaining access to the list. This will still let enumerators access the list while another thread can modify it, but that then opens another issue where an enumerator will throw an exception if the collection is modified during the enumeration.

Pete Davis