Is accessing a variable in C# an atomic operation?

Question

I've been raised to believe that if multiple threads can access a variable, then all reads from and writes to that variable must be protected by synchronization code, such as a "lock" statement, because the processor might switch to another thread halfway through a write.

However, I was looking through System.Web.Security.Membership using Reflector and found code like this:

public static class Membership
{
    private static bool s_Initialized = false;
    private static object s_lock = new object();
    private static MembershipProvider s_Provider;

    public static MembershipProvider Provider
    {
        get
        {
            Initialize();
            return s_Provider;
        }
    }

    private static void Initialize()
    {
        if (s_Initialized)
            return;

        lock(s_lock)
        {
            if (s_Initialized)
                return;

            // Perform initialization...
            s_Initialized = true;
        }
    }
}

Why is the s_Initialized field read outside of the lock? Couldn't another thread be trying to write to it at the same time? Are reads and writes of variables atomic?

Answer 1

I thought they were - I'm not sure of the point of the lock in your example unless you're also doing something to s_Provider at the same time - then the lock would ensure that these calls happened together.

Does that //Perform initialization comment cover creating s_Provider? For instance

private static void Initialize()
{
    if (s_Initialized)
        return;

    lock(s_lock)
    {
        s_Provider = new MembershipProvider ( ... )
        s_Initialized = true;
    }
}

Otherwise that static property-get's just going to return null anyway.

Answer 2

The Initialize function is faulty. It should look more like this:

private static void Initialize()
{
    if(s_initialized)
        return;

    lock(s_lock)
    {
        if(s_Initialized)
            return;
        s_Initialized = true;
    }
}

Without the second check inside the lock it's possible the initialisation code will be executed twice. So the first check is for performance to save you taking a lock unnecessarily, and the second check is for the case where a thread is executing the initialisation code but hasn't yet set the s_Initialized flag and so a second thread would pass the first check and be waiting at the lock.

John.

Answer 3

@Keith
The "Perform initialization" comment is standing in for all the config-reading, class-instantiating, and settings-setting that Membership does to initialize s_Provider, so I understand why that's in a "lock" - so that it's only done once.

@littlegeek
I think you're looking at the MembershipProvider class. My example comes from System.Web.Security.Membership, which descends directly from Object.

Answer 4

What you're asking is whether accessing a field in a method multiple times atomic -- to which the answer is no.

In the example above, the initialise routine is faulty as it may result in multiple initialision. You would need to check the sInitialized flag inside the lock as well as outside, to prevent a race condition in which multiple threads read the sInitialized flag before any of them actually does the initialisation code. eg.

private static void Initialize()
{
    if (s_Initialized)
        return;

    lock(s_lock)
    {
        if (s_Initialized)
            return;
        s_Provider = new MembershipProvider ( ... )
        s_Initialized = true;
    }
}

[Edit: John got this before me]

Answer 5

@John Richardson
You're right. The real Membership class has that second check. I left it out because what I'm really interested in is whether the first call to s_Initialized, outside the lock, is thread-safe.

Answer 6

Perhaps Interlocked gives a clue. And otherwise this one i pretty good.

I would have guessed that their not atomic.

Answer 7

I think you're asking if s_Initialized could be in an unstable state when read outside the lock. The short answer is no. A simple assignment/read will boil down to a single assembly instruction which is atomic on every processor I can think of.

I'm not sure what the case is for assignment to 64 bit variables, it depends on the processor, I would assume that it is not atomic but it probably is on modern 32 bit processors and certainly on all 64 bit processors. Assignment of complex value types will not be atomic.

John.

Answer 8

Reads and writes of variables are not atomic. You need to use Synchronisation APIs to emulate atomic reads/writes.

For an awesome reference on this and many more issues to do with concurrency, make sure you grab a copy of Joe Duffy's latest spectacle. It's a ripper!

Answer 9

"Is accessing a variable in C# an atomic operation?"

Nope. And it's not a C# thing, nor is it even a .net thing, it's a processor thing.

OJ is spot on that Joe Duffy is the guy to go to for this kind of info. ANd "interlocked" is a great search term to use if you're wanting to know more.

"Torn reads" can occur on any value whose fields add up to more than the size of a pointer.

Answer 10

