more safe configSave

4 files changed, 148 insertions, 83 deletions

diff --git a/source/net/yacy/http/servlets/YaCyDefaultServlet.java b/source/net/yacy/http/servlets/YaCyDefaultServlet.java
index afd2e577f..46f1675ab 100644
--- a/source/net/yacy/http/servlets/YaCyDefaultServlet.java
+++ b/source/net/yacy/http/servlets/YaCyDefaultServlet.java
@@ -757,9 +757,14 @@ public class YaCyDefaultServlet extends HttpServlet  {
         }

     }

 

-    protected Object invokeServlet(final Method targetMethod, final RequestHeader request, final serverObjects args) throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {

-        return targetMethod.invoke(null, request, args, Switchboard.getSwitchboard()); // add switchboard

-    }

+    protected Object invokeServlet(final Method targetMethod, final RequestHeader request, final serverObjects args) throws IllegalArgumentException, IllegalAccessException, InvocationTargetException {
+        final serverSwitch.SaveConfigOrigin previousOrigin = serverSwitch.pushSaveConfigOriginUI();
+        try {
+            return targetMethod.invoke(null, request, args, Switchboard.getSwitchboard()); // add switchboard
+        } finally {
+            serverSwitch.popSaveConfigOrigin(previousOrigin);
+        }
+    }
 

     /**

      * Returns the URL base for this peer, determined from request HTTP header "Host" when present. Use this when absolute URL rendering is required,

diff --git a/source/net/yacy/kelondro/util/FileUtils.java b/source/net/yacy/kelondro/util/FileUtils.java
index 577e3c036..4487c305b 100644
--- a/source/net/yacy/kelondro/util/FileUtils.java
+++ b/source/net/yacy/kelondro/util/FileUtils.java
@@ -42,18 +42,21 @@ import java.io.OutputStream;
 import java.io.PrintWriter;

 import java.io.Reader;

 import java.io.UnsupportedEncodingException;

-import java.io.Writer;

-import java.nio.charset.Charset;

-import java.nio.charset.StandardCharsets;

-import java.util.ArrayList;

-import java.util.Collections;

-import java.util.HashMap;

-import java.util.HashSet;

-import java.util.Iterator;

-import java.util.LinkedList;

-import java.util.List;

-import java.util.Map;

-import java.util.concurrent.ConcurrentHashMap;

+import java.io.Writer;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.zip.GZIPInputStream;

 

 import org.apache.commons.lang3.StringUtils;

@@ -61,12 +64,16 @@ import org.mozilla.intl.chardet.nsDetector;
 import org.mozilla.intl.chardet.nsPSMDetector;

 

 import net.yacy.cora.document.encoding.UTF8;

-import net.yacy.cora.storage.Files;

 import net.yacy.cora.util.ConcurrentLog;

 

-public final class FileUtils {

-

-    private static final int DEFAULT_BUFFER_SIZE = 1024; // this is also the maximum chunk size

+public final class FileUtils {
+
+    private static final int DEFAULT_BUFFER_SIZE = 1024; // this is also the maximum chunk size
+    /**
+     * Serialize save operations per destination file to avoid concurrent writes
+     * stepping on each other.
+     */
+    private static final ConcurrentHashMap<String, Object> SAVE_MAP_LOCKS = new ConcurrentHashMap<>();
 

     /**

      * Copy a whole InputStream to an OutputStream. Important : it is the responsibility of the caller to close the input and output streams.

@@ -527,43 +534,58 @@ public final class FileUtils {
     private final static String[] escaped_strings_in = {"\\\\", "\\n", "\\="};

     private final static String[] unescaped_strings_out = {"\\", "\n", "="};

 

-    public static void saveMap(final File file, final Map<String, String> props, final String comment) {

-        boolean err = false;

-        PrintWriter pw = null;

-        final File tf = new File(file.toString() + "." + (System.currentTimeMillis() % 1000));

-        try {

-            pw = new PrintWriter(tf, StandardCharsets.UTF_8.name());

-            pw.println("# " + comment);

-            String key, value;

-            for ( final Map.Entry<String, String> entry : props.entrySet() ) {

-                key = entry.getKey();

-                if ( key != null ) {

-                    key = StringUtils.replaceEach(key, unescaped_strings_in, escaped_strings_out);

-                }

-                if ( entry.getValue() == null ) {

-                    value = "";

-                } else {

-                    value = entry.getValue();

-                    value = StringUtils.replaceEach(value, unescaped_strings_in, escaped_strings_out);

-                }

-                pw.println(key + "=" + value);

-            }

-            pw.println("# EOF");

-        } catch (final  FileNotFoundException | UnsupportedEncodingException e ) {

-            ConcurrentLog.warn("FileUtils", e.getMessage(), e);

-            err = true;

-        } finally {

-            if ( pw != null ) {

-                pw.close();

-            }

-            pw = null;

-        }

-        if (!err) try {

-            forceMove(tf, file);

-        } catch (final  IOException e ) {

-            // ignore

-        }

-    }

+    public static void saveMap(final File file, final Map<String, String> props, final String comment) {
+        final String lockKey = file.getAbsolutePath();
+        final Object lock = SAVE_MAP_LOCKS.computeIfAbsent(lockKey, k -> new Object());
+        synchronized (lock) {
+            File tf = null;
+            PrintWriter pw = null;
+            try {
+                final File parent = file.getAbsoluteFile().getParentFile();
+                if (parent != null && !parent.exists() && !parent.mkdirs()) {
+                    ConcurrentLog.warn("FileUtils", "Could not create parent directory for " + file);
+                    return;
+                }
+                final String baseName = file.getName();
+                final String prefix = (baseName.length() < 3) ? (baseName + "___").substring(0, 3) : baseName;
+                tf = File.createTempFile(prefix, ".tmp", parent);
+
+                pw = new PrintWriter(tf, StandardCharsets.UTF_8.name());
+                pw.println("# " + comment);
+                String key, value;
+                for ( final Map.Entry<String, String> entry : props.entrySet() ) {
+                    key = entry.getKey();
+                    if ( key != null ) {
+                        key = StringUtils.replaceEach(key, unescaped_strings_in, escaped_strings_out);
+                    }
+                    if ( entry.getValue() == null ) {
+                        value = "";
+                    } else {
+                        value = entry.getValue();
+                        value = StringUtils.replaceEach(value, unescaped_strings_in, escaped_strings_out);
+                    }
+                    pw.println(key + "=" + value);
+                }
+                pw.println("# EOF");
+                pw.flush();
+                pw.close();
+                pw = null;
+
+                forceMove(tf, file);
+            } catch (final FileNotFoundException | UnsupportedEncodingException e ) {
+                ConcurrentLog.warn("FileUtils", "Could not write map to temporary file for " + file + ": " + e.getMessage(), e);
+            } catch (final IOException e) {
+                ConcurrentLog.severe("FileUtils", "Could not persist map to " + file + ": " + e.getMessage(), e);
+            } finally {
+                if (pw != null) {
+                    pw.close();
+                }
+                if (tf != null && tf.exists()) {
+                    FileUtils.deletedelete(tf);
+                }
+            }
+        }
+    }
 

     public static void saveMapB(final File file, final Map<String, byte[]> props, final String comment) {

         final HashMap<String, String> m = new HashMap<>();

@@ -889,13 +911,15 @@ public final class FileUtils {
      * @param to

      * @throws IOException

      */

