unexpected multi thread output in java

Question

public class ConTest {

    @Test
    void name2() {
        final MyCounter myCounter = new MyCounter();
        final Thread t1 = new Thread(() ->
            myCounter.increment()
        );
        final Thread t2 = new Thread(() ->
            myCounter.increment()
        );
        t1.start();
        t2.start();
        System.out.println(myCounter.count);
    }

    @Test
    void name3() {
        final MyCounter myCounter = new MyCounter();
        final ExecutorService service = Executors.newFixedThreadPool(2);

        for (int i = 0; i < 2; i++) {
            service.execute(() -> {
                myCounter.increment();
            });
        }
        System.out.println(myCounter.count);
    }

    static class MyCounter {
        private AtomicLong count = new AtomicLong();

        public void increment() {
            count.incrementAndGet();
        }
    }
}

AtomicLong is safe when multi thread.

That is, in the example above, it was executed with 2 threads, so the result should be 2 no matter how many times it is executed.

However, after trying both tests several times, the result is sometimes 1. Why is this happening?

DuncG · Accepted Answer · 2021-10-16 14:34:37Z

1

You aren't waiting for any of the background threads or tasks to end before you print the value of the counter. To wait on the tasks to exit, you'll need to add this for threads:

t1.join();
t2.join();

Add this for the service, which prevents new tasks being added and waits a sensible period for them to end:

service.shutdown();
boolean done = awaitTermination(pickSuitablyLongPeriod, TimeUnit.MILLISECONDS);

Once you have ensured the background tasks are completed, the correct result should be printed when you run:

System.out.println(myCounter.count);

edited Oct 16, 2021 at 14:34

answered Oct 16, 2021 at 14:30

DuncG

16.3k3 gold badges31 silver badges44 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

roy man · Accepted Answer · 2021-10-16 15:00:39Z

1

This is because the threads are still processing when you call the System.out.println. In this case you would need to block the main thread before you print out the counter.

in the example of the Executor you can just await the termination:

final ExecutorService service = Executors.newFixedThreadPool(2);
final MyCounter myCounter = new MyCounter();

for (int i = 0; i < 100; i++) {
    service.submit(myCounter::increment);
}
service.shutdown();
while (!service.awaitTermination(100, TimeUnit.MILLISECONDS)) {
    System.out.println("waiting");
}
System.out.println(myCounter.count);

you should avoid to block in productive code, have a look at the Publish/Subscribe design pattern

edited Oct 16, 2021 at 15:00

answered Oct 16, 2021 at 14:53

roy man

968 bronze badges

Comments

Oussama ZAGHDOUD · Accepted Answer · 2021-10-16 15:37:52Z

1

dont forget to use shutdown() with Executors

see the comments here :

   // Here you start the 2  threads 
    for (int i = 0; i < 2; i++) {
        service.execute(() -> {
            myCounter.increment();
        });
    }

    // we are not sure here that your 2 threads terminate their tasks or not !!

    // the print will be executed by the Main Thread and maybe before the 2 threads terminate their 
    // job ,
    // maybe just one terminate , maybe no one from your 2 threads increment the count .
      System.out.println(myCounter.count);

You can use Future class , instead of execute you can use submit() , the retrun type will be of Type Futre<?> (accept void ) , after that with the Future object returned the method get() will block the execution until the result returned from the service :

Example method name3() : will return always 2

      void name3() {
        final MyCounter myCounter = new MyCounter();
        final ExecutorService service = Executors.newFixedThreadPool(2);
        Future<?> f = null;
        for (int i = 0; i < 2; i++) {
            f =service.submit(() -> {
                myCounter.increment();
            });
            try {
                f.get();
            } catch (InterruptedException | ExecutionException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }

         System.out.println(myCounter.count);
         service.shutdown();
    }

edited Oct 16, 2021 at 15:37

answered Oct 16, 2021 at 14:42

Oussama ZAGHDOUD

2,1898 silver badges18 bronze badges

2 Comments

Abra Over a year ago

I think it is better to call method shutdown as soon as you have submitted all the tasks. After calling shutdown, all the submitted tasks will continue to run. shutdown just means that no more tasks can be submitted.

Oussama ZAGHDOUD Over a year ago

A thread executor creates a non-daemon thread on the first task that is executed, so failing to call shutdown() will result in your application never terminating. it is very important to call shutDown() .

Tcheutchoua Steve · Accepted Answer · 2021-10-16 15:11:38Z

In addition to the above answers, you could add some prints to better understand what is happening.

In summary. You need to wait for the threads to finish executing before expecting the results, so it is not an issue of AtomicLong.

I modified the code, added some prints, and here are results from an execution.

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicLong;

import org.junit.jupiter.api.Test;

public class ConTest {

  @Test
  void name2() {
    final MyCounter myCounter = new MyCounter();
    final Thread t1 = new Thread(() -> {
      myCounter.increment();
      System.out.println("Counter increment t1 completed and the value is " + myCounter.getCount());
    });
    final Thread t2 = new Thread(() -> {
      myCounter.increment();
      System.out.println("Counter increment t2 completed and the value is " + myCounter.getCount());

    });

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

    System.out.println(myCounter.count.get());
  }

  @Test
  void name3() {
    final MyCounter myCounter = new MyCounter();
    final ExecutorService service = Executors.newFixedThreadPool(2);

    for (int i = 0; i < 2; i++) {
      service.execute(() -> {
        myCounter.increment();
        System.out.println("incrementing for count and the value is " + myCounter.getCount());
      });
    }

    System.out.println(myCounter.count.get());
  }

  class MyCounter {

    private AtomicLong count = new AtomicLong();

    public void increment() {
      count.incrementAndGet();
    }

    public long getCount(){
      return count.get();
    }
  }
}

Results (name2)

1
Counter increment t1 completed and the value is 1
Counter increment t2 completed and the value is 2

Results (name3)

incrementing for count and the value is 1
1
incrementing for count and the value is 2

You could also use a debugger to have a better understanding.

Collectives™ on Stack Overflow

unexpected multi thread output in java

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related