Java8: Issue on running a Thread example

Question

I came across the following example on winterbe.com which is demonstrating the use of Atomic variables.

// From http://winterbe.com/posts/2015/05/22/java8-concurrency-tutorial-atomic-concurrent-map-examples/
public class Test_AtomicInteger {
  public static void main(String[] args) {
      AtomicInteger atomicInt = new AtomicInteger(0);

      ExecutorService executor = Executors.newFixedThreadPool(2);

      IntStream.range(0, 1000)
          .forEach(i -> {
              Runnable task = () ->
                  atomicInt.updateAndGet(n -> n + 2);
              executor.submit(task);
          });

      executor.shutdownNow();

      System.out.println(atomicInt.get());    // => 2000
  }
}

Understand how the expected value 2000 is deduced from a Thread-Safe scenario. However, when I tried to execute it on eclipse IDE, it gives a different output value every time on each run. Would like see if anyone knows why it behaves like this. Thanks a lot.

score 3 · Answer 1 · answered Jan 23 '18 at 11:10

As said by others, shutdownNow() is inappropriate as it may cause enqueued tasks to be abandoned while at the same time does not wait for the completion of currently running tasks.

A correct sequence would be shutdown() followed by awaitTermination, however, you can do the same much simpler:

AtomicInteger atomicInt = new AtomicInteger(0);
ExecutorService executor = Executors.newFixedThreadPool(2);
executor.invokeAll(Collections.nCopies(1000, () -> atomicInt.updateAndGet(n -> n + 2)));
System.out.println(atomicInt.get());    // => 2000
executor.shutdown(); // only for cleanup

Here, invokeAll will invoke all tasks, all of them may run concurrently, and waits for the completion of all tasks. The executor doesn’t even need to be shut down, but could be reused for other tasks, however, it should be shut down once it is not needed anymore, to cleanup the underlying resources.

Collections.nCopies is the simplest way to get a List of identical elements, without even needing the storage to hold that number of references.

Since invokeAll expects a list of Callables rather than Runnables, the tasks will be Callables, but that doesn’t affect the semantic of this code.

score 2 · Accepted Answer · edited Jan 23 '18 at 11:33

2

Basically, Thread main is calling shutdownNow before all the tasks that were executed are done (even without calling shutdownNow you would still not see 2000, because you are querying the AtomicInteger still before the executor is done).

You really want to block until your executor is done or the timeout occurs:

executor.shutdown();
executor.awaitTermination(100, TimeUnit.MILLISECONDS);

If you would have looked carefully the author of the post where this was taken from defines:

 public static void stop(ExecutorService executor) {
    try {
        executor.shutdown();
        executor.awaitTermination(60, TimeUnit.SECONDS);
    }

    ....

edited Jan 23 '18 at 11:33

Holger

285,553
42
434
765

answered Jan 23 '18 at 10:25

Eugene

117,005
15
201
306

1

@Holger edited, thank you. I wanted to post something with `invokeAll`, just could not figure out how to it nicely. I would have assumed that `List> list = Collections.nCopies(1000, () -> atomicInt.updateAndGet(n -> n + 2))` would require a cast since it could be a `Supplier` too, but i guess the target type is also taken into consideration (since java-8 right?) and thus no cast needed – Eugene Jan 23 '18 at 11:30
2

Indeed, the target type tells it, otherwise, it could be *anything*, not just `Supplier` or `Callable`. Without target types, lambda’s would be almost unusable, so it’s obvious why that has been added in Java 8… – Holger Jan 23 '18 at 11:34
Thanks for the explanation! – Patrick C. Jan 23 '18 at 15:07

score 1 · Answer 3 · answered Jan 23 '18 at 10:06

The JavaDoc for shutdownNow says:

Attempts to stop all actively executing tasks, halts the processing of waiting tasks, and returns a list of the tasks that were awaiting execution.

This method does not wait for actively executing tasks to terminate. Use awaitTermination to do that.

So this does not wait for all the tasks you have submitted to finish, so just get the results for the threads that did manage to run.

To shutdown the service and wait for everything to finish replace the shutdownNow with something like:

executor.shutdown();

executor.awaitTermination(10, TimeUnit.SECONDS);

(you will need to catch the InterruptedException from awaitTermination somewhere).

Java8: Issue on running a Thread example

3 Answers3