Why hashmap is not thread safe？

Question

import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.*;


public class TestLock {
    private static ExecutorService executor = Executors.newCachedThreadPool();
    private static Map<Integer, Integer> map = new HashMap<>(1000000);
    private static CountDownLatch doneSignal = new CountDownLatch(1000);

    public static void main(String[] args) throws Exception {

        for (int i = 0; i < 1000; i++) {
            final int j = i;
            executor.execute(new Runnable() {
                @Override
                public void run() {
                    map.put(j, j);
                    doneSignal.countDown();
                }
            });
        }
        doneSignal.await();
        System.out.println("done,size:" + map.size());
    }
}

Some people say that hashmap insertion is not safe when concurrency. Because the hashmap will perform capacity expansion operations, but I set the size here to 1000000, which will only expand at 750,000. I am doing 1000 inserts here, so I won't expand it. So there should be no problem. But the result is always less than 1000, what went wrong?

Thread safety problems are not limited to expansion operations. Any modification operation can lead to race conditions, visibility issues, atomicity problems. — JB Nizet, Sep 27 '18 at 05:53
To illustrate, this simple operation, executed at each put, is not thread-safe: `++size`. — JB Nizet, Sep 27 '18 at 06:00
No, it is not "some people" that say `HashMap` is not thread-safe, it is [the documentation](https://docs.oracle.com/javase/10/docs/api/java/util/HashMap.html). Tip: If a class does not explicitly promise thread-safety in its documentation, always assume it is *not* thread-safe. — Basil Bourque, Sep 27 '18 at 06:04

Stephen C · Accepted Answer · 2021-07-13T07:37:27.593

Why hashmap is not thread safe？

Because the javadocs say so. See below.

You stated:

Some people say that hashmap insertion is not safe when concurrency.

It is not just "some people"¹. The javadocs state this clearly:

"Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more mappings; merely changing the value associated with a key that an instance already contains is not a structural modification."

You asked:

I am doing 1000 inserts here, so I won't expand it. So there should be no problem. But the result is always less than 1000, what went wrong?

It is not just expansion of the hash array that you need to think about. It is not just insertion. Any operation that performs a structural modification on the HashMap needs to be synchronized ... or you may get unspecified behavior².

And that is what you got.

^{1 - I strongly recommend that you do not rely on your intuition or on what "some people" say. Instead, take the time to read and understand the relevant specifications; i.e. the javadocs and the Java Language Specification.

2 - In this example, you can easily see why you get unspecified behavior by reading the HashMap source code. For instance in the OpenJDK Java 11 source code, size() is not synchronized, and it returns the value of the private transient int size field. This is not thread-safe. When other threads add or remove map entries they will update size, the thread calling size() is liable to get a stale value.}

Thank you very much, I will study more documents. – WQ WQ Sep 27 '18 at 06:18 — WQ WQ, Sep 27 '18 at 06:18

score 2 · Answer 2 · answered Sep 27 '18 at 05:54

"Because the hashmap will perform capacity expansion operations" is not only reason why HashMap is not thread safe.

You have to refer to Java Memory Model to understand what guarantee it can offer.

One of such guarantee is visibility. This mean that changes made in one thread may not be visible in other threads unless specific conditions are meet.

score 1 · Answer 3 · answered Sep 27 '18 at 05:57

1

Well the Question title is not really describing what you are asking for. Anyways,

Here you have set the Capacity to 1000000. Not the size.

Capacity : Initially how many slots to have in this hashmap. Basically Empty Slots.

Size : Number of Elements filled in the Map.

So even though you set the capacity to 1000000, you don't have that many elements at the end. So the number of elements filled in the map will be returned through .size() method. Its nothing related to a concurrent issue. And yes HashMap is not thread safe due to several reasons.

answered Sep 27 '18 at 05:57

Damith

740
4
12

I did 1000 inserts. – WQ WQ Sep 27 '18 at 06:02
That's pretty obvious as you Try to see the number of elements in the map via Main thread while the insertion may be going on. However the best case you can get is 1000. surely not 1000000. depending on how many elements have added by the executor thread at that particular instance when main thread is reading, your main thread reading will be changed. – Damith Sep 27 '18 at 06:14

score 0 · Answer 4 · answered Sep 27 '18 at 06:22

If you see the implementation of 'put' in HashMap class here, no where 'synchronize' is used although it does many thread-unsafe operations like creation of TreeNode if key's hash is not found, increment the modCount etc.

ConcurrentHashMap would be suitable for your use case

Marc Le Bihan · Answer 5 · 2018-09-27T07:14:32.220

-4

If you need a thread-safe HashMap, you may use the Hashtable class instead.

Unlike the new collection implementations, Hashtable is synchronized. If a thread-safe implementation is not needed, it is recommended to use HashMap in place of Hashtable, says the javadoc about Hashtable.

Same, if you need a thread-safe ArrayList one day, use Vector.

EDIT : Oh, I suggested the wrong way to do. My appologies ! comments suggest better solutions than mine : Collections.synchronizedXxx() or ConcurrentHashMap, that worked for the opener of this question.

edited Sep 27 '18 at 07:14

answered Sep 27 '18 at 05:59

Marc Le Bihan

2,308
2
23
41

2

Please, don't. These classes shouldn't be used anymore. If you want **synchronized** collections, then use `Collections.synchronizedXxx()`. – JB Nizet Sep 27 '18 at 06:01
1

Or - as Hashtable's docs state - ConcurrentHashMap – mtj Sep 27 '18 at 06:02
Changing to ConcurrentHashMap returns the correct size of 1000。 – WQ WQ Sep 27 '18 at 06:05
@mtj Actually, the [`HashMap` documentation](https://docs.oracle.com/javase/10/docs/api/java/util/HashMap.html) as of Java 10 makes no mention of [`ConcurrentHashMap`](https://docs.oracle.com/javase/10/docs/api/java/util/concurrent/ConcurrentHashMap.html). It discusses only: `Map m = Collections.synchronizedMap(new HashMap(...));` – Basil Bourque Sep 27 '18 at 06:08
@Marc You are welcome to edit your Answer at any time, to incorporate improvements suggested in these Comments. – Basil Bourque Sep 27 '18 at 06:10
I didn't know it was a good practice to re-edit an answer (I am a new user of Stackoverflow). But an old user of Java (since 1.2). Yes, I learned it with `Vector` and `Hashtable`... I went on `HashMap` and `ArrayList` when Vectors and Hastables has been deprecated, but I never believed they were so rejected now. – Marc Le Bihan Sep 27 '18 at 07:19

Why hashmap is not thread safe？

5 Answers5