4
votes

I know that TreeMap is not thread safe. I am trying to do a comparison of TreeMap with ConcurrentSkipListMap. Code I use is shown below and I want to make sure if the error I am getting is due TreeMap not being threadsafe and not because of some other.

Exception in thread "pool-1-thread-52" java.lang.NullPointerException at java.util.TreeMap.rotateLeft(TreeMap.java:2060) at java.util.TreeMap.fixAfterInsertion(TreeMap.java:2127) at java.util.TreeMap.put(TreeMap.java:574) at ThreadTestTreeMap$1.run(ThreadTestTreeMap.java:39) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) at java.lang.Thread.run(Thread.java:745)

import com.google.common.collect.Ordering;
import java.util.Map;
import java.util.TreeMap;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

public class ThreadTestTreeMap {
    public static Map<String, Object> map;
    public static int THREADS =  100;
    public static long averageTime = 0;

public static void main(String args[]) throws InterruptedException {
    for (int i = 0; i < 1; i++) {
        map = new TreeMap<>(Ordering.natural());
//            map = new ConcurrentSkipListMap<>(Ordering.natural());

        long time = System.nanoTime();
        ExecutorService service = Executors.newFixedThreadPool(THREADS);

        for (int j = 0; j < THREADS; j++) {
            final int finalJ = j;
            service.execute(new Runnable() {
                public void run() {
                    try {
                        Thread.sleep(THREADS - finalJ);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    long threadId = Thread.currentThread().getId();
                    map.put("tag"+threadId, "hello");
            }});
        }
        service.shutdown();
        service.awaitTermination(Long.MAX_VALUE, TimeUnit.DAYS);
        long timeUsed = (System.nanoTime() - time) / 1000000L;
        averageTime += timeUsed;
        System.out.println("All threads are completed in "
                + timeUsed + " ms");
    }
    System.out.println("The average time is " + averageTime / 10 + " ms");
}
}
1
Synchronize access with synchronized(map) { map.put("tag"+threadId, "hello"); } and rerun the code to check. Anyway, TreeMap isn't thread safe so, it's GUARANTEED you get an error (NullPointerExcaption or any other exception or logical error, it depends on implementation, see sources) while concurrent modification. Actually you get undefined behaviour with a set of possible errors.AnatolyG
@AnatolyG no, it's not guaranteed that you will get an exception or logical error; it's just not guaranteed that you won't.Andy Turner
@Andy :) Literally speaking yes, you're right. But this doesn't matter at all for me when I write my code. I expect the worst POSSIBLE scenario. And you see how easy to get it happened.AnatolyG
@AnatolyG Of course it matters. If there is no guarantee, you can't write code that relies on a guarantee. There's not much point in adding 'literally speaking' to any topic in IT. The whole discipline is literal.user207421
@AnatolyG that is an appropriate strategy, but describing it as a "guarantee" is not correct. Guarantees mean that you will get a big slap in the face when you do something wrong, so you know to correct it; what you have here is a problem that will only notice under certain conditions.Andy Turner

1 Answers

6
votes

Whether or not the NullPointerException is a direct result of the concurrent modification, it states in the Javadoc for TreeMap:

Note that this implementation is not synchronized. If multiple threads access a map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally.

As you are modifying the map in multiple threads without synchronization, you are not using the class as it is intended to be used.

Add external synchronization :)