Noob
Noob

Reputation: 1049

Boolean Property Getter and Setter Locking

Is there any reason why you would create locks around the getter and setter of a boolean property like this?

  private _lockObject = new object();
  private bool _myFlag;
  public bool MyFlag
  {
    get
    {
      lock (_lockObject)
      {
        return _myFlag;
      }
    }
    set
    {
      lock (_lockObject)
      {
        _myFlag = value;
      }
    }
  }

Upvotes: 18

Views: 9072

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1500795

Well, you don't need locks necessarily - but if you want one thread to definitely read the value that another thread has written, you either need locks or a volatile variable.

I've personally given up trying to understand the precise meaning of volatile. I try to avoid writing my own lock-free code, instead relying on experts who really understand the memory model.

EDIT: As an example of the kind of problem this can cause, consider this code:

using System;
using System.Threading;

public class Test
{
    private static bool stop = false;

    private bool Stop
    {
        get { return stop; }
        set { stop = value; }
    }

    private static void Main()
    {
        Thread t = new Thread(DoWork);
        t.Start();
        Thread.Sleep(1000); // Let it get started
        Console.WriteLine("Setting stop flag");
        Stop = true;
        Console.WriteLine("Set");
        t.Join();
    }

    private static void DoWork()
    {
        Console.WriteLine("Tight looping...");
        while (!Stop)
        {
        }
        Console.WriteLine("Done.");
    }
}

That program may or may not terminate. I've seen both happen. There's no guarantee that the "reading" thread will actually read from main memory - it can put the initial value of stop into a register and just keep using that forever. I've seen that happen, in reality. It doesn't happen on my current machines, but it may do on my next.

Putting locks within the property getter/setter as per the code in the question would make this code correct and its behaviour predictable.

For more on this, see this blog post by Eric Lippert.

Upvotes: 25

JK.
JK.

Reputation: 5136

Reads and writes of bool are atomic.

However the name "flag" indicates that separate threads will be reading/writing until some condition occurred. To avoid unexpected behavior due to optimization you should consider adding the volatile keyword to you bool declaration.

Upvotes: 1

Ben Voigt
Ben Voigt

Reputation: 283684

There's no reason to have a lock right there.

Taking a lock may well be appropriate in your design, but it's very doubtful that this is the right granularity.

You need to make your design thread-safe, not individual properties (or even entire objects).

Upvotes: -2

Related Questions