Ack, nevermind... as pointed out, this is indeed incorrect. It doesn't prevent a second thread from entering the "initialize" code section. Bah.

You could also decorate s_Initialized with the volatile keyword and forego the use of lock entirely.

Answer 11

For the definitive answer go to the spec. :)

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."

So that confirms that s_Initialized will never be unstable, and that read and writes to primitve types are atomic.

Interlocking creates a memory barrier to prevent the processor from reordering reads and writes. The lock creates the only required barrier in this example.

John.

Answer 12

You could also decorate s_Initialized with the volatile keyword and forego the use of lock entirely.

That is not correct. You will still encounter the problem of a second thread passing the check before the first thread has had a chance to to set the flag which will result in multiple executions of the initialisation code.

John.

Answer 13

The correct answer seems to be, "Yes, mostly."

1. John's answer referencing the CLI spec indicates that accesses to variables not larger than 32 bits on a 32-bit processor are atomic.

2. Further confirmation from the C# spec, section 5.5, Atomicity of variable references:

   Reads and writes of the following data types are 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 are also atomic. Reads and writes of other types, including long, ulong, double, and decimal, as well as user-defined types, are not guaranteed to be atomic.

3. The code in my example was paraphrased from the Membership class, as written by the ASP.NET team themselves, so it was always safe to assume that the way it accesses the s_Initialized field is correct. Now we know why.

Edit: As Thomas Danecker points out, even though the access of the field is atomic, s_Initialized should really be marked volatile to make sure that the locking isn't broken by the processor reordering the reads and writes.

Answer 14

Hang about -- the question that is in the title is definitely not the real question that Rory is asking.

The titular question has the simple answer of "No" -- but this is no help at all, when you see the real question -- which i don't think anyone has given a simple answer to.

The real question Rory asks is presented much later and is more pertinent to the example he gives.

Why is the s_Initialized field read outside of the lock?

The answer to this is also simple, though completely unrelated to the atomicity of variable access.

The s_Initialized field is read outside of the lock because locks are expensive.

Since the s_Initialized field is essentially "write once" it will never return a false positive.

It's economical to read it outside the lock.

This is a low cost activity with a high chance of having a benefit.

That's why it's read outside of the lock -- to avoid paying the cost of using a lock unless it's indicated.

If locks were cheap the code would be simpler, and omit that first check.

(edit: nice response from rory follows. Yeh, boolean reads are very much atomic. If someone built a processor with non-atomic boolean reads, they'd be featured on the DailyWTF.)

Answer 15

@Leon
I see your point - the way I've asked, and then commented on, the question allows it to be taken in a couple of different ways.

To be clear, I wanted to know if it was safe to have concurrent threads read and write to a boolean field without any explicit synchronization code, i.e., is accessing a boolean (or other primitive-typed) variable atomic.

I then used the Membership code to give a concrete example, but that introduced a bunch of distractions, like the double-check locking, the fact that s_Initialized is only ever set once, and that I commented out the initialization code itself.

My bad.

Answer 16

This is a (bad) form of the double check locking pattern which is not thread safe in C#!

There is one big problem in this code:

s_Initialized is not volatile. That means that writes in the initialization code can move after s_Initialized is set to true and other threads can see uninitialized code even if s_Initialized is true for them. This doesn't apply to Microsoft's implementation of the Framework because every write is a volatile write.

But also in Microsoft's implementation, reads of the uninitalized data can be reordered (ie. prefetched by the cpu), so if s_Initalized is true, reading the data that should be initialized can result in reading old, uninitalized data because of cache-hits (ie. the reads are reordered).

For example:
Thread 1 reads s_Provider (which is null)
Thread 2 initializes the data
Thread 2 sets s_Initalized to true
Thread 1 reads s_Initialized (which is true now)
Thread 1 uses the previously read Provider and gets a NullReferenceException

Moving the read of s_Provider before the read of s_Initialized is perfectly legal because there is no volatile read anywhere.

If s_Initialized would be volatile, the read of s_Provider would not be allowed to move before the read of s_Initialized and also the initialization of the Provider is not allowed to move after s_Initialized is set to true and everything is ok now.

Joe Duffy also wrote an Article about this problem: Broken variants on double-checked locking