Shutting down worker threads gracefully

Question

I don't understand why in this implementation stopped is not volatile - If a different thread updates this will it be reflected correctly?

Secondly is testing (!Stopping) atomic?

using System;
using System.Threading;

/// <summary>
/// Skeleton for a worker thread. Another thread would typically set up
/// an instance with some work to do, and invoke the Run method (eg with
/// new Thread(new ThreadStart(job.Run)).Start())
/// </summary>
public class Worker
{
    /// <summary>
    /// Lock covering stopping and stopped
    /// </summary>
    readonly object stopLock = new object();
    /// <summary>
    /// Whether or not the worker thread has been asked to stop
    /// </summary>
    bool stopping = false;
     /// <summary>
    /// Whether or not the worker thread has stopped
    /// </summary>
    bool stopped = false;

    /// <summary>
    /// Returns whether the worker thread has been asked to stop.
    /// This continues to return true even after the thread has stopped.
    /// </summary>
    public bool Stopping
    {
        get
        {
            lock (stopLock)
            {
                return stopping;
            }
        }
    }

    /// <summary>
    /// Returns whether the worker thread has stopped.
    /// </summary>
    public bool Stopped
    {
        get
        {
            lock (stopLock)
            {
                return stopped;
            }
        }
    }

    /// <summary>
    /// Tells the worker thread to stop, typically after completing its 
    /// current work item. (The thread is *not* guaranteed to have stopped
    /// by the time this method returns.)
    /// </summary>
    public void Stop()
    {
        lock (stopLock)
        {
            stopping = true;
        }
    }

    /// <summary>
    /// Called by the worker thread to indicate when it has stopped.
    /// </summary>
    void SetStopped()
    {
        lock (stopLock)
        {
            stopped = true;
        }
    }

    /// <summary>
    /// Main work loop of the class.
    /// </summary>
    public void Run()
    {
        try
        {
            while (!Stopping)
            {
                // Insert work here. Make sure it doesn't tight loop!
                // (If work is arriving periodically, use a queue and Monitor.Wait,
                // changing the Stop method to pulse the monitor as well as setting
                // stopping.)

                // Note that you may also wish to break out *within* the loop
                // if work items can take a very long time but have points at which
                // it makes sense to check whether or not you've been asked to stop.
                // Do this with just:
                // if (Stopping)
                // {
                //     return;
                // }
                // The finally block will make sure that the stopped flag is set.
            }
        }
        finally
        {
            SetStopped();
        }
    }
}

edit

This code is from an article written by Jon Skeet.

Answer 1

Because it is only ever accessed inside the lock. A lock also acts to ensure that you see the up-to-date values.

Re atomicity (and I assume you really mean synchronized here?); it makes no odds; even if Stopping is synchronized, we can no longer trust the value is up to date as soon as we exit the lock, which we have. Hence !Stopping is no more or less synchronized than Stopping. The important thing is that we know we have at least checked recently. There is an edge case where the flag is changed just after we check, but that is fine: when we checked, it was true that we should continue.

Answer 2

See this entry on SO. It explains why lock is preferred over volatile.

Answer 3

Behavior of this code is defined in section 3.10 of the C# Language Specification:

Execution of a C# program proceeds such that the side effects of each executing thread are preserved at critical execution points. A side effect is defined as a read or write of a volatile field, a write to a non-volatile variable, a write to an external resource, and the throwing of an exception. The critical execution points at which the order of these side effects must be preserved are references to volatile fields (§10.5.3), lock statements (§8.12), and thread creation and termination.

In other words, the lock statement is sufficient guarantee to avoid the need to declare the stopped field volatile.

How this rule is implemented by the JIT compiler is an interesting question, given that the lock statement merely calls the Monitor.Enter() and Exit() methods. I don't think it has special knowledge of those methods, I think it is a side-effect of entering the try block which starts after the Enter() call. But that's just a guess.