Is it necessary to make this variable volatile?

Question

I was going through an "JAX London 2011" presentation on "Modern Java Concurrency". Between the time duration 43:20 - 43:40, a person from the audience says the shutdown variable in the code below should have been declared as volatile and the presenters agree with it (and say that it was pointed out earlier as well, but they just didnt get to modify the presentation). The code in question is:

public abstract class QueueReaderTask implements Runnable {

  private boolean shutdown = false;
  protected BlockingQueue<WorkUnit<String>> lbq;

  public void run() {
    while (!shutdown) {
      try {
        WorkUnit<String> wu = lbq.poll(10, TimeUnit.MILLISECONDS);
        if (wu != null) { doAction(wu.getWork()); }
      } catch (InterruptedException e) {
        shutdown = true;
      }
    }
  }

  public abstract void doAction(String msg);
  public void setQueue(BlockingQueue<WorkUnit<String>> q) { lbq = q; }
}

My Question: I dont think that shutdown should be declared volatile. My reasoning is that shutdown being a member of a Runnable, each task/thread will have a distinct private copy of that variable. So, why make it volatile?

But since this was discussed in JAX 2011, I am assuming there were lots of expert Java developers in that audience. I dont think all of them would have missed this ! So, what am I missing ?

P.S:- I can understand that a variable should be declared volatile if it was (potentially) shared by multiple threads, as in the Double-Checked-Locking pattern :

class Foo {
        private volatile Helper helper = null;
        public Helper getHelper() {
            if (helper == null) {
                synchronized(this) {
                    if (helper == null)
                        helper = new Helper();
                }
            }
            return helper;
        }
}

Answer 1

each task/thread will have a distinct private copy of that variable. So, why make it 'volatile' ?

You are correct if the shutdown boolean is only modified from within the QueueReaderTask instance. In that case shutdown is only being modified by the one thread and doesn't need to be volatile.

Frankly, the code looks strange to me. Why catch InterruptedException, set the shutdown boolean, and then loop around and exit. Why now just do the following? Why have the shutdown flag at all?

while (true) {
  try {
    WorkUnit<String> wu = lbq.poll(10, TimeUnit.MILLISECONDS);
    if (wu != null) { doAction(wu.getWork()); }
  } catch (InterruptedException e) {
     Thread.currentThread().interrupt();
     return;
  }
}

Maybe there is extra code that was removed in the post? If not, I wonder if this was copy and pasted from a larger section of code where shutdown was set to true also in a method call.

P.S:- I can understand that a variable should be declared 'volatile' if it was (potentially) shared by multiple threads, as in the Double-Checked-Locking pattern :

Right. A typical pattern is that shutdown is modified from another thread which is telling the thread to stop processing. In that case it needs to be volatile.