Is a string property itself threadsafe?

Question

String's in C# are immutable and threadsafe. But what when you have a public getter property? Like this:

public String SampleProperty{
    get;
    private set;
}

If we have two threads and the first is calling 'get' and the second is calling 'set' at the "same" time, what will happen?

IMHO the set must made a lock to be thread-safe like this:

private string sampleField;
private object threadSafer = new object();

public String SampleProperty{
    get{ return this.sampleField; }
    private set{
        lock(threadSafer){
            sampleField = value;
        }
    }
 }

Answer 1

A reference-type field's get/set (ldfld/stfld) is (IIRC) guaranteed to be atomic, so there shouldn't be any risk of corruption here. So it should be thread-safe from that angle, but personally I'd lock the data at a higher level - i.e.

lock(someExternalLock) {
    record.Foo = "Bar";
}

or maybe:

lock(record.SyncLock) {
    record.Foo = "Bar";
}

This allows you to make multiple reads/updates to the same object as an atomic operation, so that other threads can't get an invalid object-state

Answer 2

Setting the string is an atomic operation, i.e. you will either get the new string or the old string, you'll never get garbage.

If you're doing some work e.g.

obj.SampleProperty = "Dear " + firstName + " " + lastName;

then string concatination all happens before the call to set, therefore sampleField will always either be the new string or the old.

If however your string concatination code is self referential e.g.

obj.SampleProperty += obj.SampleProperty + "a";

and else where on another thread you have

obj.SampleProperty = "Initial String Value";

Then you need the lock.

Consider you're working with an int. If you're assigning to the int, and any value you get from the int is valid, then you don't need to lock it.

However, if the int is keeping count of the number of widgets processed by two or more threads, for the count to be accurate, you need to lock the int. It's the same situation for strings.

I've a feeling I didn't explain this very well, hope it helps.

Thanks

BW

Answer 3

This is thread-safe without any need for locking. Strings are reference types, so only a reference to the string is being modified. References are of a type are guaranteed to be atomic (Int32 on 32 bit systems and Int64 on 64 bit).

HTH, Kent

Answer 4

Your second code sample is definitely not right, because locks only have the desired effect when they are used in all places where the variable is accessed (both for get and set), so the get would also need a lock.

However, when getting and setting a reference-type field as a property like this, then adding a lock statement doesn't add any value. Assignments to pointers are guaranteed to be atomic in the .NET environment, and if multiple threads are changing a property then you have an inherent race condition anyway (where threads may see different values; this may or may not be a problem) so there's little point in locking.

So for what it does, the first piece of code is fine. But whether you really want to build inherent race conditions into a multi-threaded application is another matter.

Answer 5

Most of the answers are using the word "atomic" as if atomic changes are all that are needed. They're not, usually.

This has been mentioned in the comments, but not usually in the answers - that's the only reason for me providing this answer. (The point about locking at a coarser granularity, to allow things like appending, is entirely valid as well.)

Usually you want a reading thread to see the latest value of the variable/property. That isn't guaranteed by atomicity. As a quick example, here's a bad way to stop a thread:

class BackgroundTaskDemo
{
    private bool stopping = false;

    static void Main()
    {
        BackgroundTaskDemo demo = new BackgroundTaskDemo();
        new Thread(demo.DoWork).Start();
        Thread.Sleep(5000);
        demo.stopping = true;
    }

    static void DoWork()
    {
         while (!stopping)
         {
               // Do something here
         }
    }
}

DoWork may well loop forever, despite the write to the boolean variable being atomic - there's nothing to stop the JIT from caching the value of stopping in DoWork. To fix this, you either need to lock, make the variable volatile or use an explicit memory barrier. This all applies to string properties as well.