3

I have a hashmap used in multiple threads at the same time. To make it thread safe I put it into a synchronized block:

private final Map<Long, DeviceConnection> mapConnections = new HashMap()<>;

...

synchronized (mapConnections) {
        List<Long> toClear = new ArrayList<>();
        for (Map.Entry<Long, AndroidSocketConnection> entry : mapConnections.entrySet()) {
            if (entry.getValue().isReadyToRemove())) {
                removed++;
                toClear.add(entry.getKey());
            }
        }
        for(Long toC : toClear) {
            mapConnections.remove(toC);
        }
    }

I thought if I put it into synchronized block I do not have to care about such stuff, but this Exception is thrown:

java.util.ConcurrentModificationException

at java.util.HashMap$HashIterator.nextNode(HashMap.java:1442)

at java.util.HashMap$EntryIterator.next(HashMap.java:1476)

at java.util.HashMap$EntryIterator.next(HashMap.java:1474)

at myPackage.network.DeviceHandler.doClearing(DeviceHandler.java:51) // -> this line contains the for loop head of the code I showed

at java.lang.Thread.run(Thread.java:748)

Community
  • 1
  • 1
julienduchow
  • 1,026
  • 1
  • 11
  • 29
  • Is there other code elsewhere that operates on `mapConnections`? – Joni Mar 31 '20 at 18:53
  • yes sure. Thats why I am using synchronized block. I put stuff and remove stuff from this map all the time in other threads – julienduchow Mar 31 '20 at 19:01
  • 2
    do those other threads also access through `synchronized (mapConnections)` blocks? – Joni Mar 31 '20 at 19:02
  • No, but I have references onto objects in this HashSet in which I change attributes not in synchronized blocks could that be a problem? – julienduchow Mar 31 '20 at 19:09
  • Well that explains the `ConcurrentModificationException`. As for operating on other objects without synchronization, yes it is definitely a problem. It won't cause a `ConcurrentModificationException` but you'll have bugs that are hard to understand and difficult to reproduce. – Joni Mar 31 '20 at 19:16

1 Answers1

3

It will only be thread-safe if every access (both reads and writes) to the map is performed via a synchronized block.

ConcurrentModificationException will be thrown when the map is being iterated on while it is being modified.

I would suggest you switch to a ConcurrentHashMap which is thread-safe and will be a drop-in replacement.

ᴇʟᴇvᴀтᴇ
  • 12,285
  • 4
  • 43
  • 66
  • Sounds good, but I put all other read stuff on this HashSet into synchronized blocks aswell. – julienduchow Mar 31 '20 at 19:03
  • But I have references onto objects in this HashSet in which I change attributes not in synchronized blocks could that be a problem? – julienduchow Mar 31 '20 at 19:04
  • 1
    @julien-100000 Do those changed attributes affect the result of the `hashCode` method? That would be very bad. – Basil Bourque Mar 31 '20 at 19:06
  • yes Im pretty sure they do, because the class is annotated with Lomboks @Data – julienduchow Mar 31 '20 at 19:07
  • If I use ConcurrentHashMap do I still need to care that the hashcode is not changed or can I ignore this then? @BasilBourque – julienduchow Mar 31 '20 at 19:12
  • 1
    No, it won't work if you change the hashCode of an item's key. You will have to remove the entry first, make the alterations and add an updated version afterwards. https://stackoverflow.com/questions/26959545/why-does-changing-the-hashcode-of-an-object-used-as-a-key-in-a-hashmap-make-a-lo But your key is `Long`, so it probably won't be affected by this issue. It doesn't matter if you change the hashCode of one of the values, only the key. – ᴇʟᴇvᴀтᴇ Mar 31 '20 at 19:16
  • Ah okay. Does it work if I remove my own hashcode implementation and you the default one from the Object class? – julienduchow Mar 31 '20 at 19:24
  • 1
    @julien-100000 [*What's the implementation of hashCode in java Object?*](https://stackoverflow.com/q/13602501/642706). The upshot: When depending on `hashCode`, such as usage as a key in a hash-based map, you should implement `equals` & `hashCode` using a stable value within the object such as an employee ID for `Employee` class, or a product ID field for a `Product` class. "Stable" means the value will not change while your app is running, or at least not change during the life of the hash-based usage such as being a key in a map. – Basil Bourque Mar 31 '20 at 19:28
  • Okay I understand. Thank you both for your help! :) – julienduchow Mar 31 '20 at 19:33