From 9cd693a464b19f8788ba4438d68367b1aed4014b Mon Sep 17 00:00:00 2001 From: Michael Peter Christen Date: Sun, 8 Feb 2026 15:09:05 +0100 Subject: more safe configSave --- .../net/yacy/http/servlets/YaCyDefaultServlet.java | 11 +- source/net/yacy/kelondro/util/FileUtils.java | 144 ++++++++++++--------- source/net/yacy/search/query/SearchEvent.java | 4 +- source/net/yacy/server/serverSwitch.java | 72 ++++++++--- 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 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 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 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 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 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 props, final String comment) { final HashMap 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 configRemoved; private final NavigableMap workerThreads; private YaCyHttpServer httpserver; // implemented HttpServer - private final ConcurrentMap upnpPortMap = new ConcurrentHashMap<>(); - private boolean isConnectedViaUpnp; + private final ConcurrentMap upnpPortMap = new ConcurrentHashMap<>(); + private boolean isConnectedViaUpnp; + private static final ThreadLocal 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 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 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 -- cgit v1.2.3