CODEWITHSUNDEEP
java
Tyler Sebastian
Tyler Sebastian

Reputation: 9448

Concurrent Access HashMap

The code below runs into the issue of java.util.ConcurrentModificationException. Is there any way to prevent this or allow this?

public void saveHomes() throws IOException {
    BufferedWriter br;
    br  = new BufferedWriter(new FileWriter(homeFile));
    Map<String, Location> homesLoc;

    System.out.println(homes2.keySet());
    for (String player : homes2.keySet()) {
        homesLoc = homes2.get(player);
        for (String name : homesLoc.keySet()) {
            br.write(player + " " + homesLoc.get(name) + " " + name);
            br.newLine();
            br.flush();
        }
    }

    br.close();
}

Upvotes: 2

Views: 3270

Answers (5)

Adrian Shum
Adrian Shum

Reputation: 40036

Most of the answers here seem to have misunderstood the meaning of ConcurrentModificationException, making them incomplete or even incorrect answers.

First thing you need to understand is, ConcurrentModificationException has nothing to do with concurrent access to the collection by multiple threads. It can happen even in a single-threaded application. So, using synchronized Map implementations is NOT a correct solution for the problem.

ConcurrentModificationException happens mostly when

  1. You get an iterator from a collection
  2. The collection is structurally modified, making your iterator no longer valid
  3. If you use the iterator now, it will throw ConcurrentModificationException.

Therefore, even the Map is accessed by single thread, you may still have problem.

From your code, there is no obvious logic that modified the collections (homes2 / homesLoc). That may be caused by

  1. It is modified in another thread, which we cannot see in your code, or
  2. The Map is an implementation that, even a get() is treated as structural modified. Access-ordered LinkedHashMap is an example. (we cannot see in your code either)

There are different solutions, depending on your need:

  1. using ConcurrentHashMap, which guarantees that iterator is not throwing ConcurrentModificationException. Iteration will be base on the order at the time iterator is created
  2. if it is another thread updating the map which cause the problem, you may also consider proper synchronization control on the map when you are iterating through it
  3. if it is caused by Access-Ordered LinkedHashMap, you may simply change a bit in your logic, by iterating through yourMap.entries(), so that you don't need to use extra get() to get the value.

Upvotes: 6

Lai Xin Chu
Lai Xin Chu

Reputation: 2482

You should use an java.util.Iterator to iterate through the keyset. This will prevent a java.util.ConcurrentModificationException

Code:

public void saveHomes() throws IOException {
    BufferedWriter br;
    br  = new BufferedWriter(new FileWriter(homeFile));
    Map<String, Location> homesLoc;

    System.out.println(homes2.keySet());
    Iterator<String> players = homes2.keySet().iterator();

    while (players.hasNext()) {
        String player = players.next();
        homesLoc = homes2.get(player);

        Iterator<String> names = homesLoc.keySet().iterator();
        while (names.hasNext()) {
            String name = names.next();
            br.write(player + " " + homesLoc.get(name) + " " + name);
            br.newLine();
            br.flush();
        }
    }

    br.close();
}

Upvotes: -1

renz
renz

Reputation: 1070

"Java Collection classes are fail-fast which means that if the Collection will be changed while some thread is traversing over it using iterator, the iterator.next() will throw a ConcurrentModificationException."

You are changing homesLoc and then traversing it.

Upvotes: 1

Bohemian
Bohemian

Reputation: 424983

This is a wheel that's already been invented:

Use a ConcurrentHashMap

Using it instead of a (non-threadsafe) HashMap will make your concurrency problems go away.

Upvotes: 3

aaronman
aaronman

Reputation: 18750

I don't think you should use a hashmap for this since you are just iterating through it, but in java a hashtable is synchronized so that should get rid of the concurrent access problem.

Upvotes: -1

Related Questions