0

Created 2 threads and called the join() method on them. I was expecting the threads would run sequentially and give the desired result, but I am getting some random values in the result.

public class TestJoinMethod {
    volatile private int count =0;

    public static void main(String[] args) {
        TestJoinMethod testJoinMethod = new TestJoinMethod();
        testJoinMethod.execute();
    }

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

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

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

        try {
            t1.join();
            System.out.println(count);
            t2.join();
            System.out.println(count);
        } catch (InterruptedException e) {
            throw new RuntimeException(e);
        }
    }
}

Using the above code to get the value of count variable. As per my understanding the join() method waits for the thread execution to be completed before the control of the calling thread goes to next statement. Expected result : sysout statement should print 10000 , 200000 respectively. However, the result is giving some random values : 14125 , 14125 or 16911, 16911. Not able to understand why is this happening here, went through some tutorials also, but couldn't understand the exact issue here.

Appreciate your help here.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
nupur_s
  • 9
  • 3

2 Answers2

2

So let's think about the sequence that happens here. This part:

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

Starts the two threads running, concurrently with each other. We haven't executed a join() yet, just started the two threads, so they're both running.

    try {
        t1.join();
        System.out.println(count);

Then we get to this join(). Both t1 and t2 have been running for a while at this point. Finally, t1 finishes and then we print out count. But t2 has also been running, so we expect count to have been incremented more than it would have been by t1 alone.

Then we get to this:

        t2.join();
        System.out.println(count);

This does pretty much the same thing: wait for t2 to finish, then print out count again.

It's essentially impossible to predict how much of the execution of t1 and t2 may overlap with each other though. Maybe a lot, but maybe almost none at all.

Worse, you haven't done anything to protect access to count, you're getting a data race. The result is, in a word, garbage.

To get the result you're apparently looking for, you'd have prevent concurrent access of your threads though (in other words, render the use of threads utterly useless). The obvious way to do that would be something like:

t1.start();
t1.join();
System.out.println(count);
t2.start();
t2.join();
System.out.println(count);

This should produce the result you desire--but it's equivalent to just running all the code in one thread, so there's no real point to any of it.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
1

The compiler already is warning you that

Non-atomic operation on volatile field 'count'

The threads can randomly read the same value and update it at the same time, which will result in the same number. For example:

  1. Thread 1 reads count = 0
  2. Thread 2 reads count = 0
  3. Thread 1 does 0+1
  4. Thread 1 updates count = 1
  5. Thread 2 does 0+1
  6. Thread 2 updates count = 1

This can be solved synchronizing the access to the count variable, for example using AtomicInteger:

private final AtomicInteger count = new AtomicInteger(0);

count.getAndIncrement();

Also note that the final result will be 20000 for both threads, you're incrementing the same counter.

m0skit0
  • 25,268
  • 11
  • 79
  • 127