-    private static void forceMove(final File from, final File to) throws IOException {

-        if ( !(to.delete() && from.renameTo(to)) ) {

-            // do it manually

-            Files.copy(from, to);

-            FileUtils.deletedelete(from);

-        }

-    }

+    private static void forceMove(final File from, final File to) throws IOException {
+        final Path fromPath = from.toPath();
+        final Path toPath = to.toPath();
+        try {
+            java.nio.file.Files.move(fromPath, toPath, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
+        } catch (final AtomicMoveNotSupportedException e) {
+            java.nio.file.Files.move(fromPath, toPath, StandardCopyOption.REPLACE_EXISTING);
+        }
+    }
 

     /**

      * Creates a temp file in the default system tmp directory (System property ""java.io.tmpdir"")

diff --git a/source/net/yacy/search/query/SearchEvent.java b/source/net/yacy/search/query/SearchEvent.java
index 31bf06c05..6f5e87f01 100644
--- a/source/net/yacy/search/query/SearchEvent.java
+++ b/source/net/yacy/search/query/SearchEvent.java
@@ -951,7 +951,9 @@ public final class SearchEvent implements ScoreMapUpdatesListener {
             }
         }
          */
-        this.snippets.putAll(solrsnippets);
+        if (solrsnippets != null && !solrsnippets.isEmpty()) {
+            this.snippets.putAll(solrsnippets);
+        }
         assert (nodeList != null);
         if (nodeList.isEmpty()) return;
 
diff --git a/source/net/yacy/server/serverSwitch.java b/source/net/yacy/server/serverSwitch.java
index edeaf7e67..f2eea88b0 100644
--- a/source/net/yacy/server/serverSwitch.java
+++ b/source/net/yacy/server/serverSwitch.java
@@ -56,9 +56,13 @@ import net.yacy.kelondro.workflow.BusyThread;
 import net.yacy.peers.Seed;

 import net.yacy.search.SwitchboardConstants;

 

-public class serverSwitch {

-

-    // configuration management

+public class serverSwitch {
+    public enum SaveConfigOrigin {
+        UI,
+        BOT
+    }
+
+    // configuration management
     private final File configFile;

     private final String configComment;

     public final File dataPath;

@@ -70,8 +74,9 @@ public class serverSwitch {
     private final ConcurrentMap<String, String> configRemoved;

     private final NavigableMap<String, BusyThread> workerThreads;

     private YaCyHttpServer httpserver; // implemented HttpServer

-    private final ConcurrentMap<String, Integer> upnpPortMap = new ConcurrentHashMap<>();

-    private boolean isConnectedViaUpnp;

+    private final ConcurrentMap<String, Integer> upnpPortMap = new ConcurrentHashMap<>();
+    private boolean isConnectedViaUpnp;
+    private static final ThreadLocal<SaveConfigOrigin> saveOriginContext = ThreadLocal.withInitial(() -> SaveConfigOrigin.BOT);
 

     public serverSwitch(final File dataPath, final File appPath, final String initPath, final String configPath) {

         // we initialize the switchboard with a property file,

@@ -147,9 +152,9 @@ public class serverSwitch {
             }

         }

 

-        // save result; this may initially create a config file after

-        // initialization

-        this.saveConfig();

+        // save result; this may initially create a config file after
+        // initialization
+        this.saveConfigBot();
 

         // init thread control

         this.workerThreads = new TreeMap<>();

@@ -294,13 +299,17 @@ public class serverSwitch {
         this.setConfig(key, Double.toString(value));

     }

 

-    public void setConfig(final String key, final String value) {

-        // set the value

-        final String oldValue = this.configProps.put(key, value);

-        if (oldValue == null || !value.equals(oldValue)) {

-            this.saveConfig();

-        }

-    }

+    public void setConfig(final String key, final String value) {
+        // set the value
+        final String oldValue = this.configProps.put(key, value);
+        if (oldValue == null || !value.equals(oldValue)) {
+            if (saveOriginContext.get() == SaveConfigOrigin.UI) {
+                this.saveConfigUI();
+            } else {
+                this.saveConfigBot();
+            }
+        }
+    }
 

     public void setConfig(final String key, final String[] value) {

         final StringBuilder sb = new StringBuilder();

@@ -496,10 +505,35 @@ public class serverSwitch {
     /**

      * write the changes to permanent storage (File)

      */

-    private void saveConfig() {

-        final ConcurrentMap<String, String> configPropsCopy = new ConcurrentHashMap<>(this.configProps);

-        FileUtils.saveMap(this.configFile, configPropsCopy, this.configComment);

-    }

+    public void saveConfigUI() {
+        this.saveConfig(SaveConfigOrigin.UI);
+    }
+
+    public void saveConfigBot() {
+        this.saveConfig(SaveConfigOrigin.BOT);
+    }
+
+    private void saveConfig(final SaveConfigOrigin origin) {
+        final ConcurrentMap<String, String> configPropsCopy = new ConcurrentHashMap<>(this.configProps);
+        FileUtils.saveMap(this.configFile, configPropsCopy, this.configComment);
+        if (this.log != null && this.log.isFine()) {
+            this.log.fine("Saved config to " + this.configFile + " (origin=" + origin + ")");
+        }
+    }
+
+    public static SaveConfigOrigin pushSaveConfigOriginUI() {
+        final SaveConfigOrigin previous = saveOriginContext.get();
+        saveOriginContext.set(SaveConfigOrigin.UI);
+        return previous;
+    }
+
+    public static void popSaveConfigOrigin(final SaveConfigOrigin previous) {
+        if (previous == null) {
+            saveOriginContext.remove();
+        } else {
+            saveOriginContext.set(previous);
+        }
+    }
 

     /**

      * Gets configuration parameters which have been removed during