7

I am very new to C# and I wanted to ask if I have this situation in MULTI THREADS (pseudo code):

public class ClassA
{
     ClassB c = new ClassB();
     public void someMethod()
     {
          c.myVar = 1;
          // Some other stuff
          c.myVar = 0;
     }
}

public class ClassB
{
     internal int myVar;

     public void MethodA()
     {
        if(myVar = 1)
              myVar = 0;
     }
}

If someMethod() and MethodA() can be active in separate threads, then MethodA() could evaluate the if statement as true; but before it sets myVar = 0, someMethod() sets myVar = 0 making it incorrect to have set myVar to 0 in MethodA()!!

Basically, how do I lock myVar:

  • can I lock{} on myVar's property (set, get)
  • do I need to use Interlock (I have no experience yet of Interlock though)?
Luiso
  • 4,173
  • 2
  • 37
  • 60
SimpleOne
  • 1,066
  • 3
  • 12
  • 29
  • 3
    1) Don't call a variable of type ClassB for c, thats confusing. – Albin Sunnanbo Nov 02 '10 at 20:54
  • 1
    did you mean `if (myVar == 1)`? – Vlad Nov 02 '10 at 20:55
  • 2
    2) You can not access c.myVar since it is a private member of ClassB. – Albin Sunnanbo Nov 02 '10 at 20:55
  • 1
    What you're describing in your question are definitions for ClassA and ClassB. You need to describe the way those are used in your threads though. As an example, if you have Thread1 that creates an instance of ClassA and Thread2 that creates an instance of ClassB, then the ClassA.c variable in Thread1 won't be the same object (in memory) as the instance of ClassB in Thread2. So, in this example, you don't need locks because you aren't using the same object instance. – Remus Nov 02 '10 at 21:00
  • 1
    Why is it incorrect to set `myVar` to zero if it has already been set to zero? Since setting a field is an idempotent operation I don't see how it can be "wrong" to do it twice. Can you explain a little more about what problem you are trying to solve? – Mark Byers Nov 02 '10 at 21:00
  • Sorry, I have asked this very badly!! Basically, if I have a situation wherein I have one instance of classB with a variable, myVar, and two methods each in separate threads that can use/change myVar - how do I ensure the work on myVar is thread safe? (If at all possible) – SimpleOne Nov 02 '10 at 21:15

4 Answers4

17

You should create a private object that will allow for locking:

private readonly object _locker = new object();

Then in your property get/set methods, lock around it:

get { lock (_locker) { return this.myVar; } }
set { lock (_locker) { this.myVar = value; } }

Make sure your method uses the lock also:

public void MethodA()
{
    lock(_locker)
    {
        if(myVar == 1)
          myVar = 0;
    }
}
Matt Davis
  • 45,297
  • 16
  • 93
  • 124
aqwert
  • 10,559
  • 2
  • 41
  • 61
4

It looks like you are trying to implement some sort of signalling mechanism. Instead of writing your own you could use one of the classes provided in the .NET library such as ManualResetEvent.

Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
2

This is how I do it.

    static readonly object _myVar_Lock = new object();
    private int _myVar = 0;

    public int myVar
    {
        get { lock (_myVar_Lock) { return this._myVar; } }
        set { lock (_myVar_Lock) { this._myVar = value; } }
    }
Dusty Lau
  • 591
  • 4
  • 18
  • 1
    No, this is completely useless, the reading and writing of this._myVar in this case is atomic according to c# spec. (since it is an int). The locking is needed to handle the case `if (myVar == 1) myVar = 0;` without the possibility of another thread changing myVar to lets say 2 in between. – Albin Sunnanbo Nov 02 '10 at 20:59
  • Albin - please can you explain a little further as to why this won't work? – SimpleOne Nov 02 '10 at 21:21
  • Albin, you're wrong. Why would there be an Exchange() method on the Interlocked class if "myVar = value" is atomic? The answer is that the write is not atomic. http://msdn.microsoft.com/en-us/library/d3fxt78a.aspx – OJ. Nov 02 '10 at 21:54
  • @OJ, a write is atomic, but a combined read and write is not atomic. By the time you've entered the property setter, you've already performed the read, so having the lock in the property setter is useless. The only possible benefit of the lock is that it imposes a memory barrier on the field (similar to marking it volatile), but this does not address the higher-level synchronization concern. – Dan Bryant Nov 02 '10 at 22:33
1

I would certainly rethink your overall approach, but if you are wanting to synchronize access to the members of ClassB from different sections of code then you could steal a not-so-good design pattern from the ICollection interface and expose a SyncRoot property that can be used to acquire the same lock as the original instance.

public class ClassA
{
  private ClassB c = new ClassB();

  public void someMethod()
  {
    lock (c.SyncRoot)
    {
      c.MyVar = 1;
      // Some other stuff
      c.MyVar = 0;
    }
  }
}

public class ClassB
{
  private object m_LockObject = new object();
  private int m_MyVar;

  public object SyncRoot
  {
    get { return m_LockObject; }
  }

  public int MyVar
  {
    get { lock (SyncRoot) return m_MyVar; }
    set { lock (SyncRoot) m_MyVar = value; }
  }

  public void MethodA()
  {
    lock (SyncRoot)
    {
      if (m_MyVar == 1) m_Var = 0;
    }
  }
}
Brian Gideon
  • 47,849
  • 13
  • 107
  • 150