5

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");
}
}
wolφi
  • 8,091
  • 2
  • 35
  • 64
Rajind Ruparathna
  • 2,215
  • 24
  • 55
  • 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 May 05 '16 at 10:33
  • 3
    @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 May 05 '16 at 10:35
  • @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 May 05 '16 at 10:38
  • @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 May 05 '16 at 11:02
  • @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 May 05 '16 at 11:02
  • @Andy OK, then I GUARANTEE :) you get a big slap in the face from your app with just a few of sim writers and readers. Sooner or later, depends on frequency, but you get – AnatolyG May 05 '16 at 12:07
  • @EJP No, it doesn't just because I "can't write code" ANYWAY. I can't write code for both definitions "it's guaranteed that I will get an exception or logical error" or "it's just not guaranteed that I won't". I CAN'T. End of the story. – AnatolyG May 05 '16 at 12:34

1 Answers1

6

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 :)

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
  • 1
    Thanks. Actually I know that TreeMap is not thread safe. I am not planning to use it in a concurrent environment. I just would like to know what the error would be if I use it in a concurrent environment. I'm getting a similar error in a java client (influxdb-java) https://github.com/influxdata/influxdb-java. So I want to make sure this error is occurring due the use of TreeMap in that source. – Rajind Ruparathna May 05 '16 at 10:38
  • 1
    If you don't use the class as the documentation says it should be used, you're on your own. There are no guarantees about what will happen. If you want to see if it is because of *not* having synchronization.... see if it still happens if you add synchronization. – Andy Turner May 05 '16 at 10:42
  • @RajindRuparathna did you find a solution for it in influxdb-java? I am facing same issue currently. – mns Oct 16 '19 at 08:24
  • Found the relevant issue on the influxdb-java client here : https://github.com/influxdata/influxdb-java/issues/170 – mns Oct 16 '19 at 08:31