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?

Heinrik

For those of you reading this post as it currently exists, it is a little confusing. The code above has been adjusted to display a correct way to double check with a lock (inside & outside). However, the question text refers to the initial code sample which did not include the second s_initialized check. It's a bit confusing without looking at the edit history. – Caber

@Caber Sorry for the confusion, but the question DOES refer to the current code, including the second check inside the lock. This is not a question about how to do double-check locking, it's about atomic memory access, and whether a boolean could be written to at the same time as it's being read. – Heinrik

Answer 1

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 smaller than 32 bits are atomic.

In particular, double and long (Int64 and UInt64) are not guaranteed to be atomic on a 32-bit platform. You can use the methods on the Interlocked class to protect these.

Additionally, while reads and writes are atomic, there is a race condition with addition, subtraction, and incrementing and decrementing primitive types, since they must be read, operated on, and rewritten. The interlocked class allows you to protect these using the CompareExchange and Increment methods.

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

Haemophilia

Though accessing a memory location no larger than the native word size is atomic, the sample code provided in the question is not thread safe because of read/write reordering. See my answer for more details. – Purveyance

Good spot, Thomas. Everyone should read be sure to read your answer as well. – Haemophilia

C# 4 spec 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. 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. – Compartment

Answer 2

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 uninitialized data can be reordered (i.e. prefetched by the cpu), so if s_Initialized is true, reading the data that should be initialized can result in reading old, uninitialized 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\_Initialized 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

Purveyance

Good point. Reading the article you linked to, whether the code in Membership is right or wrong seems to depend on the precise details of the compiler and the processor, but I agree that s_Initialized should be made volatile just to be sure. – Heinrik

It is wrong according to .net's Memory Modell. On certain processors and/or with certain compilers, you may be lucky enought that it still works. Joe Duffy wrote another article: bluebytesoftware.com/blog/2008/07/17/… – Purveyance

The absence of write reordering is a characteristic of x86 and x64 but not necessarily Microsoft's .NET, i.e. I would not rely upon it if Microsoft add support for other architectures where the hardware does reorder writes. – Norse

Well @Thomas I don't think this is an issue here, because of the lock statement already done what you are locking for by using volatile: The lock statement creates a full memory fence, and that is even more that what the volatile do, the volatile is creating a half fence "acquire and release semantics" while lock creates full fence, So because of the MemoryBarrier created by lock the s_Provider will be fresh here; to understand it more you could read the above code like: Thread.MemoryBarrier(); .... s_Initialized = true; Thread.MemoryBarrier(); return s_Provider;` – Sinister

@JalalAldeenSaa'd You're right about the memory barrier, but in this case that only guarantees s_Initialized has the correct value in the Provider property. It would have been different if s_Initialized simply wasn't there and only s_Provider would have been checked - but unfortunately that's not the case here. Another way to fix it is to put a Thread.MemoryBarrier() before the s_Initialized = true;. Note that atomicity in this case doesn't matter - it's about reordering of instructions. – Copolymerize

Answer 3

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

Incompetence

Answer 4

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.

Heinrik

I see your point, but the code in my example is accurate with respect to how the locking is done in the actual Membership class. Perhaps the ASP.NET team know enough about their compiler and their targeted processor to be confident that no reordering is going to occur. All the same... – Heinrik

...I agree with Joe Duffy in the article you linked to when he says that we should always use "volatile" in situations like this just to be on the safe side. – Heinrik

If "int" is atomic, why does Interlocked.Increment()/Decrement() accept int types? – Nimble

because Interlocked.Increment does the load, increment and store in one atomic operation. – Purveyance

Answer 5

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.

Haemophilia

That's no better, you might as well remove the statement outside the lock. Also, if the only thing that is done here is to set initialized to true then the original "non-safe" ver. was good enough. At worst you set it twice, the only reason for the early return is performance (not correctness). – Gabel

I did say that the first check is for performance. Locks are very expensive so it's always worth doing. On the second point, I think it's reasonable to assume more complex code has been omitted. I somehow doubt MS would go to the expense of a lock unnecessarily. – Haemophilia

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. – Heinrik

Answer 6

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.

Haemophilia

Answer 7

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!

Goolsby

Answer 8

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

Incompetence

Answer 9

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.

Haemophilia

Answer 10

An If (itisso) { check on a boolean is atomic, but even if it was not there is no need to lock the first check.

If any thread has completed the Initialization then it will be true. It does not matter if several threads are checking at once. They will all get the same answer, and, there will be no conflict.

The second check inside the lock is necessary because another thread may have grabbed the lock first and completed the initialization process already.

Thibault

Answer 11

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.

Culpepper

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. – Heinrik

Answer 12

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 initialization. You would need to check the s_Initialized flag inside the lock as well as outside, to prevent a race condition in which multiple threads read the s_Initialized flag before any of them actually does the initialisation code. E.g.,

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

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

Infield

Answer 13

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

I would have guessed that their not atomic.

Fluted

Answer 14

To make your code always work on weakly ordered architectures, you must put a MemoryBarrier before you write s_Initialized.

s_Provider = new MemershipProvider;

// MUST PUT BARRIER HERE to make sure the memory writes from the assignment
// and the constructor have been wriitten to memory
// BEFORE the write to s_Initialized!
Thread.MemoryBarrier();

// Now that we've guaranteed that the writes above
// will be globally first, set the flag
s_Initialized = true;

The memory writes that happen in the MembershipProvider constructor and the write to s_Provider are not guaranteed to happen before you write to s_Initialized on a weakly ordered processor.

A lot of thought in this thread is about whether something is atomic or not. That is not the issue. The issue is the order that your thread's writes are visible to other threads. On weakly ordered architectures, writes to memory do not occur in order and THAT is the real issue, not whether a variable fits within the data bus.

EDIT: Actually, I'm mixing platforms in my statements. In C# the CLR spec requires that writes are globally visible, in-order (by using expensive store instructions for every store if necessary). Therefore, you don't need to actually have that memory barrier there. However, if it were C or C++ where no such guarantee of global visibility order exists, and your target platform may have weakly ordered memory, and it is multithreaded, then you would need to ensure that the constructors writes are globally visible before you update s_Initialized, which is tested outside the lock.

Sustain

Answer 15

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.

Gastrostomy

Making s_Initialized volatile won't help. In particular, it won't stop two CPUs from reading the same value and erroneously proceeding to create the object. Making it volatile could seem to correct issues, because accesses to volatile variables may serialize memory accesses entirely, leaving memory in an inconsistent state periodically, but hiding the bugs well enough to get through tests but fail under real-world stress. – Sustain