0

Why is count 0?

I start the thread 1 then I start thread 2. Count should be 2000. But it shows count to be 0. Someone please explain in simple terms.

public class App {
    private int count = 0;

    public static void main(String[] args) {
        App app = new App();
        app.doWork();
    }

    public void doWork(){
        Thread t1 = new Thread(new Runnable(){
            public void run(){
                for(int i = 0;i < 10000;i++){
                    count++;
                }
            }
        });

        Thread t2 = new Thread(new Runnable(){
            public void run(){
                for(int i = 0;i < 10000;i++){
                    count++;
                }
            }
        });
        t1.start();
        t2.start();

        System.out.println("Count is: " + count);
    }
}
zhm
  • 3,513
  • 3
  • 34
  • 55
Josh Bauss
  • 15
  • 1
  • 2
    Wait for the threads to run. after `t2.start();` add `t1.join(); t2.join();` – Elliott Frisch Mar 30 '17 at 03:26
  • Your code has several problems. As for the immediate question, my guess is that those threads haven't actually started executing. More important perhaps is that your `count` variable is not thread safe, i.e. the two threads could interleave when incrementing it, giving incorrect results. – Tim Biegeleisen Mar 30 '17 at 03:27
  • why haven't the threads started executing? Isn't start() suppose to execute them? Why do I have to wait with t1.join() and t2.join() ? – Josh Bauss Mar 30 '17 at 03:30
  • @TimBiegeleisen The point of this exercise is probably to illustrate that exact behavior. – Bill the Lizard Mar 30 '17 at 03:35

4 Answers4

4

At the time you're printing out your thread count, the threads have not finished executing yet.

To demonstrate, add a Thread.sleep() instruction before printing out the thread count:

public class App {
    private int count = 0;

    public static void main(String[] args) throws InterruptedException {
        App app = new App();
        app.doWork();
    }

    public void doWork() throws InterruptedException {
        Thread t1 = new Thread(new Runnable() {
            public void run() {
                for (int i = 0; i < 10000; i++) {
                    count++;
                }
            }
        });
        Thread t2 = new Thread(new Runnable() {
            public void run() {
                for (int i = 0; i < 10000; i++) {
                    count++;
                }
            }
        });

        t1.start();
        t2.start();

        Thread.sleep(5000);

        System.out.println("Count is: " + count); // Count is: 20000
    }
}

Also note that operations on primitives are not thread-safe and that the count++ operation is not atomic. You should synchronize access to your count variable, or use an AtomicInteger or LongAdder instead of an int. As it stands, you might end up with a count anywhere between zero and 20,000.

Robby Cornelissen
  • 91,784
  • 22
  • 134
  • 156
  • 1
    Is `count++` thread safe? What would happen if the bytecode for `count++` could interleave for the the threads? Could this lead to an incorrect final count, or even a final count of zero? – Tim Biegeleisen Mar 30 '17 at 03:33
  • ok so the thread hasn't started but why hasn't it started? t1.start() should start it so why wait? – Josh Bauss Mar 30 '17 at 03:35
  • @Tim , its not necessary when you called start method it will be started. – Hus Mukh Mar 30 '17 at 03:36
  • @TimBiegeleisen Operations on primitives are not thread-safe. Should use an `AtomicInteger` instead of an `int`, but I'm sure you knew that :) – Robby Cornelissen Mar 30 '17 at 03:36
  • @JoshBauss Have not started is maybe the wrong way of putting it. I'll update my answer to "have not finished executing yet". – Robby Cornelissen Mar 30 '17 at 03:38
  • @RobbyCornelissen even if the execution hasn't been finished should count be more than 0? I get it that I have to wait but I still don't see the underlying reason why it has to be done like that. – Josh Bauss Mar 30 '17 at 03:41
  • @JoshBauss Depends on how the JVM schedules execution of the threads. It's not under your control, although you can influence it by setting the thread's priority. – Robby Cornelissen Mar 30 '17 at 03:41
  • I believe this is causing race condition. For me the count comes to 11065 – Devendra Lattu Mar 30 '17 at 04:17
  • 1
    @DevendraLattu Indeed. It's caused by the `count` variable not being thread-safe, as noted at the end of my answer. – Robby Cornelissen Mar 30 '17 at 04:29
  • 1
    That's correct @RobbyCornelissen, however, as per my understanding, the count is an instance variable, and both the threads are trying to update the value of count at the same time. Therefore, I thought it might lead to a race condition. – Devendra Lattu Mar 30 '17 at 04:40
  • 1
    Not only is `count` not thread safe (threads may see stale values), the `count++` operation is not atomic. – bowmore Mar 30 '17 at 04:55
  • 1
    Oh, and `AtomicInteger` is a good way to solve the thread safety, but since Java 8, a better fit for this particular scenario is `LongAdder`. – bowmore Mar 30 '17 at 04:59
1

Integer's increment is not safe to multithreading. You should use AtmoicInteger like this:

public class App {

    private AtomicInteger count = new AtomicInteger(0);

    public static void main(String[] args) throws InterruptedException {
        App app = new App();
        app.doWork();
    }

    public void doWork() throws InterruptedException {
        Thread t1 = new Thread(new Runnable(){
            public void run(){
                for(int i = 0;i < 10000;i++){
                    count.getAndIncrement();
                }
            }
        });

        Thread t2 = new Thread(new Runnable(){
            public void run(){
                for(int i = 0;i < 10000;i++){
                    count.getAndIncrement();
                }
            }
        });
        t1.start();
        t2.start();
        t1.join();
        t2.join();
        System.out.println("Count is: " + count);
    }
}
Pang
  • 9,564
  • 146
  • 81
  • 122
Shu
  • 21
  • 5
0

It is because you are printing count in main thread. Main thread executed in its normal way , before child thread started. That's why your count is 0 , try to print count after a while , like put Thread.sleep(2000) before printing it.

Hus Mukh
  • 125
  • 12
  • Or you can check value of count in side run method. Just put a if clause in both thread if(count > 1999){ print count.... } – Hus Mukh Mar 30 '17 at 03:30
  • Is `count++` thread safe? If two threads attempt to do this to the same member variable at the same time, could this lead to problems? – Tim Biegeleisen Mar 30 '17 at 03:30
  • No for now your count variable not to be called thread safe. If you need thread safety you must use synchronized block / method. Seems you are new in Java Threading , go with some Java Threading tutorial to get a better understanding. – Hus Mukh Mar 30 '17 at 03:33
0

You need to make the code block which increments the count as synchronized in order to avoid the race condition.

In addition to this there are multiple ways to solve this problem-
1. Hard code Thread.sleep(miliseconds value) in Main thread. [Not recommended]
2. join() the two threads to the main Thread so that the control returns to the main thread only when the execution of the two threads is complete.

public class App implements Runnable {
    private int count = 0;

    public static void main(String[] args) throws InterruptedException {
        App app = new App();
        app.doWork();
    }

    public void doWork() throws InterruptedException {
        Thread t1 = new Thread(this, "Thread - 1");
        Thread t2 = new Thread(this, "Thread - 2");

        t1.start();
        t2.start();

        //1.
        t1.join();
        t2.join();

        //2.
        //Thread.sleep(1000);

        System.out.println("Count is: " + count); // Count is: 2000
    }

    @Override
    public synchronized void run() {

        for (int i = 0; i < 1000; i++) {
            count++;
        }
    }
}
Community
  • 1
  • 1
Devendra Lattu
  • 2,732
  • 2
  • 18
  • 27