0

I'm learning concurrency basics in Java and I'm stuck with one problem in this example below. There are two threads that should run concurrently and finish their loops working on some global static object. I expected that one thread could finish before another and print different value but I didn't expect to see both of threads printing the same value which suggest that both of them did not finish their loops before printing output.

Code:

public class TestThreads {
    public static void main(String[] args) {
        ThreadOne t1 = new ThreadOne();
        ThreadTwo t2 = new ThreadTwo();
        Thread one = new Thread(t1);
        Thread two = new Thread(t2);
        one.start();
        two.start();
    }
}

class Accum {
    private static Accum a = new Accum();
    private int counter = 0;
    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("index in one = " + x);
        }           
        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("index in two = " + x);
        }
        System.out.println("two " + a.getCount());
    }
}

Why is this happening? When I tried to debug with print statements (commented in code above) everything starts to work as expected.

My output: (it's different each time, this is also strange)

one 82067 two 82067

This is actually a code-puzzle from book "Head First Java" (2nd Edition), but author provides the same code as above as solution to different behavior.

Book output: (the same code as above)

one 98098 two 98099

Book explanation:

Threads from two different classes are updating the same object in a third class, because both threads are accessing a single instance of Accum.

To sum up, my questions are:

  1. Why the output of my code is different each time?
  2. Why loops in threads are not finishing their work?
  3. Why my output is different from this provided in book?**

** - Additional

contrl
  • 109
  • 1
  • 7
  • You don't wait for the threads to finish. Use `one.join(); two.join();`. – Andy Turner Jan 03 '19 at 14:11
  • 1
    It's called race condition, which might produce unpredictable result. Check this for more. https://stackoverflow.com/questions/34510/what-is-a-race-condition – xingbin Jan 03 '19 at 14:15
  • @AndyTurner, joining the two threads from `main()` won't change anything. As written, the `main()` thread doesn't do anything except die after it starts the two workers. What difference would it make whether it waits before it dies? – Solomon Slow Jan 03 '19 at 14:52
  • I don't know the answer, but those two threads are going to spend most of their time sleeping, and therefore, I would not be surprised if they run in close-to-lock-step with each other. (1) What happens if you change the loop boundaries? Like, 99 and 120 instead of 99 and 98? (2) What happens if you take out the `sleep()` calls? – Solomon Slow Jan 03 '19 at 14:56
  • @AndyTurner yes I know that method, but actually, I'm waiting for them to finish because print statements are after loops in both run() methods. – contrl Jan 03 '19 at 15:06
  • @SolomonSlow in both modifications program still provides non-reliable output (not finishing loops, different each time) – contrl Jan 03 '19 at 15:06
  • I thought your question was, why to the two threads always print the same value as each other? The reason why they do _not_ always print the same value each _time_ probably is due to the race condition. – Solomon Slow Jan 03 '19 at 15:14

1 Answers1

2

no idea how the book expects that to work, but your Accum instance is not thread safe. therefore, each time you run it you could get a different answer. If you make updateCounter() and getCount() synchronized, it should work reliably.

jtahlborn
  • 52,909
  • 5
  • 76
  • 118
  • Yes: the synchronisation not only ensures that both thread cannot execute a method at the same time, but on a multicore/multiprocessor machine, it also ensures that the content of the cache memory is consistent. – Maurice Perry Jan 03 '19 at 14:35
  • 1
    Note that you can also use an `AtomicInteger` instead of an `int`. – Maurice Perry Jan 03 '19 at 14:40
  • @jtahlborn it's working but not answering my question completely. Additional question: how can it be not thread safe? I thought that if there is only one statement in the method we can call it 'atomic'. – contrl Jan 03 '19 at 15:09
  • 2
    @contrl it must be atomic at the hardware level. An increment is three cpu steps: load, add, store. – Ben Manes Jan 03 '19 at 17:56
  • 1
    @contrl in addition to what Ben Manes said (which is correct), there is no guarantee that any given "single statement" in java is atomic (e.g. assignment to a double). there are _specific_ guarantees about _specific_ statements, but you need to know what they are. – jtahlborn Jan 03 '19 at 19:14
  • 2
    An increment is not atomic, but further, when multiple threads update the same variable without a proper thread safe construct, there is no guaranty that one thread sees the updates of the other, so they could overwrite the variable with the result of a calculation based on an older value. – Holger Jan 11 '19 at 14:59