CODEWITHSUNDEEP
javaandroiddebuggingconcurrency
The Beruriah Incident
The Beruriah Incident

Reputation: 3257

Resolving ConcurrentModificationException from modifying overlays

I've been trying to fix an error causing an intermittent ConcurrentModificationException. What's happening is that I have tons of geopoints being displayed with an ItemizedOverlay. When the map is moved, however, I'm trying to clear out all current overlay items (the geopoints) and replace them with new points appropriate to the new view window.

I therefore have a callback function that clears out the old overlays and replaces them with new ones. I think my bug stems from multiple threads trying to do this simultaneously. The relevant sections are below. I have a very limited understanding of how the overlays and such work on a low level, so I was wondering if anyone could confirm (or refute) that this could be causing issues.

//first clear out the old overlays
List<Overlay> mapOverlays = mapView.getOverlays();
mapOverlays.clear();

//create the new overlays, each initialized with appropriate Drawables
MenuOverlay lowOverlay = new MenuOverlay(this, lowRisk);//(all valid Drawables)
MenuOverlay medOverlay = new MenuOverlay(this, medRisk);
MenuOverlay highOverlay = new MenuOverlay(this, highRisk);

//populate the overlays

//add the new overlays into the list of overlays
mapOverlays.add(lowOverlay);
mapOverlays.add(medOverlay);
mapOverlays.add(highOverlay);

//make the map refresh; this operation has to be pushed to another thread
Runnable runnable = new Runnable() {
    public void run() {
        mapView.invalidate();
    }
};
runOnUiThread(runnable);

I tried making this method synchronized, but the error still occurred. Could this arise from the new runnable being pushed to the UI thread before the previous runnable terminates maybe? I've seen mention that populate is a better way than invalidate, although I'm not entirely sure how they're different. Any ideas how to resolve this?

Upvotes: 1

Views: 157

Answers (1)

acj
acj

Reputation: 4821

Modifying the set of overlays should always be done on the UI thread. The List that getOverlays() returns is owned by the MapView, and it can decide to view or manipulate the list at any time.

Since you're working with a lot of geopoints, it's likely that your background thread is clearing (or adding) overlays while the MapView is iterating over them on the UI thread. That would trigger a ConcurrentModificationException because the iterator is invalidated when its underlying set of overlays changes. Sometimes the changes are not immediately visible to the UI thread, so the crash is intermittent.

Setting up the overlays is usually the slow part of this type of workflow. To avoid the concurrent modification, you could set up your overlays in the background thread and then make all of your calls to clear() and add() inside of the Runnable. (Another option is to use an AsyncTask.)

For example:

// Slow setup steps
final MenuOverlay lowOverlay = new MenuOverlay(this, lowRisk);
final MenuOverlay medOverlay = new MenuOverlay(this, medRisk);
final MenuOverlay highOverlay = new MenuOverlay(this, highRisk);

Runnable runnable = new Runnable() {
    public void run() {
        // Anything that touches UI widgets (map, overlay set, views, etc.)
        List<Overlay> mapOverlays = mapView.getOverlays();
        mapOverlays.clear();

        mapOverlays.add(lowOverlay);
        mapOverlays.add(medOverlay);
        mapOverlays.add(highOverlay);

        mapView.invalidate();
    }
};

runOnUiThread(runnable);

Upvotes: 1

Related Questions