Is it thread-safe to read a property from an instantiated object? [closed]

Question

Closed. This question needs to be more focused. It is not currently accepting answers.

---

Want to improve this question? Update the question so it focuses on one problem only by editing this post.

Closed 9 years ago.

Improve this question

Can I safely read a IsBusy boolean property for an instantiated singleton from multiple threads without causing any thread-safety issues, or am I already in dangerous waters?

Answer 1

Unless you have any memory barriers, you're potentially in dangerous waters, if the value can ever change.

Let's take the singleton part of this out of the equation, as it's not particularly relevant. Consider this simple example:

// Not safe!
public class BusyIndicator
{
    public bool IsBusy { get; set; }
}

Now if you have one thread which is along the lines of:

foo.IsBusy = true;
// do some work
foo.IsBusy = false;

and another thread, only started after IsBusy is set to true, that has:

// Do some work
while (foo.IsBusy) { }

... then it's possible that the second thread will spin forever... there's no memory barrier to ensure that it "sees" the change from the first thread.

You can implement this safely either using locking or using Interlocked, e.g.

// Safe (in terms of memory barriers)
public class BusyIndicator
{
    private int busy = 0;

    public bool IsBusy
    {
        get { return Interlocked.CompareExchange(ref busy, 0, 0) == 1; }
        set { Interlocked.Exchange(ref busy, value ? 1 : 0); }
    }
}

Answer 2

If you have defined the property IsBusy as follows, then you can definitely have thread safety issues:

public bool IsBusy { get; set; }

The big issue here is the value can be checked and then changed before the value is set. That means you have a race condition. ONE way to solve this is:

// this is going to keep trying to lock until it succeeds, after which it 
//     will do whatever work needs done.
while (true) {
  bool lockObtained = false;
  lock (objectInstance) {
    if (!objectInstance.IsBusy) {
      lockObtained = true;
      objectInstance.IsBusy = true;
    }
  }

  if (lockObtained) {
    // good to go here

    // and don't forget to clear the IsBusy
    objectInstance.IsBusy = false;
    break;
  }
}

This uses the built in .NET locking mechanism to lock the object so you can check and set without worrying about process preemption in the middle.