I'm new to programming and been studying threads for some time now.

So, the following code should give an output of:

one 98098
two 98099

and it does sometimes.

When I try to run it for a couple of times, it gives different outputs. I can understand that the JVM controls the threads and I can't directly affect it, but some of the outputs are less than 98,000 even though the for loop is adding 1000 for 98 times. How is this happening? Can a thread leave lines behind? Or did I do something wrong (note: the expected output sometimes shows on the screen, but not always)

public class TestThreads {
    public static void main(String [] args) {
        ThreadOne t1 = new ThreadOne();
        Thread one = new Thread(t1);
        ThreadTwo t2 = new ThreadTwo();
        Thread two = new Thread(t2);
        one.start();    
        two.start();
    }
}
class Accum {
    private int counter = 0;
    private static Accum a = new Accum();
    private Accum() {
    }
    public static Accum getAccum() {
        return a;
    }
    public int getCount() {
        return counter;
    }
    public void updateCounter(int add) {
        counter  = add;
    }
}

class ThreadOne implements Runnable {
    Accum a = Accum.getAccum();
    public void run() {
        for(int x=0; x < 98; x  ) {
            a.updateCounter(1000);
            try {
                Thread.sleep(50);
            } catch(InterruptedException ex) { }
        }
        System.out.println("one " a.getCount());
    }
}

class ThreadTwo implements Runnable {
    Accum a = Accum.getAccum();
    public void run() {
        for(int x=0; x < 99; x  ) {
            a.updateCounter(1);
            try {
                Thread.sleep(50);
            } catch(InterruptedException ex) { }
        }
        System.out.println("two " a.getCount());
    }
}

CodePudding user response：

Basically, your updateCounter method isn't thread-safe. If it's called from two threads at the same time, you can lose information.

Let's rewrite it to make it more obvious why that's the case:

public void updateCounter(int add) {
    // Fetch
    int originalValue = counter;
    // Compute
    int newValue = originalValue   add;
    // Store
    counter = newValue;
}

Imagine what happens if two threads come into the method at the same time. We'll pretend that there's some "total ordering" of what happens - the reality is more complex than that, but even the simplified form shows the problem. Suppose counter has a value of 5 to start with, and on thread x we're calling updateCounter(3) and on thread y we're calling updateCounter(4). We could imagine this sequence of events:

1. Thread x executes the "fetch" operation: originalValue = 5 (local variable, unaffected by thread y)
2. Thread y executes the "fetch" operation: originalValue = 5
3. Thread x executes the "compute" operation: newValue = 8
4. Thread y executes the "compute" operation: newValue = 9
5. Thread x executes the "store" operation: counter = 8 (note that newValue in thread x is separate to the one in thread y)
6. Thread y executes the "store" operation: counter = 9

So we end up with the value of counter being 9... as if the updateCounter(3) call had never taken place. If the last two operations happened in the reverse order, then counter would be 8 instead.

The way to fix this is to use the AtomicInteger class which is designed specifically to make operations like this atomic:

class Accum {
    private final AtomicInteger counter = new AtomicInteger(0);
    private static Accum a = new Accum();

    private Accum() {
    }

    public static Accum getAccum() {
        return a;
    }

    public int getCount() {
        return counter.get();
    }

    public void updateCounter(int add) {
        counter.addAndGet(add);
    }
}