Fix possible ConcurrentModificationException

Access to the models list is now secured by a Lock

src/main/java/ctbrec/recorder/LocalRecorder.java

@@ -18,6 +18,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.NoSuchElementException;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,6 +50,7 @@ public class LocalRecorder implements Recorder {
     private volatile boolean recording = true;
     private List<File> deleteInProgress = Collections.synchronizedList(new ArrayList<>());
     private RecorderHttpClient client = new RecorderHttpClient();
+    private ReentrantLock lock = new ReentrantLock();
 
     public LocalRecorder(Config config) {
         this.config = config;
@@ -76,22 +78,32 @@ public class LocalRecorder implements Recorder {
     public void startRecording(Model model) {
         if (!models.contains(model)) {
             LOG.info("Model {} added", model);
-            models.add(model);
+            lock.lock();
-            config.getSettings().models.add(model);
+            try {
+                models.add(model);
+                config.getSettings().models.add(model);
+            } finally {
+                lock.unlock();
+            }
         }
     }
 
     @Override
     public void stopRecording(Model model) throws IOException {
-        if (models.contains(model)) {
+        lock.lock();
-            models.remove(model);
+        try {
-            config.getSettings().models.remove(model);
+            if (models.contains(model)) {
-            if (recordingProcesses.containsKey(model)) {
+                models.remove(model);
-                stopRecordingProcess(model);
+                config.getSettings().models.remove(model);
+                if (recordingProcesses.containsKey(model)) {
+                    stopRecordingProcess(model);
+                }
+                LOG.info("Model {} removed", model);
+            } else {
+                throw new NoSuchElementException("Model " + model.getName() + " ["+model.getUrl()+"] not found in list of recorded models");
             }
-            LOG.info("Model {} removed", model);
+        } finally {
-        } else {
+            lock.unlock();
-            throw new NoSuchElementException("Model " + model.getName() + " ["+model.getUrl()+"] not found in list of recorded models");
         }
     }
 
@@ -102,9 +114,14 @@ public class LocalRecorder implements Recorder {
             return;
         }
 
-        if (!models.contains(model)) {
+        lock.lock();
-            LOG.info("Model {} has been removed. Restarting of recording cancelled.", model);
+        try {
-            return;
+            if (!models.contains(model)) {
+                LOG.info("Model {} has been removed. Restarting of recording cancelled.", model);
+                return;
+            }
+        } finally {
+            lock.unlock();
         }
 
         Download download;
@@ -135,12 +152,17 @@ public class LocalRecorder implements Recorder {
 
     @Override
     public boolean isRecording(Model model) {
-        return models.contains(model);
+        lock.lock();
+        try {
+            return models.contains(model);
+        } finally {
+            lock.unlock();
+        }
     }
 
     @Override
     public List<Model> getModelsRecording() {
-        return Collections.unmodifiableList(models);
+        return Collections.unmodifiableList(new ArrayList<>(models));
     }
 
     @Override
@@ -157,16 +179,21 @@ public class LocalRecorder implements Recorder {
     }
 
     private void stopRecordingProcesses() {
-        for (Model model : models) {
+        lock.lock();
-            Download recordingProcess = recordingProcesses.get(model);
+        try {
-            if (recordingProcess != null) {
+            for (Model model : models) {
-                try {
+                Download recordingProcess = recordingProcesses.get(model);
-                    recordingProcess.stop();
+                if (recordingProcess != null) {
-                    LOG.debug("Stopped recording for {}", model);
+                    try {
-                } catch (Exception e) {
+                        recordingProcess.stop();
-                    LOG.error("Couldn't stop recording for model {}", model, e);
+                        LOG.debug("Stopped recording for {}", model);
+                    } catch (Exception e) {
+                        LOG.error("Couldn't stop recording for model {}", model, e);
+                    }
                 }
             }
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -275,19 +302,24 @@ public class LocalRecorder implements Recorder {
         public void run() {
             running = true;
             while (running) {
-                for (Model model : getModelsRecording()) {
+                lock.lock();
-                    try {
+                try {
-                        if (!recordingProcesses.containsKey(model)) {
+                    for (Model model : getModelsRecording()) {
-                            boolean isOnline = model.isOnline(IGNORE_CACHE);
+                        try {
-                            LOG.trace("Checking online state for {}: {}", model, (isOnline ? "online" : "offline"));
+                            if (!recordingProcesses.containsKey(model)) {
-                            if (isOnline) {
+                                boolean isOnline = model.isOnline(IGNORE_CACHE);
-                                LOG.info("Model {}'s room back to public. Starting recording", model);
+                                LOG.trace("Checking online state for {}: {}", model, (isOnline ? "online" : "offline"));
-                                startRecordingProcess(model);
+                                if (isOnline) {
+                                    LOG.info("Model {}'s room back to public. Starting recording", model);
+                                    startRecordingProcess(model);
+                                }
                             }
+                        } catch (Exception e) {
+                            LOG.error("Couldn't check if model {} is online", model.getName(), e);
                         }
-                    } catch (Exception e) {
-                        LOG.error("Couldn't check if model {} is online", model.getName(), e);
                     }
+                } finally {
+                    lock.unlock();
                 }
 
                 try {