java.util.ConcurrentModificationException on moving jMapViewer

Question

I'm trying to update markers on a jMapViewer once every 5 seconds. This seems to be working fine until you move the map. At this point it throws a java.util.ConcurrentModificationException.

I believe this is to do with the different processes trying to access the map markers list at the same time but I'm not sure how to fix it.

   timer.scheduleAtFixedRate(new TimerTask() {
        @Override
        public void run() {
            loadUnits();
        }
    }, 5 * 1000, 5 * 1000);

   private void loadUnits() {      
    String query = "SELECT callsign, currentlat,currentlon,previouslat,previouslon,    mobile, uniticon FROM unit WHERE isdeleted=0;";
    rs = DBase.runQuery(query);
    kit.removeAllMapMarkers();
    MapMarkerUnit x;
    try {
        while (rs.next()) {
           x = new MapMarkerUnit(rs.getDouble("currentlat"),rs.getDouble("currentlon"));
           if (rs.getInt("mobile") == 1) x.setMovement(true);
           else x.setMovement(false);
           x.setIconName(rs.getString("uniticon"));
           x.setPriority(1);
           kit.addMapMarker(x);
        }
    }
    catch (SQLException e) {
        System.out.print(e.toString());
    }
}

Thanks for your help.

Kieran

RS is a ResultSet pulled from a MySQL database. Sorry MWE? A little new to this. — KieranS, Sep 02 '14 at 14:46
Minimum Working Example, in other words code one can compile and experiment with... — Willem Van Onsem, Sep 02 '14 at 14:46
@CommuSoft Thanks - have updated my question with more of the code. I'm looking at your code below - hopefully between that and google I'll be able to understand it. — KieranS, Sep 02 '14 at 14:56
[`SwingWorker`](http://docs.oracle.com/javase/tutorial/uiswing/concurrency/worker.html) may be an alternative. — trashgod, Sep 11 '14 at 15:02

Willem Van Onsem · Accepted Answer · 2014-09-02T14:53:32.873

You can do this using a Semaphore, Mutex, a monitor (with synchronized in the method signature) or a lock (synchronize on object). There exist lock-free and wait-free approaches as well, but these algorithms are more complex and are only useful under special circumstances.

Examples

The problem is probably that map is modified concurrently, using a lock, one can write:

synchronize(map) {
    map.removeAllMapMarkers();
    MapMarkerUnit x;
    try {
        while (rs.next()) {
           x = new MapMarkerUnit(rs.getDouble("currentlat"),rs.getDouble("currentlon"));
           if (rs.getInt("mobile") == 1) x.setMovement(true);
           else x.setMovement(false);
           x.setIconName(rs.getString("uniticon"));
           x.setPriority(1);
           map.addMapMarker(x);
        }
    }
    catch (SQLException e) {
        System.out.print(e.toString());
    }
}

This results in the fact that only one Thread can access map (if it runs this code). If one thread is in the synchronize block, all other threads wait at the beginning of the block.

A problem with this approach is the so called Readers-Writers problem. Most datastructures allow to be read by multiple Readers, but if some Thread want to write something (modify something as well), no Reader can be active. In that case one uses a ReadWriteLock:

private ReadWriteLock rwl = new ReentrantReadWriteLock();

public void writeSomething() {
   rwl.writeLock().lock();
   try {
      //Modify/write something
   } finally {
      rwl.writeLock().unlock();
   }
}

public String readSomething() {
   rwl.readLock().lock();
   try {
      //Read something
   } finally {
      rwl.readLock().unlock();
   }
}

You better use finally blocks so that even if an Exception is thrown you still unlock the lock, otherwise no other object will be able to enter the critical section.

Looking into this problem further. map is an object of class jMapViewer. This stores a list of map markers in a List. It then iterates through this to redraw the map markers. The problem appears to be that the map.removeAllMarkers() removes them while this is happening. — KieranS, Sep 02 '14 at 18:55
Your example above makes sense but I'm not sure how to implement it. synchronize(map) doesn't appear to work as the iterator is inside map already. — KieranS, Sep 02 '14 at 18:56
@KieranS: in that case you should lock the `map` before retrieving the list of markers and only unlock (end the scope) if no action will be performed on that list anymore. — Willem Van Onsem, Sep 02 '14 at 19:01
That worked - performing the operations on a separate list. Thanks for all your help and time! — KieranS, Sep 02 '14 at 19:45
@KieranS: You should also check if the action is eventually repeated every time. Locks can cause *deadlocks*: where two or more threads wait for each other to perform an action. — Willem Van Onsem, Sep 02 '14 at 20:31

java.util.ConcurrentModificationException on moving jMapViewer

1 Answers1