Why this Thread code print wrong unpredicted result sometimes?

Question

I'm java beginner and it's first time to use thread.

class Counter2 {
    private int value = 0;

    public void increment() {
        value++;
        printCounter();
    }

    public void decrement() {
        value--;
        printCounter();
    }

    public void printCounter() {
        System.out.println(value);
    }

}

class MyThread3 extends Thread {
    
    Counter2 sharedCounter;

    public MyThread3(Counter2 c) {
        
        this.sharedCounter = c;

    }

    public void run() {
        int i = 0;

        while (i <= 100) {
            sharedCounter.increment();
            sharedCounter.decrement();
            
            try {
                sleep((int) (Math.random() * 2));
            } catch (InterruptedException e) {
            }
//          System.out.println(i);
            i++;
        }
    }
}

public class MyTest {

    public static void main(String[] args) {
        Thread t1, t2;
        Counter2 c = new Counter2();
        t1 = new MyThread3(c);
        t1.start();
        t2 = new MyThread3(c);
        t2.start();
        
    }

}

This code has 2 threads and 1 Counter, which is shared between the threads. The threads just repeat plus 1, minus 1 to the counter value. So, if I guess, the result should be 0. Because initial value was 0 and the number of incremented and decremented are the same. But some times the last printing number is not the 0, but -1 or -2 etc. please explain why this is this.

Your value isn’t volatile so visibility isn’t guaranteed. Further increment and decrement operators are shorthand for the compound assignment operator, they are not atomic. Without external synchronisation, your counter isn’t threadsafe. — Boris the Spider, Jun 04 '21 at 05:10
I have duplinked this to an earlier question with counters and threads that is essentially the same problem, but a bit simpler. (The details are a different, but the cause of the problem is the same and the solutions are the same.) — Stephen C, Jun 04 '21 at 06:27
My advice: read a good tutorial on thread safety in Java. Or better still the definitive Bloch textbook (see below). This is a topic that you can't figure out by trial and error. — Stephen C, Jun 04 '21 at 06:29

Ravindra Ranwala · Answer 1 · 2021-06-04T06:32:28.080

There are two issues here. They are atomicity and visibility aspects of concurrency. Both increment and decrement are compound actions and need to be atomically performed in a multi-threaded environment. Apart from that you should not read a stale value whenever you read the counter. None of these are guaranteed by your current implementation.

Coming back to the solution, one naive way of achieving this is by using synchronized methods which uses a lock on the current instance to achieve the thread-safety. But that comes at a fairly high cost and incurs more lock overhead.

A much better approach would be to use CAS based non-blocking synchronization to achieve the task at hand. Here's how it looks in practice.

class Counter2 {
        private LongAdder value = new LongAdder();

        public void increment() {
            value.increment();;
            printCounter();
        }

        public void decrement() {
            value.decrement();;
            printCounter();
        }

        public void printCounter() {
            System.out.println(value.intValue());
        }

    }

Since you are a beginner, I would recommend you to read the great book Java Concurrency in Practice 1st Edition which explains all these basics in a very nice, graspable manner by some of the great authors in our era ! If you have any questions about the contents of the book, you are welcome to post the questions here too. Read it from cover to cover at least twice !

Update

CAS is so called ComparaAndSwap is a lock free synchronization scheme achieved by using low level CPU instructions. Here it reads the value of the counter before the increment/decrement and then at the time it is updated, it checks whether the initial value is still there. If so, it updates the value successfully. Otherwise, chances are that another thread concurrently updating the value of the counter, hence the increment/decrement operation fails and it retries it again.

Basil Bourque · Accepted Answer · 2021-06-04T06:31:53.407

The Answer by Ranwala is correct.

`AtomicInteger`

An alternative solution I prefer is the use of the Atomic… classes. Specifically here, AtomicInteger. This class is a thread-safe wrapper around an integer.

Change your member field from Counter2 sharedCounter; to AtomicInteger sharedCounter;. Then use the various methods on that class to increment, to decrement, and to interrogate for current value.

You can then discard your Counter2 class entirely.

Executors

Also, you should know that in modern Java, we rarely need to address the Thread class directly. Instead we use the executors framework added to Java 5.

Define your tasks as either a Runnable or Callable. No need to extend from Thread.

See tutorial by Oracle, and search existing posts here on Stack Overflow.

Why this Thread code print wrong unpredicted result sometimes?

2 Answers2

AtomicInteger

Executors

`AtomicInteger`