5

I have a generic field and a property that encapsulates it:

T item;

public T Item
{
    get { return item; }
    set { item = value; }
}

The problem is that this property can be written to from one thread and read from multiple threads at the same time. And if T is a struct, or long, readers might get results that are part old value and part new value. How can I prevent that?

I tried using volatile, but that's not possible:

A volatile field cannot be of the type 'T'.

Since this is a simpler case of code I've already written, which uses ConcurrentQueue<T>, I thought about using it here too:

ConcurrentQueue<T> item;

public T Item
{
    get
    {
        T result;
        item.TryPeek(out result);
        return item;
    }

    set
    {
        item.TryEnqueue(value);
        T ignored;
        item.TryDequeue(out ignored);
    }
}

This would work, but it seems to me that it's overcomplicated solution to something that should be simple.

Performance is important, so, if possible, locking should be avoided.

If a set happens at the same time as get, I don't care whether get returns the old value or the new value.

svick
  • 236,525
  • 50
  • 385
  • 514
  • Why did you want to avoid locking, but thought of using the ConcurrentQueue? Do you think it's lock free inside? – Andriy Kizym Jul 18 '12 at 15:31
  • 1
    @anderhil I'm not sure how exactly is it implemented, but I thought all of the concurrent collections are as lock-free as possible. – svick Jul 18 '12 at 15:32

3 Answers3

3

I originally considered Interlocked, but I don't think it actually helps here as T isn't constrained to be a reference type. (If it were, the atomicity would be fine already.)

I would honestly start with locking - then measure performance. If the lock is uncontended, it should be really cheap. Only consider getting more esoteric when you've proved that the simplest solution is too slow.

Basically your expectation that this is simple fails due to the unconstrained genericity here - the most efficient implementation will differ based on the type.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • That's what I was checking, but Interlocked.Exchange(...) requires T to be a reference type. :) – Ani Jul 18 '12 at 15:14
  • @ananthonline: Indeed - that's why I discarded it too. Sorry, meant to include that. – Jon Skeet Jul 18 '12 at 15:15
  • Wait, won't this (http://msdn.microsoft.com/en-us/library/f2090ex9) overload of Interlocked.Exchange work? It takes objects. – Ani Jul 18 '12 at 15:16
  • @ananthonline it takes references to `object`s, which means the field would have to be declared as `object`, which means boxing. – svick Jul 18 '12 at 15:18
  • Will the boxing/unboxing be "cheaper" than locking? – Andriy Kizym Jul 18 '12 at 15:19
  • Well - it will be boxed automatically from T, will it not? And yes, I believe boxing should be far cheaper than locking. – Ani Jul 18 '12 at 15:19
  • You don't need to use `Interlocked` at all for reads/writes. `Interlocked` is useful when you when you want to use the *old* value, which isn't being done here. Simple assignment/read is fine, thread-safe, and lockless. – casperOne Jul 18 '12 at 15:21
  • Normally, I would start with locking too. But this code is for a highly concurrent library, so I try to avoid locks when possible. – svick Jul 18 '12 at 15:22
3

It completely depends on the type, T.

If you are able to place a class constraint on T then you don't need to do anything in this particular case. Reference assignments are atomic. This means that you can't have a partial or corrupted write to the underlying variable.

Same thing goes for reads. You won't be able to read a reference that is partially written.

