Use a scheduled thread pool in JsonStorage + Bugfixes (#3874)

* Use a scheduled thread pool in JsonStorage to avoid one thread per instance
* Removed an incorrect conversion between millis and nanos

Signed-off-by: Jörg Sautter <[email protected]>

bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java

@@ -23,13 +23,15 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.Timer;
-import java.util.TimerTask;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.common.ThreadPoolManager;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.config.core.ConfigurationDeserializer;
 import org.openhab.core.config.core.OrderingMapSerializer;
@@ -60,6 +62,7 @@
  *         de-/serialization, keep json structures in map
  * @author Sami Salonen - ordered inner and outer serialization of Maps,
  *         Sets and properties of Configuration
+ * @author Jörg Sautter - use a scheduled thread pool
  */
 @NonNullByDefault
 public class JsonStorage<T> implements Storage<T> {
@@ -75,8 +78,8 @@ public class JsonStorage<T> implements Storage<T> {
     private static final String BACKUP_EXTENSION = "backup";
     private static final String SEPARATOR = "--";
 
-    private final Timer commitTimer;
-    private @Nullable TimerTask commitTimerTask;
+    private final ScheduledExecutorService scheduledExecutorService;
+    private @Nullable ScheduledFuture<?> commitScheduledFuture;
 
     private long deferredSince = 0;
 
@@ -112,7 +115,7 @@ public JsonStorage(File file, @Nullable ClassLoader classLoader, int maxBackupFi
                 .setPrettyPrinting() //
                 .create();
 
-        commitTimer = new Timer();
+        scheduledExecutorService = ThreadPoolManager.getScheduledPool("JsonStorage");
 
         Map<String, StorageEntry> inputMap = null;
         if (file.exists()) {
@@ -333,11 +336,11 @@ private void writeDatabaseFile(File dataFile, String data) throws IOException {
      * require a read and write, and is thus slower).
      */
     public synchronized void flush() {
-        // Stop any existing timer
-        TimerTask commitTimerTask = this.commitTimerTask;
-        if (commitTimerTask != null) {
-            commitTimerTask.cancel();
-            this.commitTimerTask = null;
+        // Stop any existing scheduled commit
+        ScheduledFuture<?> commitScheduledFuture = this.commitScheduledFuture;
+        if (commitScheduledFuture != null) {
+            commitScheduledFuture.cancel(false);
+            this.commitScheduledFuture = null;
         }
 
         if (dirty) {
@@ -376,41 +379,29 @@ private void cleanupBackups() {
         }
     }
 
-    private class CommitTimerTask extends TimerTask {
-        @Override
-        public void run() {
-            // Save the database
-            flush();
-        }
-    }
-
     public synchronized void deferredCommit() {
         dirty = true;
 
-        // Stop any existing timer
-        TimerTask commitTimerTask = this.commitTimerTask;
-        if (commitTimerTask != null) {
-            commitTimerTask.cancel();
-            this.commitTimerTask = null;
+        // Stop any existing scheduled commit
+        ScheduledFuture<?> commitScheduledFuture = this.commitScheduledFuture;
+        if (commitScheduledFuture != null) {
+            commitScheduledFuture.cancel(false);
+            this.commitScheduledFuture = null;
         }
 
         // Handle a maximum time for deferring the commit.
         // This stops a pathological loop preventing saving
-        if (deferredSince != 0 && deferredSince < System.nanoTime() - (maxDeferredPeriod * 1000L)) {
+        if (deferredSince != 0 && deferredSince < System.currentTimeMillis() - maxDeferredPeriod) {
             flush();
-            // as we committed the database now, there is no need to start a new commit
-            // timer
+            // as we committed the database now, there is no need to schedule a new commit
             return;
         }
 
         if (deferredSince == 0) {
-            deferredSince = System.nanoTime();
+            deferredSince = System.currentTimeMillis();
         }
 
-        // Create the timer task
-        commitTimerTask = new CommitTimerTask();
-
-        // Start the timer
-        commitTimer.schedule(commitTimerTask, writeDelay);
+        // Schedule the commit
+        this.commitScheduledFuture = scheduledExecutorService.schedule(this::flush, writeDelay, TimeUnit.MILLISECONDS);
     }
 }