Search code examples
javaconcurrencyvolatile

How does volatile keyword ensure an object`s fields are visible to other threads?


public class ObjectPropertiesVolatileTest {

    static Cup cup = new Cup();

    public static void changeColor() {
        cup.setColor("black"); // change color of cup to black
    }

    public static void main(String[] args) {
        cup.setColor("red"); // initialize color of cup as red

        for(int i = 0; i < 3; i++) {
            new Thread(){
                public void run() {
                    for(int i = 0; i < 10; i++) 
                        System.out.println(Thread.currentThread().getName()
                                + " " + i + " is " + cup.getColor());

                    if(Thread.currentThread().getName().equals("Thread-1"))
                        changeColor(); // Thread-1 changes the color of cup

                    for(int i = 0; i < 10; i++) 
                        System.out.println(Thread.currentThread().getName()
                                + " " +i + " is " + cup.getColor());

                }
            }.start();
        }

    }

}

class Cup {

    private volatile String color;

    public String getColor() {
        return color;
    }

    public void setColor(String color) {
        this.color = color;
    }
}

My question is: how to ensure the field color of the object cup is visible to other threads only using volatile keyword instead of using synchronization or lock?


Solution

  • I believe it is working fine. The problem is that the printing, call to print statement, merging of strings in System.out.println(Thread.currentThread().getName() + " " +i + " is " + cup.getColor());, etc. are not synchronised.

    I added print statements to your getters/setters to log the time of calls:-

    public String getColor() {
        System.out.println("-------------------------get " + Thread.currentThread().getName() + this.color + " " + System.nanoTime());
        return color;
    }
    
    public void setColor(String color) {
        System.out.println("-------------------------set " + Thread.currentThread().getName() + color + this.color + " " + System.nanoTime());
        this.color = color;
    }
    

    A snippet of what it printed:-

    .
    .
    .
    Thread-0 4 is red
    -------------------------get Thread-1red 1356826035808750
    Thread-1 8 is red
    -------------------------get Thread-2red 1356826035425706
    Thread-2 2 is red
    -------------------------get Thread-1red 1356826035963697
    Thread-1 9 is red
    -------------------------get Thread-0red 1356826035894581
    -------------------------set Thread-1blackred 1356826036165653
    -------------------------get Thread-2red 1356826036036858
    Thread-2 3 is black
    -------------------------get Thread-1black 1356826036219728
    Thread-1 0 is black
    Thread-0 5 is red
    -------------------------get Thread-1black 1356826036402925
    Thread-1 1 is black
    -------------------------get Thread-2black 1356826036305139
    Thread-2 4 is black
    -------------------------get Thread-1black 1356826036535236
    Thread-1 2 is black
    .
    .
    .
    

    The problem you are concerned about is this line:- Thread-0 5 is red It should have printed black, right?

    Well, this thread entered the getter before the other thread changed the value. This is evident from these lines:-

    -------------------------get Thread-0red 1356826035894581
    -------------------------set Thread-1blackred 1356826036165653
    

    So it read the value correctly but the processing done after that took a long time as compared to the time taken by other threads in doing what they were doing.

    Happy for someone to correct me if I am wrong. :)