Fixed #1991 ConcurrentModificationException on clearing cache thread

Korne127 · Apr 5, 2023 · 8aa3c18 · 8aa3c18
1 parent 49013ed
commit 8aa3c18
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
 - [#1994] Replace command in commandline with three argument causing replacements file load
 - [#1477] Open file (Context menu) with unicode characters, unicode in paths, on Windows
 - Starting app with parameters causing wrong GUI init
+- [#1991] ConcurrentModificationException on clearing cache thread
 
 ## [18.4.0] - 2023-03-19
 ### Added
@@ -3019,6 +3020,7 @@ All notable changes to this project will be documented in this file.
 [#1993]: https://www.free-decompiler.com/flash/issues/1993
 [#1994]: https://www.free-decompiler.com/flash/issues/1994
 [#1477]: https://www.free-decompiler.com/flash/issues/1477
+[#1991]: https://www.free-decompiler.com/flash/issues/1991
 [#1888]: https://www.free-decompiler.com/flash/issues/1888
 [#1892]: https://www.free-decompiler.com/flash/issues/1892
 [#355]: https://www.free-decompiler.com/flash/issues/355

diff --git a/libsrc/ffdec_lib/src/com/jpexs/helpers/Cache.java b/libsrc/ffdec_lib/src/com/jpexs/helpers/Cache.java
@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
-import java.util.ConcurrentModificationException;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -43,6 +42,8 @@ public class Cache<K, V> implements Freed {
     private Map<K, V> cache;
     private Map<K, Long> lastAccessed;
 
+    private static final Object instancesLock = new Object();
+
     private static final List<WeakReference<Cache>> instances = new ArrayList<>();
 
     public static final int STORAGE_FILES = 1;
@@ -54,27 +55,28 @@ public class Cache<K, V> implements Freed {
     private final boolean memoryOnly;
 
     private final String name;
-    
+
     private final boolean temporary;
-            
+
     private static final long CLEAN_INTERVAL = 5 * 1000; //5 seconds
-    
-    private static Thread oldCleaner = null;  
+
+    private static Thread oldCleaner = null;
 
     static {
         Runtime.getRuntime().addShutdownHook(new Thread() {
 
             @Override
             public void run() {
-                for (WeakReference<Cache> cw : instances) {
-                    Cache c = cw.get();
-                    if (c != null) {
-                        c.clear();
-                        c.free();
+                synchronized (instancesLock) {
+                    for (WeakReference<Cache> cw : instances) {
+                        Cache c = cw.get();
+                        if (c != null) {
+                            c.clear();
+                            c.free();
+                        }
                     }
                 }
             }
-
         });
     }
 
@@ -83,37 +85,41 @@ public static <K, V> Cache<K, V> getInstance(boolean weak, boolean memoryOnly, S
             oldCleaner = new Thread("Old cache cleaner") {
                 @Override
                 public void run() {
-                    while(!Thread.interrupted()) {
+                    while (!Thread.interrupted()) {
                         try {
                             Thread.sleep(CLEAN_INTERVAL);
                         } catch (InterruptedException ex) {
                             return;
                         }
                         try {
-                            clearAllOld();                        
+                            clearAllOld();
                         } catch (Exception cme) {
                             Logger.getLogger(Cache.class.getSimpleName()).log(Level.SEVERE, "Error during clearing cache thread", cme);
                         }
                     }
-                }                
+                }
             };
             oldCleaner.setDaemon(true);
             oldCleaner.setPriority(Thread.MIN_PRIORITY);
             oldCleaner.start();
         }
         Cache<K, V> instance = new Cache<>(weak, memoryOnly, name, temporary);
-        instances.add(new WeakReference<>(instance));
+        synchronized (instancesLock) {
+            instances.add(new WeakReference<>(instance));
+        }
         return instance;
     }
 
     private static int storageType = STORAGE_FILES;
 
     public static void clearAll() {
-        for (WeakReference<Cache> cw : instances) {
-            Cache c = cw.get();
-            if (c != null) {
-                c.clear();
-                c.initCache();
+        synchronized (instancesLock) {
+            for (WeakReference<Cache> cw : instances) {
+                Cache c = cw.get();
+                if (c != null) {
+                    c.clear();
+                    c.initCache();
+                }
             }
         }
     }
@@ -174,7 +180,7 @@ private Cache(boolean weak, boolean memoryOnly, String name, boolean temporary)
         initCache();
     }
 
-    public synchronized boolean contains(K key) {        
+    public synchronized boolean contains(K key) {
         boolean ret = cache.containsKey(key);
         if (ret) {
             lastAccessed.put(key, System.currentTimeMillis());
@@ -208,7 +214,7 @@ public synchronized void put(K key, V value) {
 
     @Override
     public boolean isFreeing() {
-        throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.
+        throw new UnsupportedOperationException("Not supported yet.");
     }
 
     @Override
@@ -223,7 +229,7 @@ public Set<K> keys() {
         ret.addAll(cache.keySet());
         return ret;
     }
-    
+
     private synchronized int clearOld() {
         long currentTime = System.currentTimeMillis();
         Set<K> keys = new HashSet<>(lastAccessed.keySet());
@@ -232,26 +238,28 @@ private synchronized int clearOld() {
             return 0;
         }
         int num = 0;
-        for(K key:keys) {
+        for (K key : keys) {
             long time = lastAccessed.get(key);
             if (time < currentTime - temporaryThreshold) {
                 remove(key);
                 num++;
             }
-        }        
+        }
         return num;
     }
-    
+
     private static void clearAllOld() {
         int num = 0;
-        for (WeakReference<Cache> cw : instances) {
-            Cache c = cw.get();
-            if (c != null) {
-                if (c.temporary) {
-                    num += c.clearOld();
+        synchronized (instancesLock) {
+            for (WeakReference<Cache> cw : instances) {
+                Cache c = cw.get();
+                if (c != null) {
+                    if (c.temporary) {
+                        num += c.clearOld();
+                    }
                 }
             }
-        }        
+        }
         if (num > 0) {
             System.gc();
         }