Java Volatile variable still causing race condition

Question

I was running an experiment on different approaches to deal with race condition in multi-threaded java applications . Strategies like atomic variables, synchronize worked well , but I dont see the issue being solved when using volatile variables. Here the code and output for reference.

Can you please guide on what could be the reason for the volatile variable to still lead to a race condition?

package com.shyam.concurrency;


public class main {
    public static void main(String[] args) {

   demoClass dm1 = new demoClass();
        Thread th1 = new Thread(()->{
        int i =0;
        do {
            i++;
            dm1.setCounter();
            dm1.setAtomicCounter();
            dm1.setSyncCounter();
            dm1.setVolatileCounter();
        } while (i < 100000);
        });

        Thread th2 = new Thread(()->{
            int i =0;
            do {
                i++;
                dm1.setCounter();
                dm1.setAtomicCounter();
                dm1.setSyncCounter();
                dm1.setVolatileCounter();
            } while (i < 100000);

        });

        th1.start();
        th2.start();

        try {
            th1.join();
            th2.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("Normal counter(Race condition) : " + dm1.getCounter() );
        System.out.println("Synchronized counter is :" + dm1.getSyncCounter());
        System.out.println("Atomic counter is :" + dm1.getAtomicCounter());
        System.out.println("Volatile counter is :" + dm1.getVolatileCounter());

The code that has the increment logic is here:

package com.shyam.concurrency;

import java.util.concurrent.atomic.AtomicInteger;

public class demoClass {
    private  int counter ;
    private int syncCounter;
    private volatile  int volatileCounter = 0;
    private AtomicInteger  atomicCounter  = new AtomicInteger() ;


 
    public int getAtomicCounter() {
        return atomicCounter.intValue();
    }

    public void setAtomicCounter() {
        this.atomicCounter.addAndGet(1);
    }

    public int getCounter() {
        return counter;
    }

    public void setCounter() {
        this.counter++;
    }
    public synchronized int getSyncCounter() {
        return syncCounter;
    }

    public synchronized void setSyncCounter() {
        this.syncCounter++;
    }

    public int getVolatileCounter() {
        return volatileCounter;
    }

    public void setVolatileCounter() {
        this.volatileCounter++;
    }
}

And here's the output i get:

Normal counter(Race condition) : 197971
Synchronized counter is :200000
Atomic counter is :200000
Volatile counter is :199601

Answer 1

Visibility versus Atomicity

The volatile solves only the problem of visibility. This means that every thread sees the current value of a variable rather than possibly seeing a stale cached value.

Your line:

this.volatileCounter++;

… is performing multiple operations:

• Fetch the current value of that variable
• Increment that value
• Store the new value in that variable

This group of operations is not atomic.

While one thread has fetched the value but not yet incremented and stored the new value, a second thread may access the same current value. Both threads increment the same initial value, so both threads produce and save the same redundant new value.

For example, two or more threads might access a value of 42. All of those threads would then increment to 43, and each thread would store 43. That number 43 would be stored over and over again. Some other thread might have even seen one of those 43 writes, and then incremented & stored a 44. One of the remaining threads yet to write its 43 would end up overriding the write of 44. So not only might you waste some attempts to increment and thereby fail to move the number forward, you might actually see the number move backward (effectively decrement).

If you want to use volatile you must protect the code to make atomic the multiple operations. The synchronized keyword is one such solution.

Personally, I prefer using the AtomicInteger approach instead. If you instantiate an AtomicInteger before any access attempt, and never replace that instance, then visibility of the reference variable for that AtomicInteger is a non-issue. Having no opportunity for stale cached values mean no visibility problem. And regarding racy access to its payload, the methods of the AtomicInteger provide atomicity for the simple manipulations (hence the name, obviously),

To learn more about visibility issues, study the Java Memory Model. And read the excellent book, Java Concurrency In Practice by Brian Goetz, et al.