If T is a struct, then only the following structures can be read/assigned atomically (according to section 12.5 of the C# specification, emphasis mine, also justifies the above statement):

Reads and writes of the following data types shall be atomic: bool, char, byte, sbyte, short, ushort, uint, int, float, and reference types. In addition, reads and writes of enum types with an underlying type in the previous list shall also be atomic. Reads and writes of other types, including long, ulong, double, and decimal, as well as user-defined types, need not be atomic. Aside from the library functions designed for that purpose, there is no guarantee of atomic read-modify-write, such as in the case of increment or decrement.

So if all you're doing is trying to read/write, and you meet one of the conditions above, then you don't have to do anything (but it means that you also have to place a constraint on the type T).

If you can't guarantee the constraint on T, then you'll have to resort to something like the lock statement to synchronize access (for reads and writes as mentioned before).

If you find that using the lock statement (really, the Monitor class) degrades performance, then you can use the SpinLock structure, as it's meant to help in places where Monitor is too heavy:

T item;

SpinLock sl = new SpinLock();

public T Item
{
    get 
    { 
        bool lockTaken = false;

        try
        {
            sl.Enter(ref lockTaken);
            return item; 
        }
        finally
        {
            if (lockTaken) sl.Exit();
        }
    }
    set 
    {
        bool lockTaken = false;

        try
        {
            sl.Enter(ref lockTaken);
            item = value;
        }
        finally
        {
            if (lockTaken) sl.Exit();
        }
    }
}

However, be careful, as the performance of SpinLock can degrade and will be the same as the Monitor class if the wait is too long; of course, given that you are using simple assignments/reads, it shouldn't take too long (unless you are using a structure which is just massive in size, due to copy semantics).

Of course, you should test this yourself for the situations that you predict that this class will be used and see which approach is best for you (lock or the SpinLock structure).

Community
  • 1
  • 1
casperOne
  • 73,706
  • 19
  • 184
  • 253
  • So you're saying it's impossible to read unconstrained `T` without using some kind of locking? – svick Jul 18 '12 at 15:28
  • @svick Pretty much. There are guarantees that the CLR will make for you for *certain* types, but not for *all* types. If you want to cover *all* types, then you have to use a locking mechanism of *some* type. If you're overly concerned about the performance of `lock` (because tests have told you so) then I'd look into `SpinLock` (as mentioned); but make sure you test to make sure that you actually *get* a performance boost and aren't hurting yourself. – casperOne Jul 18 '12 at 15:32
-2

Why do you need to protect this at all?

Changing the referenced instance of a variable is a atomic operation. So what ever you read with get won't be invalid. You can't tell if its the old or the new instance when set is running at the same time. But other then that you should be fine.

Partition I, Section 12.6.6 of the CLI spec states: "A conforming CLI shall guarantee that read and write access to properly aligned memory locations no larger than the native word size is atomic when all the write accesses to a location are the same size."

And as your variable is a reference type it always has the size of the native word. So your result is never invalid if you do something like this:

Private T _item;
public T Item
{
    get
    {
        return _item;
    }

    set
    {
        _item = value
    }
}

Example if you want to stick to the generic stuff and use it for everything. The approach is the use of a carrier helper class. It reduces the performance considerably but it will be lock free.

Public Foo
{
    Private Carrier<T> 
    {
        T _item
    }

    Private Carrier<T> _item;
    public T Item
    {
        get
        {
            Dim Carrier<T> carrier = _item;
            return carrier.item;
        }



set
    {
        Dim Carrier<T> carrier = new Carrier<T>();
        carrier.item = value;
        _item = carrier;
    }
}

}

This way you can ensure that you always use referenced types and your access is lock-free. Downside is that all set operations create garbage.

Nitram
  • 6,486
  • 2
  • 21
  • 32
  • 2
    No, changes of variables *aren't* necessarily atomic. The specification quote you gave explicitly states **no larger than the native word size*. The variable *isn't* always a reference type - it's generic. The OP specifically called out the possibility of it being `long` or a custom struct. – Jon Skeet Jul 18 '12 at 15:14
  • I see. I highly suggest then stepping away from this generic stuff and use more type specific features. Or you use a carrier class. – Nitram Jul 18 '12 at 15:17
  • You seem to be mixing your languages. – Daniel Kelley Jul 18 '12 at 15:37
  • Gah. Yes. Sorry. More of the vb.net person myself. But You get the point I think. – Nitram Jul 18 '12 at 15:43