Is the volatile keyword required for this double checked lock to work? [duplicate]

This question already has answers here: Closed 11 years ago.

Possible Duplicate:

The need for volatile modifier in double checked locking in .NET

Given the following code snippet, which can be executed by more than one thread at once, would the volatile keyword be required to ensure that the 'real value' of connected is always read from the memory and not a cache?

(Assume that Disconnect() 开发者_开发问答will only be called once (i.e. if it doesn't work properly the first time and the value of connected is read as false, it's not going to be attempted again)).

public class MyClass
{
    private readonly object syncRoot = new object();

    private bool connected; 

    public void Disconnect()
    {
        if (connected)
        {
            lock (syncRoot)
            {
                if (connected)
                {
                    // log off here
                    // ...
                    connected = false;
                }
            }
        }
    }

    public void Connect()
    {
        lock (syncRoot)
        {
            // blah
            // blah
            connected = true;
        }
    }
}

My feeling is that if double checked locking is used then it does need to be marked volatile since if it reads the incorrect value the first time, then it's going to think it's actually disconnected and won't go into the lock statement. I also think that in this case double checked locking isn't appropriate / isn't going to provide any performance gain and just a normal lock would do the job.

I'd like someone to confirm or deny these thoughts.

---

Provided the Connect() method also uses a lock (syncRoot) the code should work without volatile.

But you're not going to see any benefit from dcl here, why go through the trouble/risks?

---

It is my understanding of volatile that yes you would want it. I believe volatile will enforce that the value is read each time and would preclude the compiler caching the value or performing some other optimization that would cause the "correct" value not to be read from memory.

See Eric Lippert's answer where he mentions the use of volatile reads.

---

You are correct, the volatile keyword reads from memory and not the CPU cache. But it does not mean it is thread safe. Change the syncLock variable to a static modifier to ensure this is thread-safe across multiple instances. This is the recommended approach.