Fix #1402, makes caches access in cgeomap synchronized

2 files changed, 49 insertions, 22 deletions

diff --git a/main/src/cgeo/geocaching/maps/CGeoMap.java b/main/src/cgeo/geocaching/maps/CGeoMap.java
index 7cdd935..b0c0ca1 100644
--- a/main/src/cgeo/geocaching/maps/CGeoMap.java
+++ b/main/src/cgeo/geocaching/maps/CGeoMap.java
@@ -339,7 +339,7 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
     }
 
     protected void countVisibleCaches() {
-        final ArrayList<cgCache> protectedCaches = new ArrayList<cgCache>(caches);
+        final List<cgCache> protectedCaches = caches.getAsList();
 
         int count = 0;
         if (protectedCaches.size() > 0) {
@@ -721,7 +721,8 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
                 if (live && !isLoading() && CollectionUtils.isNotEmpty(caches)) {
                     final List<String> geocodes = new ArrayList<String>();
 
-                    List<cgCache> cachesProtected = new ArrayList<cgCache>(caches);
+                    final List<cgCache> cachesProtected = caches.getAsList();
+
                     try {
                         if (cachesProtected.size() > 0) {
                             final GeoPointImpl mapCenter = mapView.getMapViewCenter();
@@ -1209,7 +1210,8 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
                     final boolean excludeMine = Settings.isExcludeMyCaches();
                     final boolean excludeDisabled = Settings.isExcludeDisabledCaches();
 
-                    final ArrayList<cgCache> tempList = new ArrayList<cgCache>(caches);
+                    final List<cgCache> tempList = caches.getAsList();
+
                     for (cgCache cache : tempList) {
                         if ((cache.isFound() && excludeMine) || (cache.isOwn() && excludeMine) || (cache.isDisabled() && excludeDisabled)) {
                             caches.remove(cache);
@@ -1307,7 +1309,7 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
                 }
 
                 // display caches
-                final List<cgCache> cachesToDisplay = new ArrayList<cgCache>(caches);
+                final List<cgCache> cachesToDisplay = caches.getAsList();
                 final List<cgWaypoint> waypointsToDisplay = new ArrayList<cgWaypoint>(waypoints);
                 final List<CachesOverlayItemImpl> itemsToDisplay = new ArrayList<CachesOverlayItemImpl>();
 
diff --git a/main/src/cgeo/geocaching/utils/LeastRecentlyUsedSet.java b/main/src/cgeo/geocaching/utils/LeastRecentlyUsedSet.java
index 72728ec..eea7a11 100644
--- a/main/src/cgeo/geocaching/utils/LeastRecentlyUsedSet.java
+++ b/main/src/cgeo/geocaching/utils/LeastRecentlyUsedSet.java
@@ -1,16 +1,22 @@
 package cgeo.geocaching.utils;
 
 import java.util.AbstractSet;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 
 /**
- * Set wrapper for the LeastRecentlyUsedMap.
- *
+ * Synchronized set wrapper for the LeastRecentlyUsedMap.
+ * 
  * This code is heavily based on the HashSet code that represent Map as a Set.
  * Unfortunately HashSet does not allow to use a custom Map as its Storage.
  * Therefore overriding removeEldestEntry() is impossible for a normal LinkedHashSet.
  *
+ * Synchronization is added to guard against concurrent modification. Iterator
+ * access has to be guarded externally or the synchronized getAsList method can be used
+ * to get a clone for iteration
+ * 
  * @author Teschi
  */
 public class LeastRecentlyUsedSet<E> extends AbstractSet<E>
@@ -33,6 +39,7 @@ public class LeastRecentlyUsedSet<E> extends AbstractSet<E>
 
     /**
      * Copy of the HashSet code if iterator()
+     * Iterator access has to be synchronized externally!
      *
      * @see HashSet
      */
@@ -42,83 +49,101 @@ public class LeastRecentlyUsedSet<E> extends AbstractSet<E>
     }
 
     /**
+     * Synchronized access to set size
      * Copy of the HashSet code if size()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public int size() {
+    public synchronized int size() {
         return map.size();
     }
 
     /**
+     * Synchronized check of set emptiness
      * Copy of the HashSet code if isEmpty()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public boolean isEmpty() {
+    public synchronized boolean isEmpty() {
         return map.isEmpty();
     }
 
     /**
+     * Synchronized check for containment
      * Copy of the HashSet code if contains()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public boolean contains(Object o) {
+    public synchronized boolean contains(Object o) {
         return map.containsKey(o);
     }
 
     /**
+     * Synchronized addition of an item
      * Copy of the HashSet code if add()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public boolean add(E e) {
+    public synchronized boolean add(E e) {
         return map.put(e, PRESENT) == null;
     }
 
     /**
+     * Synchronized removal of a contained item
      * Copy of the HashSet code if remove()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public boolean remove(Object o) {
+    public synchronized boolean remove(Object o) {
         return map.remove(o) == PRESENT;
     }
 
     /**
+     * Synchronized clearing of the set
      * Copy of the HashSet code if clear()
-     *
+     * 
      * @see HashSet
      */
     @Override
-    public void clear() {
+    public synchronized void clear() {
         map.clear();
     }
 
     /**
+     * (synchronized) Clone of the set
      * Copy of the HashSet code if clone()
-     *
+     * 
      * @see HashSet
      */
     @Override
     @SuppressWarnings("unchecked")
     public Object clone() {
         try {
-            final LeastRecentlyUsedSet<E> newSet = (LeastRecentlyUsedSet<E>) super.clone();
-            newSet.map = (LeastRecentlyUsedMap<E, Object>) map.clone();
-            return newSet;
+            synchronized (this) {
+                final LeastRecentlyUsedSet<E> newSet = (LeastRecentlyUsedSet<E>) super.clone();
+                newSet.map = (LeastRecentlyUsedMap<E, Object>) map.clone();
+                return newSet;
+            }
         } catch (CloneNotSupportedException e) {
             throw new InternalError();
         }
     }
 
     /**
+     * Creates a clone as a list in a synchronized fashion.
+     *
+     * @return List based clone of the set
+     */
+    public synchronized List<E> getAsList() {
+        return new ArrayList<E>(this);
+    }
+
+    /**
      * Serialization version of HashSet with the additional parameters for the custom Map
      *
      * @see HashSet