Merge pull request #9 from codeforkjeff/fix-cme

prevent ConcurrentModificationExceptions under high load in cache exp…
codeforkjeff · Oct 13, 2016 · f02b38a · f02b38a
2 parents 26e5759 + cfac7be
commit f02b38a
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 5 deletions.
diff --git a/src/main/java/com/codefork/refine/Cache.java b/src/main/java/com/codefork/refine/Cache.java
@@ -5,7 +5,7 @@
 
 import java.util.Arrays;
 import java.util.Comparator;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Simple cache implementation. This does NOT do any locking. Clients should
@@ -22,7 +22,7 @@ public class Cache<K, V> {
 
     private int lifetime = DEFAULT_LIFETIME; // in seconds
     private int maxSize = DEFAULT_MAXSIZE;
-    private HashMap<K, CachedValue> cacheMap = new HashMap<K, CachedValue>();
+    private ConcurrentHashMap<K, CachedValue> cacheMap = new ConcurrentHashMap<K, CachedValue>();
 
     public int getLifetime() {
         return lifetime;
@@ -82,7 +82,7 @@ public Cache<K, V> expireCache() {
 
         // processing keys in descending timestamp order makes
         // it easier to break out of the copy loop when we hit maxSize
-        final HashMap<K, CachedValue> oldMap = getMap();
+        final ConcurrentHashMap<K, CachedValue> oldMap = getMap();
         K[] sorted = (K[]) oldMap.keySet().toArray();
         Arrays.sort(sorted, new ReverseTimestampComparator());
         int total = oldMap.size();
@@ -110,7 +110,7 @@ public Cache<K, V> expireCache() {
         return newCache;
     }
 
-    private HashMap<K, CachedValue> getMap() {
+    private ConcurrentHashMap<K, CachedValue> getMap() {
         return cacheMap;
     }
 

diff --git a/src/main/java/com/codefork/refine/CacheExpire.java b/src/main/java/com/codefork/refine/CacheExpire.java
@@ -19,7 +19,13 @@ public void run() {
         try {
             while(keepGoing) {
                 Thread.sleep(60000);
-                cacheManager.expireCache();
+                try {
+                    cacheManager.expireCache();
+                } catch(Exception e) {
+                    // if we don't catch here, a possibly intermittent or edge-case error
+                    // causes the cache expire thread to die, and the cache will grow uncontrollably
+                    log.error("Ignoring error that occurred in cacheManager.expireCache(): " + e.toString());
+                }
             }
         } catch (InterruptedException e) {
             // noop