Confused about java concurrency results

Question

So I am studying up on java concurrency by trying to create bad concurrent examples, watch them fail and then fix them.

But the code never seems to be breaking... What am I missing here?

I have a "shared object", being my HotelWithMaximum instance. As far as I can tell, this class is not thread safe:

package playground.concurrent;

import java.util.ArrayList;
import java.util.List;

public class HotelWithMaximum {

    private static final int MAXIMUM = 20;

    private List<String> visitors = new ArrayList<String>();

    public void register(IsVisitor visitor) {
        System.out.println("Registering : " + visitor.getId());
        System.out.println("Amount of visitors atm: " + visitors.size());

        if(visitors.size() < MAXIMUM) {
            //At some point, I do expect a thread to be interfering here where the condition is actually evaluated to
            //true, but some other thread interfered, adds another visitor, causing the previous thread to go over the limit
            System.out.println("REGISTERING ---------------------------------------------------------------------");
            //The interference might also happen here i guess...
            visitors.add(visitor.getId());
        }
        else{
            System.out.println("We cant register anymore, we have reached our limit! " + visitors.size());
        }
    }

    public int getAmountOfRegisteredVisitors() {
        return visitors.size();
    }

    public void printVisitors() {
        for(String visitor: visitors) {
            System.out.println(visitors.indexOf(visitor) + " - " + visitor);
        }
    }
}

The visitors are 'Runnables' (they implement my interface IsVisitor which extends from Runnable), and they are implemented like this:

package playground.concurrent.runnables;

import playground.concurrent.HotelWithMaximum;
import playground.concurrent.IsVisitor;

public class MaxHotelVisitor implements IsVisitor{

    private final String id;
    private final HotelWithMaximum hotel;

    public MaxHotelVisitor(String id, HotelWithMaximum hotel) {
        this.hotel = hotel;
        this.id = id;
    }

    public void run() {
        System.out.println(String.format("My name is %s and I am trying to register...", id));
        hotel.register(this);
    }

    public String getId() {
        return this.id;
    }

}

Then, to make all of this run in an example, I have the following code in a different class:

public static void executeMaxHotelExample() {
        ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(6);
        HotelWithMaximum hotel = new HotelWithMaximum();

        for(int i = 0; i<100; i++) {
            executor.execute(new MaxHotelVisitor("MaxHotelVisitor-" + i, hotel));
        }

        executor.shutdown();

        try{
            boolean finished = executor.awaitTermination(30, TimeUnit.SECONDS);

            if(finished) {
                System.out.println("FINISHED WITH THE MAX HOTEL VISITORS EXAMPLE");
                hotel.printVisitors();
            }
        }
        catch(InterruptedException ie) {
            System.out.println("Something interrupted me....");
        }
    }

    public static void main(String[] args) {
        executeMaxHotelExample();
    }

Now, what am I missing? Why does this never seem to fail? The hotel class is not thread safe, right? And the only thing to make it 'enough' thread safe for this example (since no other code is messing with the thread unsafe List in the hotel class ), I should just make the register method "synchronized", right? The result of the "printVisitors()" method in the main method, always looks like this:

FINISHED WITH THE MAX HOTEL VISITORS EXAMPLE
0 - MaxHotelVisitor-0
1 - MaxHotelVisitor-6
2 - MaxHotelVisitor-7
3 - MaxHotelVisitor-8
4 - MaxHotelVisitor-9
5 - MaxHotelVisitor-10
6 - MaxHotelVisitor-11
7 - MaxHotelVisitor-12
8 - MaxHotelVisitor-13
9 - MaxHotelVisitor-14
10 - MaxHotelVisitor-15
11 - MaxHotelVisitor-16
12 - MaxHotelVisitor-17
13 - MaxHotelVisitor-18
14 - MaxHotelVisitor-19
15 - MaxHotelVisitor-20
16 - MaxHotelVisitor-21
17 - MaxHotelVisitor-22
18 - MaxHotelVisitor-23
19 - MaxHotelVisitor-24

There are nevere more then 20 visitors in the list... I find that quite weird...

Answer 1

You are right.

You can reproduce the concurrency issues when you use more executors at the same time, say Executors.newFixedThreadPool(100); instead of 6. Then more threads will try it at the same time and the probability is higher. Because the race condition/overflow can only happen once, you will have to run your main more times to get more visitors.

Further you need to add a Thread.yield() at both places where you expect the "interference", to make it more likely to happen. If the execution is very short/fast there will not be a task switch and the execution will be atomic (but not guaranteed).

You might also write the code using ThreadWeaver which does byte code manipulation (adding yields) to make such issues more likely.

With both changes I get 30 and more visitors in the hotel from time to time. I have 2x2 CPUs.

Answer 2

the code never seems to be breaking... What am I missing here?

The Java Language Specification gives implementors a lot of leeway to make the most efficient use of any given multi-processor architecture.

If you obey the rules for writing "safe" multi-threaded code, then that's supposed to guarantee that a correctly implemented JVM will run your program in the way that you expect. But if you break the rules, that does not guarantee that your program will misbehave.

Finding concurrency bugs by testing is a hard problem. A non "thread-safe" program might work 100% of the time on one platform (i.e., architecture/OS/JVM combination), it might always fail on some other platform, and its performance in some third platform might depend on what other processes are running, on the time of day or, on other variables that you can only guess at.

Answer 3

ThreadPoolExecutor is from the java.util.concurrent package

The Java Concurrency Utilities framework in the java.util.concurrent package is a library that contains thread-safe types that are used to handle concurrency in Java applications

So ThreadPoolExecutor is taking care of the syncorinous processing

take note: ThreadPoolExecutor uses BlockingQueue to manage its job queue java.util.concurrent.BlockingQueue is an interface that request all implementations to be thread-safe.

From my understanding one of the main goals of java.util.concurrent was that you can to a large extent operate without the need to use java's low-level concurrency primitives synchronized, volatile, wait(), notify(), and notifyAll() which are difficult to use.

Also note that ThreadPoolExecutor implements ExecutorService which does not guarantee all implementations are thread-safe but according to the documentation http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ExecutorService.html

Actions in a thread prior to the submission of a Runnable or Callable task to an ExecutorService happen-before any actions taken by that task, which in turn happen-before the result is retrieved via Future.get().

Explanation of happen-before:

Java™ Language Specification defines the happens-before relation on memory operations such as reads and writes of shared variables. The results of a write by one thread are guaranteed to be visible to a read by another thread only if the write operation happens-before the read operation.

In other words - generally not thread-safe. BUT

The methods of all classes in java.util.concurrent and its subpackages extend these guarantees to higher-level synchronization.