Is it safe to use get() method of LinkedHashMap in multi-thread application

Question

I want to write an multi-thread application that access LinkedHashMap with get() method (Application only get data from LinkedHashMap using get() method). The source code as below:

public class Thread1 implements Runnable {
LinkedHashMap<String, ArrayList<GroupMembers>> group;

public Thread1(LinkedHashMap<String, ArrayList<GroupMembers>> _group) {
     group = _group
}

public void run() {
    while (true) {
        Set<String> groupKeyNames = group.keySet();
        for (String groupName : groupKeyNames) {

            ArrayList<GroupMembers> members = group.get(groupName);
            /* Thread 1 processing */
            ...

        }
    }
}
}


public class Thread2 implements Runnable {
LinkedHashMap<String, ArrayList<GroupMembers>> group;

public Thread2(LinkedHashMap<String, ArrayList<GroupMembers>> _group) {
     group = _group
}

public void run() {
    while (true) {
        Set<String> groupKeyNames = group.keySet();
        for (String groupName : groupKeyNames) {

            ArrayList<GroupMembers> members = group.get(groupName);
            /* Thread 2 processing */
            ...

        }
    }
}
}

public class Sample {

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

        /* Init LinkedHashMap */
        LinkedHashMap<String, ArrayList<GroupMembers>> group;
        group.put("group1", new ArrayList<GroupMembers>());
        group.put("group2", new ArrayList<GroupMembers>());
        ...
        Thread1 t1 = new Thread(group);
        t1.start();
        Thread1 t1 = new Thread(group);
        t2.start();
    }
}

Is it safe to use LinkedHashMap.get() in above multi-thread application?

If the Map is _effectively final_, and _safely published_ (look those up), then yes, it's OK to `get()` from it without synchronization. Thread safety is only an issue when two or more threads access the same data, and one or more of them _updates_ it. — Solomon Slow, Jan 14 '16 at 17:07
This code doesn't work, since you don't assign a value to `group` before inserting values. How do you initialize it? — Andy Turner, Jan 14 '16 at 17:09
Why are you getting the `keySet`, iterating that to get the `members`? It would be easier either to just iterate `group.values()`, or `group.entrySet()` if you need the group name during iteration. — Andy Turner, Jan 14 '16 at 17:15

score 3 · Accepted Answer · answered Jan 14 '16 at 17:10

Quoting the Javadoc:

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

...

In insertion-ordered linked hash maps, merely changing the value associated with a key that is already contained in the map is not a structural modification. In access-ordered linked hash maps, merely querying the map with get is a structural modification.)

Whether it is insertion-ordered or access-ordered depends upon how you actually initialize it - which isn't included in the question:

If you construct the LinkedHashMap using new LinkedHashMap(int, float, boolean), it may be access-ordered (if you pass true as the boolean parameter), in which case it would not be thread-safe
If you construct it using any other constructor (or pass false as the boolean parameter), it would be thread-safe - provided you also do not go on and use the groups reference anywhere else.

I think the javadoc clarifies that access ordered LHM are not safe in this case - but I don't see it saying that a non-access ordered LHM would be safe... — assylias, Jan 14 '16 at 17:17

Is it safe to use get() method of LinkedHashMap in multi-thread application

1 Answers1