Why doesn't the volatile keyword work as expected in java code?

Question

I'm learning concurrency knowledge in Java. About volatile keyword, it should make variable visible in different threads. But in my demo code, it doesn't seem to work as expected. The method run() in the class which implements Runnable will never stop.

public class VisibilityDemo {

  public static void main(String[] args) throws InterruptedException {
    TimeConsumingTask timeConsumingTask = new TimeConsumingTask();
    Thread thread = new Thread(new TimeConsumingTask());
    thread.start();
    Thread.sleep(3000);
    timeConsumingTask.cancel();
  }
}

class TimeConsumingTask implements Runnable {
  private volatile boolean toCancel = false;

  @Override
  public void run() {
    while (! toCancel) {
      System.out.println("executing...");
      try {
        Thread.sleep(1000);
      } catch (InterruptedException e) {
        e.printStackTrace();
      }
    }
    if (toCancel) {
      System.out.println("Task was canceled.");
    } else {
      System.out.println("Task done.");
    }
  }

  public void cancel() {
    toCancel = true;
    System.out.println(this + " canceled.");
  }
}

Answer 1

In your main method you have two instances of your task:

  public static void main(String[] args) throws InterruptedException {
    TimeConsumingTask timeConsumingTask = new TimeConsumingTask(); //<-- one
    Thread thread = new Thread(new TimeConsumingTask()); //<-- two
    thread.start();
    Thread.sleep(3000);
    timeConsumingTask.cancel(); //<-- cancel() on first 
  }
}

You're passing one to the Thread constructor and then call cancel on the other one. You need to call cancel on the instance passed to Thread, like so:

  public static void main(String[] args) throws InterruptedException {
    TimeConsumingTask timeConsumingTask = new TimeConsumingTask(); 
    Thread thread = new Thread(timeConsumingTask); //<-- difference here
    thread.start();
    Thread.sleep(3000);
    timeConsumingTask.cancel();
  }
}