Is the following object thread safe?

Question

Is the following object thread safe? I'll make one instance and use it using two or more threads, is this a good way to approach this?

public class ASyncBuffer<T>
{
    readonly object _locker = new object();
    private T _value;
    private bool _dirty;

    public T GetValue()
    {
        lock (_locker)
        {
            _dirty = false;
            return _value;
        }
    }

    public void SetValue(T value)
    {
        lock (_locker)
        {
            _dirty = true;
            _value = value;
        }
    }

    public bool Dirty
    {
        get
        {
            lock (_locker)
            {
                return _dirty;
            }
        }
    }
}

Add `private` to your `readonly object _locker`. Without it, another object in the project could see `_locker` and add its own lock. — Hand-E-Food, Jan 17 '12 at 22:20
@Hand-E-Food, no it is `private` by default. Stating it more explicit would be better though. — H H, Jan 17 '12 at 22:33
@HenkHolterman, thanks! I thought it was 'internal' for some reason. — Hand-E-Food, Jan 17 '12 at 22:46
@KarlStrings, because any object that knows about `this` (eg. `ASyncBuffer myBuffer = new ASyncBuffer()`) can potentially do the same unexpectedly, possibly causing a deadlock. By using a private object, the class has full control over how it's locked, preventing deadlocks. — Hand-E-Food, Jan 17 '12 at 22:50

Eric Petroelje · Accepted Answer · 2012-01-17T21:51:59.337

7

The object itself is thread safe, but make sure you consider your usage of it as well. For example, if your usage looks like this:

if ( buffer.Dirty ) {
   var obj = buffer.GetValue();
}

That usage is NOT thread safe since the value of Dirty could change between when you check it and when you actually get the value.

To avoid that issue (and make minimal use of locking), you would want to use it like so:

if ( buffer.Dirty ) {
   lock(buffer) {
      if ( buffer.Dirty ) {
         var obj = buffer.GetValue();
      }
   }
}

edited Jan 17 '12 at 21:51

answered Jan 17 '12 at 21:46

Eric Petroelje

59,820
9
127
177

So then i should do something like: lock(_buffer){ if ( buffer.Dirty ) { var obj = buffer.GetValue(); } } ? – Jasper Jan 17 '12 at 21:48
Hmmm, so it's not needed to use same object to lock both the getting and the setting? – Jasper Jan 17 '12 at 22:10
Locking on the (public) buffer object itself is not so great either. Consider a `bool TryGetValue(out T value)` – H H Jan 17 '12 at 22:30

Kiril · Answer 2 · 2012-01-17T22:00:04.767

In short: no really.

Once you relinquish ownership of the value, then you can make absolutely no guarantees as to what's going to happen. This becomes particularly more pronounced when you rely on _value to have a certain value (no pun intended) in something like an if-statement. When that happens, all you've guaranteed is that the _value will not be in some partial writing state when its read.

The same is true for the dirty flag... frankly it's even more pronounced with the dirty flag.

Consider this case:

Thread 1 calls ASyncBuffer.SetValue(someValue) // sets the dirty flag to true
Thread 1 checks ASyncBuffer.Dirty  // should be true
Thread 2 calls ASyncBuffer.GetValue() // sets the flag to false
Thread 1 calls ASyncBuffer.GetValue() // you expect the dirty flag to be true, but it's not

In that sense, it's not thread safe.

score 0 · Answer 3 · answered Jan 17 '12 at 21:53

0

YES but only when accessing the property itself, as soon as the property is being used/assigned then it is up to the object being manipulated to handle its internal state being manipulated in a thread-safe manner.

answered Jan 17 '12 at 21:53

Shaun Wilde

8,228
4
36
56

Aw, so there is no way to internally handle all this stuff so I don't have to worry about it outside the object?:( – Jasper Jan 17 '12 at 22:15
Inside the implementation of the object you are storing you can use locks again to protect each property/method (use 1 or many locking objects as required) – Shaun Wilde Jan 18 '12 at 05:09

score 0 · Answer 4 · answered Jan 18 '12 at 10:21

Yes, but it's use might not be.

I'm assuming that you want to retrieve the value if and only if it's "dirty" (since that gets cleared on each retrieval, so I can't see the value in the opposite). You would therefore do:

if(buff.Dirty)
{
  T val = buff.GetValue();
  //operations on val.
}

However, if another thread calls GetValue() at the same time, then Dirty is now false.

Hence, its use is only safe for one reader thread (multiple writer threads is fine in this case, as they only ever change Dirty in the opposite direction).

If you could have multiple readers, then consider adding something like:

public bool GetIfDirty(out T value)
{
  lock (_locker)
  {
    if(!_dirty)
    {
      value = default(T);
      return false;
    }
    _dirty = false;
    value = _value;
    return true;
  }
}

Then you can both test Dirty and obtain the value if wanted, in the same threadsafe operation.

score -3 · Answer 5 · edited May 23 '17 at 12:29

-3

AFAIK, this is not a thread safe program. Your getter and setter will have different locks. Refer thread for more information

edited May 23 '17 at 12:29

Community

1
1

answered Jan 17 '12 at 21:44

Vinay

1,016
8
22

I don't understand what you mean by different locks. And the link isn't helping me. – Fantius Jan 17 '12 at 21:48
3

He has a single readonly lock object. He is not locking on the value of the property itself, so I don't think what you say is true. – Eric Petroelje Jan 17 '12 at 21:48
actually they use the same object to lock against and as such the locking around the property is thread safe. Changing the internal state of the object (as in your link) however is another matter entirely... – Shaun Wilde Jan 17 '12 at 21:50
@Eric - you are correct, the getter and setter share the lock object and are not independent. If the setter is invoked, the getter will block on the lock object until the setter releases it. – Eight-Bit Guru Jan 17 '12 at 21:52
What if I am using buffer.GetValue(); in one of the threads and buffer.SetValue(val) in the other thread. Will it be thread safe at that point in time? – Vinay Jan 17 '12 at 21:53
I am precisely referring to the example Lirik has just updated to, the thread will not be safe at that point in time. Yes it is safe if we are dealing with only 1 thread. – Vinay Jan 17 '12 at 21:55
@Vinay, all threads use the same object data. All threads use the same `_locker` value. Different `ASyncBuffer` objects will have separate `_locker` values, but that's intended. – Hand-E-Food Jan 17 '12 at 22:24

Is the following object thread safe?

5 Answers5