tags:

views:

273

answers:

2

I've got a class that is instantiated within any number of threads that are spooled up as needed. This means that any number of instantiated versions of this class can be used at any one time, and there's a portion of this class that needs to be locked to prevent concurrent access.

To prevent data issues between the various threads, I needed a way to lock a section of code from the other instantiated versions of the class in other threads. Since there can be multiple instantiated versions of this class running around, I can't just use a private member variable to lock (and I know not to use the Type, or anything publicly accessible); so I used a private static member variable.

Is that a reasonable approach to this problem? Or is there a better solution?

Sample code below:

public class MyClass
  {
    private static object LockingVar = new object();

    public void MyPublicMethod()
    {
      lock (LockingVar)
      {
         // Do some critical code
      }
  }

EDIT

MyPublicMethod is making calls to a local SQLExpress instance, it can perform selects in addition to updates and inserts, so it needs to finish before another thread gets in there and mucks it up.

+5  A: 

Looks fine to me. I'd also mark the LockingVar as readonly.

RichardOD
Ah, good catch. Thanks.
Grandpappy
+1  A: 

Yes, with your sample code, you'll achieve a global critical section on the method for all instances of the class.

If that's what you're looking for (and you have to ask yourself if you really want to have only ever one thread running that method at a time), you can also use the [MethodImpl(MethodImplOptions.Synchronized)] which gets you basically the same feature.

[MethodImpl(MethodImplOptions.Synchronized)]
public static void MyPublicMethod()
{
     // Do some critical code
}

Note: this amounts to write lock(this){} if it's a instance method or lock(typeof(MyClass)) if it's a class (static) method. Both are frown upon, so your lock(obj) pattern is better.

Yann Schwartz
Remove the static modifier if you only want to lock on a single instance and not on all the instances.
Laurent Etiemble
You're right. Corrected the answer.
Yann Schwartz