Add a CLI regression test for a project with empty settings

Runs --info over a copy of a shipped model whose Metadata/project_settings.config
has been rewritten to an empty object, so the test keeps covering the crash no
matter what settings the shipped models carry later.

Verified both ways: the test passes against this branch and fails with a
segmentation fault against a build without the guards.

.github/workflows/ofl-ota-cronjob.yml

@@ -146,7 +146,7 @@ jobs:
                 "repos/${{ github.repository }}/actions/workflows/post_merge_profiles.yml/runs" \
                 -f branch="$branch" -f event=workflow_dispatch -f per_page=100)"
               run_id="$(jq -r --arg marker "[OFL cron $dispatch_id]" \
-                '[.workflow_runs[] | select((.display_title // "") | contains($marker))] \
+                '[.workflow_runs[] | select((.display_title // "") | contains($marker))]
                 | sort_by(.created_at) | last | .id // empty' <<< "$runs_json")"
               [ -n "$run_id" ] && break
               sleep 5

src/OrcaSlicer.cpp

@@ -1913,6 +1913,9 @@ int CLI::run(int argc, char **argv)
                         old_printable_width = static_cast<int>(old_printable_bbox.size().x());
                         old_printable_depth = static_cast<int>(old_printable_bbox.size().y());
                     }
+                    // A 3mf can carry an empty project_settings.config - the models in
+                    // resources/handy_models do - and opt_float() dereferences without checking.
+                    if (config.option<ConfigOptionFloat>("printable_height"))
                         old_printable_height = (int)(config.opt_float("printable_height"));
 
                     if (config.option<ConfigOptionFloat>("extruder_clearance_height_to_rod"))
@@ -2589,6 +2592,7 @@ int CLI::run(int argc, char **argv)
                             orig_printable_width = static_cast<int>(orig_printable_bbox.size().x());
                             orig_printable_depth = static_cast<int>(orig_printable_bbox.size().y());
                         }
+                        if (config.option<ConfigOptionFloat>("printable_height"))
                             orig_printable_height = (int)(config.opt_float("printable_height"));
                         BOOST_LOG_TRIVIAL(info) << __FUNCTION__<< boost::format(":%1%, check printable size: old_printable_width=%2%, orig_printable_width=%3%, old_printable_depth=%4%, orig_printable_depth=%5%, old_printable_height=%6%, orig_printable_height=%7%")
                                     %__LINE__ %old_printable_width %orig_printable_width %old_printable_depth %orig_printable_depth %old_printable_height %orig_printable_height;
@@ -4617,6 +4621,7 @@ int CLI::run(int argc, char **argv)
                 BoundingBoxf temp_printable_bbox(temp_printable_area);
                 printer_plate.printable_width = static_cast<int>(temp_printable_bbox.size().x());
                 printer_plate.printable_depth = static_cast<int>(temp_printable_bbox.size().y());
+                if (config.option<ConfigOptionFloat>("printable_height"))
                     printer_plate.printable_height = (int)(config.opt_float("printable_height"));
             }
             if (temp_exclude_area.size() >= 4) {

src/libslic3r/AppConfig.cpp

@@ -5,7 +5,6 @@
 //BBS
 #include "Preset.hpp"
 #include "Exception.hpp"
-#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Thread.hpp"
 #include "format.hpp"
@@ -744,13 +743,10 @@ static bool verify_config_file_checksum(boost::nowide::ifstream &ifs)
 
 
 #ifdef USE_JSON_CONFIG
-std::string AppConfig::load(bool read_only)
+std::string AppConfig::load()
 {
     json j;
 
-    // Keep another instance from replacing or restoring the file mid-read.
-    InstanceLock instance_lock(read_only ? std::string() : lock_path());
-
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -1000,6 +996,7 @@ void AppConfig::save()
     // The config is first written to a file with a PID suffix and then moved
     // to avoid race conditions with multiple instances of Slic3r
     const auto path = config_path();
+    std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     json j;
 
@@ -1129,18 +1126,43 @@ void AppConfig::save()
 
         j["local_machines"][local_machine.first] = m_json;
     }
-    const std::string config_str = j.dump(1, '\t');
-    if (write_config_file(path, config_str + "\n", config_str))
+    boost::nowide::ofstream c;
+    c.open(path_pid, std::ios::out | std::ios::trunc);
+    c << j.dump(1, '\t') << std::endl;
+
+#ifdef WIN32
+    // WIN32 specific: The final "rename_file()" call is not safe in case of an application crash, there is no atomic "rename file" API
+    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
+    // we save the config file into a backup first before moving it to the final destination.
+    c << appconfig_md5_hash_line(j.dump(1, '\t'));
+#endif
+
+    c.close();
+    if (c.fail()) {
+      BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
+      return;
+    }
+
+#ifdef WIN32
+    // Make a backup of the configuration file before copying it to the final destination.
+    std::string error_message;
+    std::string backup_path = (boost::format("%1%.bak") % path).str();
+    // Copy configuration file with PID suffix into the configuration file with "bak" suffix.
+    if (copy_file(path_pid, backup_path, error_message, false) != SUCCESS)
+        BOOST_LOG_TRIVIAL(error) << "Copying from " << path_pid << " to " << backup_path << " failed. Failed to create a backup configuration.";
+#endif
+
+    // Rename the config atomically.
+    // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile.
+    // To cope with that, we already made a backup of the config on Windows.
+    rename_file(path_pid, path);
     m_dirty = false;
 }
 
 #else
 
-std::string AppConfig::load(bool read_only)
+std::string AppConfig::load()
 {
-    // Keep another instance from replacing or restoring the file mid-read.
-    InstanceLock instance_lock(read_only ? std::string() : lock_path());
-
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -1278,6 +1300,7 @@ void AppConfig::save()
     // The config is first written to a file with a PID suffix and then moved
     // to avoid race conditions with multiple instances of Slic3r
     const auto path = config_path();
+    std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     std::stringstream config_ss;
     if (m_mode == EAppMode::Editor)
@@ -1313,39 +1336,39 @@ void AppConfig::save()
     // One empty line before the MD5 sum.
     config_ss << std::endl;
 
-    const std::string config_str = config_ss.str();
-    if (write_config_file(path, config_str, config_str))
+    std::string config_str = config_ss.str();
+    boost::nowide::ofstream c;
+    c.open(path_pid, std::ios::out | std::ios::trunc);
+    c << config_str;
+#ifdef WIN32
+    // WIN32 specific: The final "rename_file()" call is not safe in case of an application crash, there is no atomic "rename file" API
+    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
+    // we save the config file into a backup first before moving it to the final destination.
+    c << appconfig_md5_hash_line(config_str);
+#endif
+    c.close();
+    if (c.fail()) {
+      BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
+      return;
+    }
+
+#ifdef WIN32
+    // Make a backup of the configuration file before copying it to the final destination.
+    std::string error_message;
+    std::string backup_path = (boost::format("%1%.bak") % path).str();
+    // Copy configuration file with PID suffix into the configuration file with "bak" suffix.
+    if (copy_file(path_pid, backup_path, error_message, false) != SUCCESS)
+        BOOST_LOG_TRIVIAL(error) << "Copying from " << path_pid << " to " << backup_path << " failed. Failed to create a backup configuration.";
+#endif
+
+    // Rename the config atomically.
+    // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile.
+    // To cope with that, we already made a backup of the config on Windows.
+    rename_file(path_pid, path);
     m_dirty = false;
 }
 #endif
 
-bool AppConfig::write_config_file(const std::string &path, std::string body, const std::string &checksum_source)
-{
-    // Everything before this is assembly; only the writes need the other instances kept out.
-    InstanceLock instance_lock(lock_path());
-#ifdef WIN32
-    // WIN32 specific: the final replace is not safe in case of an application crash, there is no atomic "rename file" API
-    // provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
-    // we save the config file into a backup first before moving it to the final destination.
-    body += appconfig_md5_hash_line(checksum_source);
-#endif
-    // Not flushed to the device: the idle handler saves on the GUI thread after
-    // any change, and the rename already gives a complete old or new file.
-    if (const std::error_code ec = write_file_atomically(path, body)) {
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the configuration " << path << ": " << ec.message() << "; trying again in 10 s";
-        m_retry_save_at = std::chrono::steady_clock::now() + std::chrono::seconds(10);
-        return false;
-    }
-    m_retry_save_at = {};
-#ifdef WIN32
-    // Written after the config, so the backup never holds a state that was not confirmed written.
-    const std::string backup_path = (boost::format("%1%.bak") % path).str();
-    if (const std::error_code ec = write_file_atomically(backup_path, body))
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the backup configuration " << backup_path << ": " << ec.message();
-#endif
-    return true;
-}
-
 bool AppConfig::get_variant(const std::string &vendor, const std::string &model, const std::string &variant) const
 {
     const auto it_v = m_vendors.find(vendor);
@@ -1834,11 +1857,6 @@ void AppConfig::reset_selections()
     }
 }
 
-std::string AppConfig::lock_path()
-{
-    return Slic3r::data_dir().empty() ? std::string() : config_path() + ".lock";
-}
-
 std::string AppConfig::config_path()
 {
 #ifdef USE_JSON_CONFIG
@@ -1875,7 +1893,7 @@ bool AppConfig::exists()
 
 std::string AppConfig::load_if_exists()
 {
-    return boost::filesystem::exists(loading_path()) ? load(/*read_only=*/true) : std::string();
+    return boost::filesystem::exists(loading_path()) ? load() : std::string();
 }
 
 }; // namespace Slic3r

src/libslic3r/AppConfig.hpp

@@ -2,7 +2,6 @@
 #define slic3r_AppConfig_hpp_
 
 #include <set>
-#include <chrono>
 #include <map>
 #include <string>
 #include "nlohmann/json.hpp"
@@ -123,18 +122,14 @@ public:
 
 	// Load the slic3r.ini from a user profile directory (or a datadir, if configured).
 	// Return an error string, or an empty string on success.
-	std::string         load(bool read_only = false);
+	std::string         load();
 	// Treat a missing config as default state; otherwise load it normally.
-	// The CLI's load: it never saves, so it takes no lock and creates no lock file.
 	std::string         load_if_exists();
 	// Store the slic3r.ini into a user profile directory (or a datadir, if configured).
 	void 			   	save();
 
 	// Does this config need to be saved?
 	bool 				dirty() const { return m_dirty; }
-	// False for ten seconds after a failed write, so the idle handler does not
-	// repeat a hopeless attempt on every event; an explicit save() always tries.
-	bool 				save_due() const { return std::chrono::steady_clock::now() >= m_retry_save_at; }
 
 
 	void				set_dirty() { m_dirty = true; }
@@ -344,8 +339,6 @@ public:
 
 	// Get the default config path from Slic3r::data_dir().
 	std::string			config_path();
-	// Lock file guarding config_path() against other running instances; empty without a data dir.
-	std::string			lock_path();
 
 	// Returns true if the user's data directory comes from before Slic3r 1.40.0 (no updating)
 	bool 				legacy_datadir() const { return m_legacy_datadir; }
@@ -456,16 +449,8 @@ private:
 
 	// Preset for each machine
 	MachineSettingMap											m_printer_settings;
-	// Writes the assembled config text, and on Windows its checksum and a backup copy; false when the
-	// config itself could not be written, in which case the caller stays dirty and retries. `checksum_source`
-	// is the text load() will verify, which for the JSON config ends before the trailing newline.
-	bool												write_config_file(const std::string &path, std::string body, const std::string &checksum_source);
-
 	// Has any value been modified since the config.ini has been last saved or loaded?
 	bool														m_dirty;
-	// After a failed write, save_due() is false for the next ten seconds, so the
-	// idle handler does not repeat a hopeless write on every event.
-	std::chrono::steady_clock::time_point						m_retry_save_at{};
 	// Original version found in the ini file before it was overwritten
 	Semver                                                      m_orig_version;
 	// Whether the existing version is before system profiles & configuration updating

src/libslic3r/CMakeLists.txt

@@ -306,8 +306,6 @@ set(lisbslic3r_sources
     Geometry/VoronoiUtils.cpp
     Geometry/VoronoiUtils.hpp
     Geometry/VoronoiVisualUtils.hpp
-    InstanceLock.cpp
-    InstanceLock.hpp
     Int128.hpp
     KDTreeIndirect.hpp
     Layer.cpp

src/libslic3r/Config.cpp

@@ -1538,9 +1538,11 @@ void ConfigBase::save_to_json(const std::string &file, const std::string &name,
     // Serialize first: if that throws (invalid UTF-8), the existing file stays untouched.
     std::ostringstream ss;
     this->save_to_json(ss, name, from, version);
-    if (const std::error_code ec = write_file_atomically(file, ss.str()))
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(": failed to save config to %1%: %2%") % file % ec.message();
-    else
+    boost::nowide::ofstream c;
+    c.open(file, std::ios::out | std::ios::trunc);
+    c << ss.str();
+    c.close();
+
     BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ":" <<__LINE__ << boost::format(", saved config to %1%\n")%file;
 }
 

src/libslic3r/InstanceLock.cpp

@@ -1,165 +0,0 @@
-#include "InstanceLock.hpp"
-
-#include <map>
-#include <memory>
-#include <system_error>
-#include <thread>
-
-#include <boost/filesystem.hpp>
-#include <boost/log/trivial.hpp>
-#include <boost/nowide/fstream.hpp>
-#ifdef _WIN32
-#include <boost/interprocess/sync/file_lock.hpp>
-#include <boost/nowide/convert.hpp>
-#else
-#include <cerrno>
-#include <fcntl.h>
-#include <sys/file.h>
-#include <unistd.h>
-#endif
-
-namespace Slic3r {
-
-#ifdef _WIN32
-// LockFileEx, held by this handle alone.
-using NativeFileLock = boost::interprocess::file_lock;
-#else
-// flock(2) rather than an fcntl lock: it belongs to this open file description,
-// so any other code in the process that opens and closes the lock file, as a
-// backup or an export walking the data dir might, cannot drop it. An fcntl
-// lock would go with the first such close.
-class NativeFileLock
-{
-public:
-    explicit NativeFileLock(const char *path) : m_fd(::open(path, O_RDWR | O_CREAT | O_CLOEXEC, 0644))
-    {
-        if (m_fd < 0)
-            throw std::system_error(errno, std::generic_category(), path);
-    }
-    ~NativeFileLock() { ::close(m_fd); }
-    bool try_lock()
-    {
-        if (::flock(m_fd, LOCK_EX | LOCK_NB) == 0)
-            return true;
-        // A signal (a child exiting, for one) interrupts the call like any other; the caller polls again.
-        if (errno == EWOULDBLOCK || errno == EINTR)
-            return false;
-        throw std::system_error(errno, std::generic_category(), "flock");
-    }
-    void unlock() { ::flock(m_fd, LOCK_UN); }
-private:
-    int m_fd;
-};
-#endif
-
-// One slot per lock file, shared by every guard in the process: one lock
-// object per path behind a mutex is what makes the guard re-entrant and safe
-// to use from the preset sync thread and the GUI thread at once. The lock
-// file is opened by the outermost guard and closed when it goes, so the file
-// is never held open between guards: whatever is at the path is what gets
-// locked, and a data dir can be removed once nothing is saving into it. The
-// file is kept rather than deleted on release because the lock state lives in
-// the kernel on the open file, and deleting it would let a third instance
-// lock a fresh file while the second still holds the old one.
-struct InstanceLock::Slot
-{
-    std::recursive_mutex mutex;
-    // Non-null exactly while this process holds the file lock.
-    std::unique_ptr<NativeFileLock> file_lock;
-    int  depth{0};
-    // Until this point, after a guard could not open, lock or wait out the
-    // file, guards do not touch it.
-    std::chrono::steady_clock::time_point cooldown_until{};
-};
-
-InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path)
-{
-    // Never freed: a save during static destruction still needs its slot.
-    static auto *registry_mutex = new std::mutex();
-    static auto *registry       = new std::map<std::string, std::unique_ptr<Slot>>();
-
-    std::lock_guard<std::mutex> guard(*registry_mutex);
-    std::unique_ptr<Slot> &slot = (*registry)[lock_file_path];
-    if (! slot)
-        slot = std::make_unique<Slot>();
-    return *slot;
-}
-
-// Starts the cool-down. Called with the slot mutex held.
-void InstanceLock::defer(Slot &slot, const std::string &reason)
-{
-    slot.cooldown_until = std::chrono::steady_clock::now() + cooldown;
-    BOOST_LOG_TRIVIAL(warning) << reason << "; proceeding without the lock for the next " << cooldown.count() << " ms";
-}
-
-// Creates the lock file if needed and opens it, or starts the cool-down.
-// Called with the slot mutex held.
-bool InstanceLock::open_lock_file(Slot &slot, const std::string &lock_file_path)
-{
-    try {
-#ifdef _WIN32
-        // The lock opens an existing file; created once, on the first miss.
-        const std::wstring wide_path = boost::nowide::widen(lock_file_path);
-        try {
-            slot.file_lock = std::make_unique<NativeFileLock>(wide_path.c_str());
-        } catch (const std::exception &) {
-            boost::nowide::ofstream(lock_file_path, std::ios::app).close();
-            slot.file_lock = std::make_unique<NativeFileLock>(wide_path.c_str());
-        }
-#else
-        slot.file_lock = std::make_unique<NativeFileLock>(lock_file_path.c_str());
-#endif
-        return true;
-    } catch (const std::exception &e) {
-        defer(slot, "Cannot open lock file " + lock_file_path + ": " + e.what() + " (check its owner and permissions)");
-        return false;
-    }
-}
-
-InstanceLock::InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout)
-{
-    if (lock_file_path.empty())
-        return;
-    m_slot       = &slot_for(lock_file_path);
-    m_slot_guard = std::unique_lock<std::recursive_mutex>(m_slot->mutex);
-    const auto now = std::chrono::steady_clock::now();
-    if (m_slot->depth == 0 && now >= m_slot->cooldown_until && open_lock_file(*m_slot, lock_file_path)) {
-        const auto deadline = now + timeout;
-        bool       taken    = false;
-        for (;;) {
-            try {
-                if ((taken = m_slot->file_lock->try_lock()))
-                    break;
-            } catch (const std::exception &e) {
-                defer(*m_slot, "Cannot lock " + lock_file_path + ": " + e.what());
-                break;
-            }
-            if (std::chrono::steady_clock::now() >= deadline) {
-                defer(*m_slot, "Another instance has held " + lock_file_path + " for over " + std::to_string(timeout.count()) + " ms");
-                break;
-            }
-            std::this_thread::sleep_for(std::chrono::milliseconds(5));
-        }
-        if (! taken)
-            m_slot->file_lock.reset();
-    }
-    // Counted last, so a throw above leaves the slot exactly as it was found.
-    ++ m_slot->depth;
-    m_locked = m_slot->file_lock != nullptr;
-}
-
-InstanceLock::~InstanceLock()
-{
-    if (m_slot == nullptr)
-        return;
-    if (-- m_slot->depth == 0 && m_slot->file_lock) {
-        try {
-            m_slot->file_lock->unlock();
-        } catch (const std::exception &e) {
-            BOOST_LOG_TRIVIAL(warning) << "Cannot unlock instance lock: " << e.what();
-        }
-        m_slot->file_lock.reset();
-    }
-}
-
-} // namespace Slic3r

src/libslic3r/InstanceLock.hpp

@@ -1,60 +0,0 @@
-#pragma once
-
-#include <chrono>
-#include <mutex>
-#include <string>
-
-namespace Slic3r {
-
-// Scoped write lock on a file shared by every running instance of the
-// application, such as the app config or the user preset directory: threads
-// of this process are serialised through a recursive mutex, other processes
-// through an advisory OS file lock on `lock_file_path`. The lock file is
-// created on first use and kept; the OS releases the lock when its holder
-// exits, so a crashed instance never leaves a stale lock behind.
-//
-// Best effort: when the lock file cannot be opened or locked, or another
-// instance still holds it after `timeout`, the guard keeps only the in-process
-// mutex, locked() reports false and the write proceeds, since a hung instance
-// must never block another one from saving. For `cooldown` afterwards guards
-// leave the file alone. The wait for the in-process mutex is bounded only by
-// the longest critical section, so a guard covers a few file operations and
-// nothing slower.
-//
-// Lock order: the preset collection mutex may be held when a guard is taken
-// (set_sync_info_and_save() calls save_info() under it), never the reverse;
-// that is why the guards sit at the leaf readers and writers and why a guard
-// must not be added around save_user_presets(), which takes the collection
-// mutex through delete_preset().
-class InstanceLock
-{
-public:
-    // Long against a critical section of milliseconds, short against the GUI
-    // thread, which is where most guards are taken.
-    static constexpr std::chrono::milliseconds default_timeout{1000};
-    // Long enough that a holder stuck in a debugger does not cost a stall per
-    // save; mutable so tests can shorten it.
-    static inline std::chrono::milliseconds cooldown{10000};
-
-    // An empty path makes the guard a no-op.
-    explicit InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout = default_timeout);
-    ~InstanceLock();
-
-    InstanceLock(const InstanceLock &) = delete;
-    InstanceLock &operator=(const InstanceLock &) = delete;
-
-    // True while this process holds the cross-process file lock.
-    bool locked() const { return m_locked; }
-
-private:
-    struct Slot;
-    static Slot &slot_for(const std::string &lock_file_path);
-    static bool  open_lock_file(Slot &slot, const std::string &lock_file_path);
-    static void  defer(Slot &slot, const std::string &reason);
-
-    Slot *m_slot{nullptr};
-    std::unique_lock<std::recursive_mutex> m_slot_guard;
-    bool  m_locked{false};
-};
-
-} // namespace Slic3r

src/libslic3r/Preset.cpp

@@ -51,9 +51,6 @@
 #include "libslic3r.h"
 #include "LifecycleEvents.hpp"
 #include "Utils.hpp"
-#include "InstanceLock.hpp"
-
-#include <sstream>
 #include "Time.hpp"
 #include "PlaceholderParser.hpp"
 #include "libslic3r/GCode/Thumbnails.hpp"
@@ -110,32 +107,6 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const
     }
 }
 
-std::string user_presets_lock_path(bool read_only)
-{
-    return read_only || data_dir().empty() ? std::string() : (fs::path(data_dir()) / (PRESET_USER_DIR ".lock")).string();
-}
-
-// Removes a preset file the scan could not load, and its .info, under the lock.
-// The scan passes read_only when it could not take the lock, since the file may
-// then be another instance's fresh write that it merely raced.
-static void remove_preset_files(const std::string &preset_file, bool read_only)
-{
-    if (read_only)
-        return;
-    InstanceLock instance_lock(user_presets_lock_path());
-    // A symlink whose target is missing stays, since the target may come back; one
-    // that cannot even be followed is removed like any other unreadable file.
-    auto remove = [](const fs::path &file) {
-        boost::system::error_code ec;
-        if (fs::status(file, ec).type() != fs::file_not_found)
-            fs::remove(file, ec);
-    };
-    fs::path file_path(preset_file);
-    remove(file_path);
-    file_path.replace_extension(".info");
-    remove(file_path);
-}
-
 std::string get_preset_bare_name(const std::string &canonical_name)
 {
     const auto pos = canonical_name.find_last_of('/');
@@ -678,20 +649,18 @@ void Preset::save_info(std::string file)
         file = idx_file.string();
     }
 
+    boost::nowide::ofstream c;
+    c.open(file, std::ios::out | std::ios::trunc);
     std::string sync_info_to_save;
     //BBS: hold is used for stop requesting to server this time
     if (this->sync_info.compare("hold") != 0)
         sync_info_to_save = this->sync_info;
-    std::ostringstream c;
     c << "sync_info" << " = " << sync_info_to_save << std::endl;
     c << "user_id" << " = " << this->user_id << std::endl;
     c << "setting_id" << " = " << this->setting_id << std::endl;
     c << "base_id" << " = " << this->base_id << std::endl;
     c << "updated_time" << " = " << std::to_string(this->updated_time) << std::endl;
-
-    InstanceLock instance_lock(user_presets_lock_path());
-    if (const std::error_code ec = write_file_atomically(file, c.str()))
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << file << ": " << ec.message();
+    c.close();
 }
 
 void Preset::remove_files(bool cloud_already_deleted)
@@ -700,7 +669,6 @@ void Preset::remove_files(bool cloud_already_deleted)
     if (this->is_project_embedded) {
         return;
     }
-    InstanceLock instance_lock(user_presets_lock_path());
     // Erase the preset file.
     boost::nowide::remove(this->file.c_str());
     fs::path idx_path(this->file);
@@ -718,11 +686,11 @@ void Preset::remove_files(bool cloud_already_deleted)
 }
 
 //BBS: add logic for only difference save
-bool Preset::save(DynamicPrintConfig* parent_config)
+void Preset::save(DynamicPrintConfig* parent_config)
 {
     //BBS: add project embedded preset logic
     if (this->is_project_embedded)
-        return true;
+        return;
     //BBS: change to json format
     //this->config.save(this->file);
     std::string from_str;
@@ -737,16 +705,12 @@ bool Preset::save(DynamicPrintConfig* parent_config)
     else
         from_str = std::string("Default");
 
+    boost::filesystem::create_directories(fs::path(this->file).parent_path());
     const std::string bare_name = get_preset_bare_name(this->name);
 
-    // What gets written: the diff against the parent, the config plus its
-    // filament id, or the config as is. Built before the lock is taken so the
-    // exclusive window covers only the file writes.
-    DynamicPrintConfig        temp_config;
-    const DynamicPrintConfig *to_save = &this->config;
-
     //BBS: only save difference if it has parent
     if (parent_config) {
+        DynamicPrintConfig temp_config;
         std::vector<std::string> dirty_options = config.diff(*parent_config);
 
         std::string extruder_id_name, extruder_variant_name;
@@ -782,22 +746,13 @@ bool Preset::save(DynamicPrintConfig* parent_config)
                     opt_dst->set(opt_src);
             }
         }
-        to_save = &temp_config;
+        temp_config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
     } else if (!filament_id.empty() && inherits().empty()) {
-        temp_config = config;
+        DynamicPrintConfig temp_config = config;
         temp_config.set_key_value(BBL_JSON_KEY_FILAMENT_ID, new ConfigOptionString(filament_id));
-        to_save = &temp_config;
-    }
-
-    std::ostringstream json;
-    to_save->save_to_json(json, bare_name, from_str, this->version.to_string());
-
-    InstanceLock instance_lock(user_presets_lock_path());
-    boost::filesystem::create_directories(fs::path(this->file).parent_path());
-    if (const std::error_code ec = write_file_atomically(this->file, json.str())) {
-        // No .info either: one without its preset reads as a cloud deletion request.
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << this->file << ": " << ec.message();
-        return false;
+        temp_config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
+    } else {
+        this->config.save_to_json(this->file, bare_name, from_str, this->version.to_string());
     }
     BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " save config for: " << this->name << " and filament_id: " << filament_id << " and base_id: " << this->base_id;
 
@@ -807,7 +762,6 @@ bool Preset::save(DynamicPrintConfig* parent_config)
         idx_file.replace_extension(".info");
         this->save_info(idx_file.string());
     }
-    return true;
 }
 
 void Preset::reload(Preset const &parent)
@@ -819,7 +773,6 @@ void Preset::reload(Preset const &parent)
     std::string                        reason;
     ForwardCompatibilitySubstitutionRule substitution_rule    = ForwardCompatibilitySubstitutionRule::Disable;
     try {
-        InstanceLock instance_lock(user_presets_lock_path());
         ConfigSubstitutions                config_substitutions = config.load_from_json(file, substitution_rule, key_values, reason);
         this->config = parent.config;
         this->config.apply(std::move(config));
@@ -1718,22 +1671,6 @@ std::string PresetCollection::canonical_preset_name(const std::string &name, con
     return get_preset_canonical_name(parsed.bare, origin);
 }
 
-PresetCollection::PresetFilesOnDisk PresetCollection::PresetFilesOnDisk::read(const boost::filesystem::path &file)
-{
-    auto read_if_present = [](const fs::path &path) -> std::optional<std::string> {
-        std::string bytes;
-        try {
-            load_string_file(path, bytes);
-        } catch (const std::exception &) {
-            return std::nullopt;
-        }
-        return bytes;
-    };
-    fs::path info_path(file);
-    info_path.replace_extension(".info");
-    return { read_if_present(file), read_if_present(info_path) };
-}
-
 PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
     const boost::filesystem::path &file, const std::string &canonical_name,
     const PresetOrigin &load_origin, ForwardCompatibilitySubstitutionRule substitution_rule,
@@ -1745,9 +1682,6 @@ PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset(
     Preset &preset = out.preset;
     preset.bundle_id = load_origin.bundle_id;
     preset.file = file.string();
-    // Before either file is parsed, so a save that lands during the parse still shows
-    // up as a difference when commit compares.
-    out.on_disk = PresetFilesOnDisk::read(file);
     // Load the preset file, apply preset values on top of defaults.
     try {
         fs::path idx_path(preset.file);
@@ -1860,8 +1794,14 @@ void PresetCollection::commit_user_preset(UserPresetLoad &&loaded, std::deque<Pr
         ++m_errors;
         BOOST_LOG_TRIVIAL(error) << error;
     }
-    if (loaded.discard_file)
-        remove_preset_files(loaded.preset.file, read_only);
+    if (loaded.discard_file && !read_only) {
+        fs::path file_path(loaded.preset.file);
+        if (fs::exists(file_path))
+            fs::remove(file_path);
+        file_path.replace_extension(".info");
+        if (fs::exists(file_path))
+            fs::remove(file_path);
+    }
     if (!loaded.install)
         return;
 
@@ -1869,12 +1809,9 @@ void PresetCollection::commit_user_preset(UserPresetLoad &&loaded, std::deque<Pr
         if (loaded.save_compatible_printers) {
             // A filesystem error from the rewrite is counted, and the preset still loads.
             try {
-                if (read_only)
-                    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << preset.name << " (not written back)";
-                else if (preset.save(nullptr))
+                if (!read_only)
+                    preset.save(nullptr);
                 BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " added compatible_printers for preset: " << preset.name;
-                else
-                    ++m_errors; // save() logged why
             } catch (const std::runtime_error &err) {
                 ++m_errors;
                 BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " can not write compatible_printers back to " << preset.file << ": " << err.what();
@@ -1960,32 +1897,11 @@ void PresetCollection::load_presets(
             return this->resolve_user_preset(files[i].path, files[i].canonical_name, resolved_origin, substitution_rule,
                                              extruder_id_name, extruder_variant_name, key_set1, key_set2);
         },
-        [&](size_t i, UserPresetLoad &&loaded) {
-            // Resolve read the files without the lock, so another instance may have saved
-            // over them since. A file that changed is resolved again under the lock before
-            // commit removes or rewrites it, which also keeps its .json and .info from two
-            // different saves apart. Without the lock nothing is checked, so commit leaves
-            // the files alone.
-            const std::string lock_path = user_presets_lock_path(read_only);
-            InstanceLock      instance_lock(lock_path);
-            if (instance_lock.locked()) {
-                const PresetFilesOnDisk   on_disk = PresetFilesOnDisk::read(files[i].path);
-                boost::system::error_code ec;
-                // The link itself, and a stat error counts as present, so a file that is
-                // there but cannot be read is still counted and removed by commit.
-                if (! on_disk.json && fs::symlink_status(files[i].path, ec).type() == fs::file_not_found)
-                    return; // removed by another instance since it was read
-                if (! (on_disk == loaded.on_disk)) {
-                    CNumericLocalesSetter locales_setter;
-                    loaded = this->resolve_user_preset(files[i].path, files[i].canonical_name, resolved_origin, substitution_rule,
-                                                       extruder_id_name, extruder_variant_name, key_set1, key_set2);
-                }
-            }
-            const bool leave_files = read_only || (! lock_path.empty() && ! instance_lock.locked());
+        [&](size_t, UserPresetLoad &&loaded) {
             // Committing can remove an unreadable preset's file, and a filesystem error
             // there is reported without stopping the rest of the directory.
             try {
-                this->commit_user_preset(std::move(loaded), presets_loaded, substitutions, preset_loaded_fn, leave_files);
+                this->commit_user_preset(std::move(loaded), presets_loaded, substitutions, preset_loaded_fn, read_only);
             } catch (const std::runtime_error &err) {
                 errors_cummulative += err.what();
                 errors_cummulative += "\n";
@@ -2295,10 +2211,7 @@ void PresetCollection::set_sync_info_and_save(std::string name, std::string sett
             preset->setting_id = setting_id;
             if (update_time > 0)
                 preset->updated_time = update_time;
-            if (preset->sync_info == "update")
-                preset->save(nullptr);
-            else
-                preset->save_info();
+            preset->sync_info == "update" ? preset->save(nullptr) : preset->save_info();
             break;
         }
     }
@@ -4380,15 +4293,8 @@ void PhysicalPrinter::update_preset_names_in_config()
     }
 }
 
-void PhysicalPrinter::save(DynamicPrintConfig* /* parent_config */)
-{
-    InstanceLock instance_lock(user_presets_lock_path());
-    this->config.save_to_json(this->file, std::string("Physical_Printer"), std::string("User"), std::string(SLIC3R_VERSION));
-}
-
 void PhysicalPrinter::save(const std::string& file_name_from, const std::string& file_name_to)
 {
-    InstanceLock instance_lock(user_presets_lock_path());
     // rename the file
     boost::nowide::rename(file_name_from.data(), file_name_to.data());
     this->file = file_name_to;
@@ -4520,7 +4426,6 @@ void PhysicalPrinterCollection::load_printers(
                 continue;
             }
             try {
-                InstanceLock instance_lock(user_presets_lock_path());
                 PhysicalPrinter printer(name, this->default_config());
                 printer.file = dir_entry.path().string();
                 // Load the preset file, apply preset values on top of defaults.
@@ -4713,10 +4618,7 @@ bool PhysicalPrinterCollection::delete_printer(const std::string& name)
 
     const PhysicalPrinter& printer = *it;
     // Erase the preset file.
-    {
-        InstanceLock instance_lock(user_presets_lock_path());
     boost::nowide::remove(printer.file.c_str());
-    }
     m_printers.erase(it);
     return true;
 }
@@ -4728,10 +4630,7 @@ bool PhysicalPrinterCollection::delete_selected_printer()
     const PhysicalPrinter& printer = this->get_selected_printer();
 
     // Erase the preset file.
-    {
-        InstanceLock instance_lock(user_presets_lock_path());
     boost::nowide::remove(printer.file.c_str());
-    }
     // Remove the preset from the list.
     m_printers.erase(m_printers.begin() + m_idx_selected);
     // unselect all printers

src/libslic3r/Preset.hpp

@@ -8,7 +8,6 @@
 #include <unordered_set>
 #include <functional>
 #include <mutex>
-#include <optional>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/filesystem/path.hpp>
 #include <boost/property_tree/ptree_fwd.hpp>
@@ -331,8 +330,7 @@ public:
 
     //BBS: add logic for only difference save
     //if parent_config is null, save all keys, otherwise, only save difference
-    // Returns false when the preset file could not be written.
-    bool                save(DynamicPrintConfig* parent_config);
+    void                save(DynamicPrintConfig* parent_config);
     void                reload(Preset const & parent);
 
     // Return a label of this preset, consisting of a name and a "(modified)" suffix, if this preset is dirty.
@@ -490,12 +488,6 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const
 // Tail segment of a canonical name — what's written to the bundle's .json filename and JSON "name" field.
 std::string get_preset_bare_name(const std::string &canonical_name);
 
-// Lock file guarding every user preset file under data_dir() against other
-// running instances and the preset sync thread. Empty without a data dir, and
-// for a read-only load (the CLI), which never rewrites or deletes and may run
-// many jobs on one data dir.
-std::string user_presets_lock_path(bool read_only = false);
-
 // Resolve an origin from a directory path when the caller passes Kind::Auto.
 PresetOrigin detect_origin_from_path(const boost::filesystem::path &path, const PresetOrigin &explicit_origin = PresetOrigin());
 
@@ -905,16 +897,6 @@ protected:
     void            set_custom_preset_alias(Preset &preset);
 
 private:
-    // A preset file and its .info as read from disk, std::nullopt for one that is missing.
-    struct PresetFilesOnDisk
-    {
-        std::optional<std::string> json;
-        std::optional<std::string> info;
-
-        static PresetFilesOnDisk read(const boost::filesystem::path &file);
-        bool operator==(const PresetFilesOnDisk &rhs) const { return json == rhs.json && info == rhs.info; }
-    };
-
     // One preset file read and flattened against the presets already in this
     // collection, before anything the collection shares has been touched.
     struct UserPresetLoad
@@ -932,9 +914,6 @@ private:
         bool        discard_file { false };
         // The .info file read beside the preset, which commit logs.
         std::string info_file;
-        // Both files as they were before the preset was read from them. Resolve runs
-        // without the instance lock, so commit compares this with the disk under it.
-        PresetFilesOnDisk on_disk;
         // Counted and logged by commit, in the order the directory listed the files.
         std::vector<std::string>   errors;
         PresetsConfigSubstitutions substitutions;
@@ -1123,7 +1102,7 @@ public:
 
     //BBS: change to json format
     //void                save() { this->config.save(this->file); }
-    void                save(DynamicPrintConfig* parent_config);
+    void                save(DynamicPrintConfig* parent_config) { this->config.save_to_json(this->file, std::string("Physical_Printer"), std::string("User"), std::string(SLIC3R_VERSION)); }
     void                save(const std::string& file_name_from, const std::string& file_name_to);
 
     void                update_from_preset(const Preset& preset);

src/libslic3r/PresetBundle.cpp

@@ -1,4 +1,3 @@
-#include <atomic>
 #include <cassert>
 #include <chrono>
 #include <ctime>
@@ -14,7 +13,6 @@
 #include "libslic3r.h"
 #include "I18N.hpp"
 #include "Utils.hpp"
-#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Model.hpp"
 #include "TriangleSelector.hpp"
@@ -1235,13 +1233,6 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
 
     const auto user_load_t0 = std::chrono::steady_clock::now();
 
-    // Reads one bundle's metadata under the lock, per file, so the lock is never
-    // held when bundles.WriteLock() is taken afterwards.
-    auto load_bundle_metadata = [read_only](const fs::path &metadata_file, BundleMetadata &metadata) {
-        InstanceLock instance_lock(user_presets_lock_path(read_only));
-        return metadata.load_from_json(metadata_file.string());
-    };
-
     // Load bundle metadata from _local directory first
     fs::path local_dir(folder / PRESET_LOCAL_DIR);
     if (fs::exists(local_dir)) {
@@ -1255,7 +1246,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!load_bundle_metadata(metadata_file, metadata)) continue;
+            if (!metadata.load_from_json(metadata_file.string())) continue;
             metadata.print_presets.clear();
             metadata.filament_presets.clear();
             metadata.printer_presets.clear();
@@ -1290,7 +1281,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!load_bundle_metadata(metadata_file, metadata)) continue;
+            if (!metadata.load_from_json(metadata_file.string())) continue;
             metadata.print_presets.clear();
             metadata.filament_presets.clear();
             metadata.printer_presets.clear();
@@ -1624,12 +1615,10 @@ PresetsConfigSubstitutions PresetBundle::import_presets(std::vector<std::string>
             if (ec) BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " create directory failed: " << ec.message();
             //create temp folder
             //std::string user_default_temp_dir = data_dir() + "/" + PRESET_USER_DIR + "/" + DEFAULT_USER_FOLDER_NAME + "/" + "temp";
-            // Under cache/, per process and per import, so two instances importing
-            // at once do not clear each other's extraction and no preset scan reads it.
-            static std::atomic<unsigned> import_counter{0};
-            fs::path temp_folder(fs::path(data_dir()) / "cache" / ("import." + std::to_string(get_current_pid()) + "." + std::to_string(import_counter++)));
+            fs::path temp_folder(configs_folder / "temp");
             std::string user_default_temp_dir = temp_folder.make_preferred().string();
-            fs::create_directories(temp_folder, ec);
+            if (fs::exists(temp_folder)) fs::remove_all(temp_folder);
+            fs::create_directory(temp_folder, ec);
             if (ec) BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << " create directory failed: " << ec.message();
 
             file = boost::filesystem::path(file).make_preferred().string();
@@ -1641,9 +1630,6 @@ PresetsConfigSubstitutions PresetBundle::import_presets(std::vector<std::string>
             status        = mz_zip_reader_init_cfile(&zip_archive, zipFile, 0, MZ_ZIP_FLAG_CASE_SENSITIVE | MZ_ZIP_FLAG_IGNORE_PATH);
             if (MZ_FALSE == status) {
                 BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " Failed to initialize reader ZIP archive";
-                if (zipFile != nullptr)
-                    std::fclose(zipFile);
-                fs::remove_all(temp_folder, ec);
                 return substitutions;
             } else {
                 BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " Success to initialize reader ZIP archive";
@@ -2251,9 +2237,10 @@ void PresetBundle::remove_user_presets_directory(const std::string preset_folder
         return;
     }
     BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(" enter, delete directory : %1%") % dir_user_presets;
-    boost::system::error_code ec;
-    InstanceLock instance_lock(user_presets_lock_path());
-    fs::remove_all(fs::path(dir_user_presets), ec);
+    fs::path folder(dir_user_presets);
+    if (fs::exists(folder)) {
+        fs::remove_all(folder);
+    }
 }
 
 void PresetBundle::update_system_preset_setting_ids(std::map<std::string, std::map<std::string, std::string>>& system_presets)
@@ -8200,13 +8187,9 @@ bool BundleMetadata::save_to_json(const std::string& path) const
         j["filament_presets"] = strip_prefix(this->filament_presets);                                                                                                                                                                                                                       
         j["printer_presets"] = strip_prefix(this->printer_presets);
 
-        const std::string content = j.dump(4);
-        InstanceLock instance_lock(user_presets_lock_path());
-        if (const std::error_code ec = write_file_atomically(path, content)) {
-            BOOST_LOG_TRIVIAL(error) << "Failed to save bundle metadata to " << path << ": " << ec.message();
-            return false;
-        }
-        return true;
+        boost::nowide::ofstream ofs(path);
+        ofs << j.dump(4);
+        return ofs.good();
     } catch (const std::exception& e) {
         BOOST_LOG_TRIVIAL(error) << "Failed to save bundle metadata to " << path << ": " << e.what();
         return false;

src/libslic3r/PresetCacheFormat.cpp

@@ -400,24 +400,46 @@ bool write_cache_blob(const std::string& path, const std::string& blob)
 {
     boost::crc_32_type crc;
     crc.process_bytes(blob.data(), blob.size());
-    // Written beside the target and moved into place: a cache is truncated and
-    // rewritten in full, so a write that dies partway would otherwise leave a
-    // header claiming more body than the file holds.
+    // Written beside the target and moved into place, as AppConfig::save does:
+    // a cache is truncated and rewritten in full, so a write that dies partway
+    // would otherwise leave a header claiming more body than the file holds.
+    // The PID suffix also keeps two instances writing the same vendor from
+    // interleaving.
+    const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + ".tmp";
     try {
         boost::filesystem::create_directories(boost::filesystem::path(path).parent_path());
+        {
+            boost::nowide::ofstream ofs(tmp_path, std::ios::binary | std::ios::trunc);
+            if (!ofs.is_open()) {
+                BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: cannot open for writing: " << tmp_path;
+                return false;
+            }
             CacheFileHeader fhdr;
             fhdr.magic     = CACHE_MAGIC;
             fhdr.version   = CACHE_VERSION;
             fhdr.data_size = static_cast<uint64_t>(blob.size());
             fhdr.crc32     = crc.checksum();
-        const std::string_view header(reinterpret_cast<const char*>(&fhdr), sizeof(fhdr));
-        if (const std::error_code ec = write_file_atomically(path, { header, std::string_view(blob) }, /*binary=*/true)) {
-            BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << path << "): " << ec.message();
+            ofs.write(reinterpret_cast<const char*>(&fhdr), sizeof(fhdr));
+            ofs.write(blob.data(), static_cast<std::streamsize>(blob.size()));
+            ofs.close();   // flush; close() raises failbit on error
+            if (! ofs.good()) {
+                BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << tmp_path << ")";
+                boost::system::error_code ec;
+                boost::filesystem::remove(tmp_path, ec);
+                return false;
+            }
+        }
+        if (const std::error_code ec = rename_file(tmp_path, path)) {
+            BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: could not move " << tmp_path << " into place: " << ec.message();
+            boost::system::error_code rm;
+            boost::filesystem::remove(tmp_path, rm);
             return false;
         }
         return true;
     } catch (const std::exception& e) {
         BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << path << "): " << e.what();
+        boost::system::error_code ec;
+        boost::filesystem::remove(tmp_path, ec);
         return false;
     }
 }

src/libslic3r/Utils.hpp

@@ -8,8 +8,6 @@
 #include <functional>
 #include <type_traits>
 #include <system_error>
-#include <initializer_list>
-#include <string_view>
 #include <regex>
 
 #include <boost/system/error_code.hpp>
@@ -226,21 +224,6 @@ extern std::vector<std::string> split_string(const std::string &str, char delimi
 // On Windows, the file explorer (or anti-virus or whatever else) often locks the file
 // for a short while, so the file may not be movable. Retry while we see recoverable errors.
 extern std::error_code rename_file(const std::string &from, const std::string &to);
-// Write `chunks`, in order, to `path` through a temporary file beside it that is
-// then renamed over the target, so a concurrent reader sees the old or the new
-// file, never a partial one. The temporary is removed on failure and an existing
-// target keeps its permissions. Text mode unless `binary`, so Windows writes CRLF
-// as the streams this replaces did. A target that is not a regular file (a
-// device or pipe) is written in place, since replacing it would change what it
-// is, and so is an existing target beside which no temporary can be created or
-// whose replace the filesystem refuses; a symlink is followed and the file it
-// names is replaced. On Windows a reader holding the
-// target open without sharing its deletion, which the C runtime does not, makes
-// the replace fall back to the in-place write too, so an unlocked reader there
-// can still see a partial file.
-extern std::error_code write_file_atomically(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary = false);
-inline std::error_code write_file_atomically(const std::string &path, const std::string &content, bool binary = false)
-	{ return write_file_atomically(path, { std::string_view(content) }, binary); }
 
 enum CopyFileResult {
 	SUCCESS = 0,

src/libslic3r/utils.cpp

@@ -9,8 +9,6 @@
 #include <stdio.h>
 #include <filesystem>
 #include <sstream>
-#include <cerrno>
-#include <mutex>
 #include <iomanip>
 #include <algorithm>
 #include <cmath>
@@ -705,99 +703,13 @@ namespace WindowsSupport
 std::error_code rename_file(const std::string &from, const std::string &to)
 {
 #ifdef _WIN32
-	// Retries and moves an open destination aside itself.
 	return WindowsSupport::rename(from, to);
 #else
-	// rename(2) replaces an existing target atomically; removing it first would
-	// leave a window in which the file does not exist at all.
-	if (boost::nowide::rename(from.c_str(), to.c_str()) == 0)
-		return {};
-	const int err = errno;
-	// Some mounts (sshfs, gvfs, MTP and a few SMB setups) refuse to replace an
-	// existing target in one step, each with the error it sees fit; every error
-	// is worth the remove-then-rename this always did, except the ones no retry
-	// can help: nothing at the source, a different device, or a directory where
-	// a file was expected and the reverse.
-	const bool worth_retrying = err != ENOENT && err != EXDEV && err != ENOTDIR && err != EISDIR;
-	if (worth_retrying && boost::nowide::remove(to.c_str()) == 0 && boost::nowide::rename(from.c_str(), to.c_str()) == 0)
-		return {};
-	return std::make_error_code(static_cast<std::errc>(err));
+	boost::nowide::remove(to.c_str());
+	return std::make_error_code(static_cast<std::errc>(boost::nowide::rename(from.c_str(), to.c_str())));
 #endif
 }
 
-static std::error_code write_whole_file(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	errno = 0;
-	FILE *file = boost::nowide::fopen(path.c_str(), binary ? "wb" : "w");
-	if (file == nullptr)
-		return std::make_error_code(errno != 0 ? static_cast<std::errc>(errno) : std::errc::io_error);
-	bool ok = true;
-	for (const std::string_view chunk : chunks)
-		ok = ok && std::fwrite(chunk.data(), 1, chunk.size(), file) == chunk.size();
-	ok = ok && std::fflush(file) == 0;
-	const int err = ok ? 0 : errno;
-	ok = std::fclose(file) == 0 && ok;
-	if (ok)
-		return {};
-	return std::make_error_code(err != 0 ? static_cast<std::errc>(err) : std::errc::io_error);
-}
-
-// The in-place fallback truncates the target, so two threads of this process
-// on the same file must not both be in it. One mutex for all such writes: they
-// are the rare case. Never freed, like the InstanceLock registry, so a save
-// during static destruction still finds it.
-static std::error_code write_in_place(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	static auto *mutex = new std::mutex();
-	std::lock_guard<std::mutex> guard(*mutex);
-	return write_whole_file(path, chunks, binary);
-}
-
-std::error_code write_file_atomically(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	boost::system::error_code bec;
-	const boost::filesystem::file_status target = boost::filesystem::symlink_status(path, bec);
-	const bool target_exists = ! bec && boost::filesystem::exists(target);
-	if (target_exists && boost::filesystem::is_symlink(target)) {
-		// A config or preset kept in a dotfiles repository: the link stays,
-		// the file it points to is replaced like any other.
-		const boost::filesystem::path resolved = boost::filesystem::canonical(path, bec);
-		if (! bec && boost::filesystem::is_regular_file(resolved, bec))
-			return write_file_atomically(resolved.string(), chunks, binary);
-	}
-	if (target_exists && ! boost::filesystem::is_regular_file(target))
-		return write_in_place(path, chunks, binary);
-
-	// Unique per process and per call, so two threads writing one target
-	// without a lock never share a temporary.
-	static std::atomic<unsigned> counter{0};
-	const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + "." + std::to_string(counter++) + ".tmp";
-	if (const std::error_code ec = write_whole_file(tmp_path, chunks, binary)) {
-		boost::nowide::remove(tmp_path.c_str());
-		if (! target_exists)
-			return ec;
-		// A directory that lets this process write its files but not create
-		// one: losing the save is worse than a reader seeing a partial file.
-		BOOST_LOG_TRIVIAL(warning) << "Cannot create a temporary beside " << path << " (" << ec.message() << "); writing in place";
-		return write_in_place(path, chunks, binary);
-	}
-#ifndef _WIN32
-	// Not on Windows, where a read-only bit on the temporary would stop the rename itself.
-	if (target_exists)
-		boost::filesystem::permissions(tmp_path, target.permissions(), bec);
-#endif
-	if (const std::error_code ec = rename_file(tmp_path, path)) {
-		boost::nowide::remove(tmp_path.c_str());
-		// A reader on Windows holding the target open without FILE_SHARE_DELETE,
-		// or a mount that cannot replace a file at all. Losing the save is worse
-		// than a reader seeing a partial file, so write in place the way this
-		// used to work before the atomic path existed.
-		BOOST_LOG_TRIVIAL(warning) << "Cannot replace " << path << " (" << ec.message() << "); writing in place";
-		return write_in_place(path, chunks, binary);
-	}
-	return {};
-}
-
 #ifdef __linux__
 // Copied from boost::filesystem.
 // Called by copy_file_linux() in case linux sendfile() API is not supported.

src/slic3r/GUI/GLCanvas3D.cpp

@@ -6546,8 +6546,11 @@ void GLCanvas3D::render_thumbnail_internal(ThumbnailData& thumbnail_data, const
     //    glsafe(::glClearColor(1.0f, 1.0f, 1.0f, 1.0f));
     BOOST_LOG_TRIVIAL(info) << boost::format("render_thumbnail: finished");
 
-    // Puts the canvas viewport back in place of the thumbnail one set above.
-    wxGetApp().plater()->get_camera().apply_viewport();
+    // Puts the canvas viewport back in place of the thumbnail one set above. The CLI renders
+    // thumbnails with no application and no plater, so there is no canvas viewport to restore.
+    if (wxTheApp != nullptr)
+        if (Plater *plater = wxGetApp().plater(); plater != nullptr)
+            plater->get_camera().apply_viewport();
 }
 
 void GLCanvas3D::render_thumbnail_framebuffer(ThumbnailData& thumbnail_data, unsigned int w, unsigned int h, const ThumbnailsParams& thumbnail_params,

src/slic3r/GUI/GUI_App.cpp

@@ -85,7 +85,6 @@
 #include "libslic3r/Model.hpp"
 #include "libslic3r/I18N.hpp"
 #include "libslic3r/PresetBundle.hpp"
-#include "libslic3r/InstanceLock.hpp"
 #include "libslic3r/Thread.hpp"
 #include "libslic3r/miniz_extension.hpp"
 #include "libslic3r/Utils.hpp"
@@ -3571,7 +3570,7 @@ bool GUI_App::on_init_inner()
             update_publish_status();
         }
 
-        if (m_post_initialized && app_config->dirty() && app_config->save_due())
+        if (m_post_initialized && app_config->dirty())
             app_config->save();
 
     });
@@ -7665,11 +7664,8 @@ void GUI_App::start_sync_user_preset(bool with_progress_dlg)
 
                             // Delete the bundle folder and bundle
                             fs::path bundle_folder = fs::path(bundle.path.c_str()).parent_path();
-                            {
                             boost::system::error_code ec;
-                                InstanceLock instance_lock(user_presets_lock_path());
                             boost::filesystem::remove_all(bundle_folder, ec);
-                            }
 
                             preset_bundle->bundles.WriteLock();
                             preset_bundle->bundles.m_bundles.erase(bundle.id);
@@ -8956,7 +8952,6 @@ void GUI_App::preset_deleted_from_cloud(std::string setting_id)
 
     // Delete the .info file after cloud deletion is confirmed
     if (!preset_file_path.empty() && fs::exists(fs::path(preset_file_path))) {
-        InstanceLock instance_lock(user_presets_lock_path());
         boost::nowide::remove(preset_file_path.c_str());
         BOOST_LOG_TRIVIAL(info) << "Deleted .info file after cloud confirmation: " << preset_file_path;
     }
@@ -9019,21 +9014,17 @@ void GUI_App::scan_orphaned_info_files()
             fs::path preset_file = info_file;
             preset_file.replace_extension(".json");
 
-            // If .json doesn't exist, .info is orphaned. Read under the lock, so a
-            // remove_files() in another instance is seen whole or not at all; the
-            // delete queue's own mutex is taken after the lock is released.
-            std::string setting_id;
-            {
-                InstanceLock instance_lock(user_presets_lock_path());
-                if (!fs::exists(preset_file))
-                    setting_id = extract_setting_id_from_info(info_file.string());
-            }
+            // If .json doesn't exist, .info is orphaned
+            if (!fs::exists(preset_file)) {
+                // Extract setting_id from .info file
+                std::string setting_id = extract_setting_id_from_info(info_file.string());
                 if (!setting_id.empty()) {
                     // Add to need_delete_presets
                     delete_preset_from_cloud(setting_id, info_file.string());
                     BOOST_LOG_TRIVIAL(info) << "Found orphaned .info file on startup: " << info_file.string();
                 }
             }
+        }
         if (ec)
             BOOST_LOG_TRIVIAL(warning) << "scan_orphaned_info_files: failed to scan " << type_dir.string() << ": " << ec.message();
     }

src/slic3r/GUI/WebGuideDialog.cpp

@@ -1478,7 +1478,9 @@ bool GuideFrame::BuildProfileDataFromVendors()
             return false;
 
         // Written through a temp file and moved into place, as the preset caches
-        // are: half a cache must never be readable.
+        // are: half a cache must never be readable, and the PID suffix keeps two
+        // instances from interleaving on one temp file.
+        const std::string tmp_path = cache_file.string() + "." + std::to_string(get_current_pid()) + ".tmp";
         try {
             json out;
             out["format"]  = 1;
@@ -1487,9 +1489,18 @@ bool GuideFrame::BuildProfileDataFromVendors()
             for (const char* key : { "model", "machine", "filament", "process" })
                 profile[key] = m_ProfileJson[key];
             boost::filesystem::create_directories(cache_file.parent_path());
-            if (const std::error_code ec = write_file_atomically(cache_file.string(), out.dump(-1, ' ', false, json::error_handler_t::ignore), /*binary=*/true))
+            {
+                boost::nowide::ofstream ofs(tmp_path, std::ios::binary | std::ios::trunc);
+                ofs << out.dump(-1, ' ', false, json::error_handler_t::ignore);
+                ofs.close();
+                if (! ofs.good())
+                    throw std::runtime_error("write failed");
+            }
+            if (const std::error_code ec = rename_file(tmp_path, cache_file.string()))
                 throw std::runtime_error(ec.message());
         } catch (const std::exception& e) {
+            boost::system::error_code rm;
+            boost::filesystem::remove(tmp_path, rm);
             BOOST_LOG_TRIVIAL(warning) << "GuideFrame: could not write the profile data cache: " << e.what();
         }
         return true;

src/slic3r/Utils/3DPrinterOS.cpp

@@ -2,7 +2,6 @@
 
 #include <algorithm>
 #include <sstream>
-#include <system_error>
 #include <exception>
 #include <boost/format.hpp>
 #include <boost/log/trivial.hpp>
@@ -581,10 +580,9 @@ bool C3DPrinterOS::save_api_session(const std::string &session, const std::strin
     j.put("session", session);
     j.put("email", email);
     try {
-        std::ostringstream json;
-        pt::write_json(json, j);
-        if (const std::error_code ec = write_file_atomically(m_api_session_file_path, json.str()))
-            throw std::system_error(ec);
+        auto temp_path = m_api_session_file_path + ".tmp";
+        pt::write_json(temp_path, j);
+        boost::filesystem::rename(temp_path, m_api_session_file_path);
     } catch (const std::exception &err) {
         BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to write json to file. Path = "
                                  << m_api_session_file_path

src/slic3r/Utils/OrcaCloudServiceAgent.cpp

@@ -1475,8 +1475,15 @@ void OrcaCloudServiceAgent::save_sync_state()
     if (sync_state_path.empty())
         return;
 
-    if (const std::error_code ec = write_file_atomically(sync_state_path, std::to_string(sync_state.last_sync_timestamp)))
-        BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: failed to save the sync state: " << ec.message();
+    try {
+        std::string tmp_path = sync_state_path + ".tmp";
+        std::ofstream ofs(tmp_path, std::ios::out | std::ios::trunc);
+        if (ofs.good()) {
+            ofs << std::to_string(sync_state.last_sync_timestamp);
+            ofs.close();
+            boost::filesystem::rename(tmp_path, sync_state_path);
+        }
+    } catch (...) {}
 }
 
 void OrcaCloudServiceAgent::clear_sync_state()
@@ -1565,10 +1572,22 @@ void OrcaCloudServiceAgent::persist_user_secret(const std::string& secret)
             wxFileName::Mkdir(path.GetPath(), wxS_DIR_DEFAULT, wxPATH_MKDIR_FULL);
         }
 
-        if (const std::error_code ec = write_file_atomically(secret_fallback_path, signed_payload, /*binary=*/true))
-            BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: cannot write user secret file " << secret_fallback_path << ": " << ec.message();
-        else
+        const std::string tmp_path = secret_fallback_path + ".tmp";
+        std::ofstream ofs(tmp_path, std::ios::out | std::ios::trunc | std::ios::binary);
+        if (ofs.good()) {
+            ofs << signed_payload;
+            ofs.flush();
+            ofs.close();
+
+            if (wxRenameFile(wxString::FromUTF8(tmp_path.c_str()), wxString::FromUTF8(secret_fallback_path.c_str()), true)) {
                 stored = true;
+            } else {
+                wxRemoveFile(wxString::FromUTF8(tmp_path.c_str()));
+                BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: failed to atomically replace user secret file";
+            }
+        } else {
+            BOOST_LOG_TRIVIAL(warning) << "OrcaCloudServiceAgent: cannot open user secret file for write - " << secret_fallback_path;
+        }
     } else {
         // Use wxSecretStore only
         wxSecretStore store = wxSecretStore::GetDefault();

src/slic3r/plugin/PluginConfig.cpp

@@ -249,10 +249,21 @@ bool PluginConfig::save()
         return false;
     }
 
-    // Written beside the target and moved into place, so a crash mid-write cannot truncate an
-    // existing config.
-    if (const std::error_code ec = write_file_atomically(path, root.dump(1, '\t') + "\n")) {
-        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to write " << path << ": " << ec.message() << "; keeping the existing config";
+    // Write to a PID-suffixed file and rename it into place, so a crash mid-write cannot truncate an
+    // existing config. Same approach as AppConfig::save().
+    const std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
+
+    boost::nowide::ofstream file;
+    file.open(path_pid, std::ios::out | std::ios::trunc);
+    file << root.dump(1, '\t') << std::endl;
+    file.close();
+    if (file.fail()) {
+        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to write " << path_pid << "; keeping the existing config";
+        return false;
+    }
+
+    if (const std::error_code rename_ec = rename_file(path_pid, path)) {
+        BOOST_LOG_TRIVIAL(error) << "PluginConfig: failed to move " << path_pid << " onto " << path << ": " << rename_ec.message();
         return false;
     }
 

tests/cli/CMakeLists.txt

@@ -11,6 +11,13 @@ endif ()
 
 add_test(NAME cli_strict_mode
     COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cli_strict.sh $<TARGET_FILE:OrcaSlicer> ${ORCA_CLI_TEST_PYTHON})
+add_test(NAME cli_empty_project_config
+    COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cli_empty_project_config.sh $<TARGET_FILE:OrcaSlicer> ${ORCA_CLI_TEST_PYTHON} ${CMAKE_SOURCE_DIR})
+set_tests_properties(cli_empty_project_config PROPERTIES
+    LABELS "CLI;RequiresApp"
+    SKIP_RETURN_CODE 77
+    TIMEOUT 300)
+
 set_tests_properties(cli_strict_mode PROPERTIES
     LABELS "CLI;RequiresApp"
     SKIP_RETURN_CODE 77

tests/cli/test_cli_empty_project_config.sh

@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+# Regression check: a 3mf whose Metadata/project_settings.config carries no settings must load.
+#
+# The CLI reads printable_height out of the project config with opt_float(), which dereferences
+# what option<>() returns. With create = false that is null when the key is absent, so a project
+# saved without settings used to take the CLI down with a segfault. Both models in
+# resources/handy_models are such files.
+#
+# usage: test_cli_empty_project_config.sh <orca-slicer binary> <python3> <source dir>
+set -u
+
+BIN="${1:-}"
+PY="${2:-python3}"
+SRC="${3:-}"
+# 77 is the test's SKIP_RETURN_CODE.
+[ -x "$BIN" ] || { echo "SKIP: orca-slicer binary not found: $BIN"; exit 77; }
+[ -f "$SRC/resources/handy_models/OrcaBadge.3mf" ] || { echo "SKIP: handy model not found"; exit 77; }
+
+WORK="$(mktemp -d "${TMPDIR:-/tmp}/orca-cli-emptycfg.XXXXXX")"
+trap 'rm -rf "$WORK"' EXIT
+mkdir -p "$WORK/datadir"
+
+# Rewrite the project settings to an empty object, so the test holds no matter what the shipped
+# models carry later on.
+cp "$SRC/resources/handy_models/OrcaBadge.3mf" "$WORK/empty_config.3mf"
+"$PY" - "$WORK/empty_config.3mf" <<'PYEOF'
+import shutil, sys, zipfile
+
+path = sys.argv[1]
+entry = "Metadata/project_settings.config"
+with zipfile.ZipFile(path) as src:
+    items = [(i, src.read(i.filename)) for i in src.infolist()]
+with zipfile.ZipFile(path + ".new", "w", zipfile.ZIP_DEFLATED) as dst:
+    seen = False
+    for info, data in items:
+        if info.filename == entry:
+            data, seen = b"{\n}\n", True
+        dst.writestr(info, data)
+    if not seen:
+        dst.writestr(entry, b"{\n}\n")
+shutil.move(path + ".new", path)
+PYEOF
+
+"$BIN" --datadir "$WORK/datadir" --info "$WORK/empty_config.3mf" > "$WORK/info.txt" 2>&1
+rc=$?
+if [ $rc -ne 0 ]; then
+    echo "FAIL: --info on a project with empty settings exited $rc"
+    tail -20 "$WORK/info.txt"
+    exit 1
+fi
+grep -q "size_x" "$WORK/info.txt" || { echo "FAIL: --info printed no geometry"; cat "$WORK/info.txt"; exit 1; }
+echo "PASS: a project with empty settings loads"

tests/libslic3r/CMakeLists.txt

@@ -56,7 +56,6 @@ add_executable(${_TEST_NAME}_tests
     test_ordering_strategies.cpp
     # test_png_io.cpp
     test_indexed_triangle_set.cpp
-    test_instance_lock.cpp
     ../libnest2d/printer_parts.cpp
     )
 

tests/libslic3r/test_instance_lock.cpp

@@ -1,228 +0,0 @@
-#include <catch2/catch_all.hpp>
-
-#include <atomic>
-#include <chrono>
-#include <thread>
-
-#include <boost/filesystem.hpp>
-
-#include "libslic3r/InstanceLock.hpp"
-#include "test_utils.hpp"
-
-#ifdef _WIN32
-#include <boost/interprocess/sync/file_lock.hpp>
-#include <boost/nowide/convert.hpp>
-#include <boost/nowide/fstream.hpp>
-#else
-#include <fcntl.h>
-#include <sys/file.h>
-#include <unistd.h>
-#endif
-
-using namespace Slic3r;
-using namespace std::chrono_literals;
-
-// Sets a process-wide knob for one test and restores it however the test ends.
-template<typename T> struct ScopedStaticValue
-{
-    T &ref;
-    T  saved;
-    ScopedStaticValue(T &ref, T value) : ref(ref), saved(ref) { ref = value; }
-    ~ScopedStaticValue() { ref = saved; }
-};
-
-TEST_CASE("InstanceLock creates its lock file and holds it for the guard's scope", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    {
-        InstanceLock lock(path);
-        REQUIRE(lock.locked());
-        REQUIRE(boost::filesystem::exists(path));
-    }
-    // Released: a fresh guard gets the lock at once instead of waiting out a timeout.
-    const auto   started = std::chrono::steady_clock::now();
-    InstanceLock again(path, 5000ms);
-    REQUIRE(again.locked());
-    // Well inside the timeout it would otherwise have waited out; loose enough for a loaded runner.
-    REQUIRE(std::chrono::steady_clock::now() - started < 4000ms);
-}
-
-TEST_CASE("InstanceLock nests within one thread", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    InstanceLock outer(path);
-    {
-        InstanceLock inner(path, 100ms);
-        REQUIRE(inner.locked());
-    }
-    // The inner guard leaving does not release the outer one.
-    REQUIRE(outer.locked());
-}
-
-TEST_CASE("InstanceLock is a no-op for an empty path and survives an unwritable one", "[InstanceLock]")
-{
-    ScopedTemporaryDir dir;
-
-    InstanceLock none("");
-    REQUIRE_FALSE(none.locked());
-
-    // The directory does not exist, so the lock file cannot be created; the
-    // guard still constructs and the write it guards can go ahead.
-    InstanceLock unwritable((dir.path() / "missing" / "shared.lock").string(), 100ms);
-    REQUIRE_FALSE(unwritable.locked());
-}
-
-TEST_CASE("InstanceLock retries a lock file it could not open once the cool-down passes", "[InstanceLock]")
-{
-    ScopedTemporaryDir dir;
-    const std::string  path = (dir.path() / "later" / "shared.lock").string();
-    ScopedStaticValue  cooldown(InstanceLock::cooldown, 300ms);
-
-    bool before_dir, during_cooldown, after_cooldown;
-    const auto started = std::chrono::steady_clock::now();
-    {
-        InstanceLock lock(path, 100ms);
-        before_dir = lock.locked();
-    }
-    boost::filesystem::create_directories(dir.path() / "later");
-    {
-        InstanceLock lock(path, 100ms);
-        during_cooldown = lock.locked();
-    }
-    const bool second_guard_inside_cooldown = std::chrono::steady_clock::now() - started < InstanceLock::cooldown;
-    std::this_thread::sleep_for(400ms);
-    {
-        InstanceLock lock(path, 100ms);
-        after_cooldown = lock.locked();
-    }
-
-    REQUIRE_FALSE(before_dir);
-    // A loaded runner may take longer than the cool-down to get here; then the
-    // second guard legitimately retried, so only assert when the timing held.
-    if (second_guard_inside_cooldown)
-        REQUIRE_FALSE(during_cooldown);
-    REQUIRE(after_cooldown);
-}
-
-TEST_CASE("InstanceLock reopens a lock file that was replaced on disk", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-    {
-        InstanceLock lock(path);
-        REQUIRE(lock.locked());
-    }
-
-    boost::filesystem::remove(path);
-    InstanceLock lock(path);
-    REQUIRE(lock.locked());
-    // Each outermost guard opens the file afresh, so the deleted path is back.
-    REQUIRE(boost::filesystem::exists(path));
-}
-
-TEST_CASE("InstanceLock serialises the threads of one process", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-
-    std::atomic<bool> holder_ready{false};
-    std::atomic<bool> holder_released{false};
-    std::thread holder([&] {
-        InstanceLock lock(path);
-        holder_ready = true;
-        std::this_thread::sleep_for(150ms);
-        holder_released = true;
-    });
-    while (! holder_ready)
-        std::this_thread::yield();
-
-    bool released_before_acquire = false;
-    {
-        InstanceLock lock(path);
-        released_before_acquire = holder_released;
-    }
-    holder.join();
-    REQUIRE(released_before_acquire);
-}
-
-// Holds the OS lock on a file through a handle of its own, as another instance would. The lock
-// belongs to the handle on Windows and to the open file description elsewhere, so the guard's
-// handle is refused while this one holds it.
-class OtherHolder
-{
-public:
-    explicit OtherHolder(const std::string &path)
-    {
-#ifdef _WIN32
-        boost::nowide::ofstream(path, std::ios::app).close();
-        m_lock = boost::interprocess::file_lock(boost::nowide::widen(path).c_str());
-        m_held = m_lock.try_lock();
-#else
-        m_fd   = ::open(path.c_str(), O_RDWR | O_CREAT, 0644);
-        m_held = m_fd >= 0 && ::flock(m_fd, LOCK_EX | LOCK_NB) == 0;
-#endif
-    }
-    ~OtherHolder() { release(); }
-    OtherHolder(const OtherHolder &) = delete;
-    OtherHolder &operator=(const OtherHolder &) = delete;
-
-    bool held() const { return m_held; }
-    void release()
-    {
-#ifdef _WIN32
-        if (m_held)
-            m_lock.unlock();
-        m_lock = boost::interprocess::file_lock();
-#else
-        if (m_fd >= 0)
-            ::close(m_fd);
-        m_fd = -1;
-#endif
-        m_held = false;
-    }
-
-private:
-#ifdef _WIN32
-    boost::interprocess::file_lock m_lock;
-#else
-    int m_fd{-1};
-#endif
-    bool m_held{false};
-};
-
-TEST_CASE("InstanceLock yields to a lock held through another handle and reports it", "[InstanceLock]")
-{
-    ScopedTemporaryFile lock_file(".lock");
-    const std::string   path = lock_file.string();
-    ScopedStaticValue   cooldown(InstanceLock::cooldown, 300ms);
-
-    OtherHolder other(path);
-    REQUIRE(other.held());
-
-    bool locked_while_other_holds;
-    {
-        InstanceLock lock(path, 100ms);
-        locked_while_other_holds = lock.locked();
-    }
-    // The timed-out wait starts a cool-down: the next guard does not touch the file.
-    const auto started = std::chrono::steady_clock::now();
-    bool       locked_during_cooldown;
-    {
-        InstanceLock lock(path, 5000ms);
-        locked_during_cooldown = lock.locked();
-    }
-    const auto cooldown_wait = std::chrono::steady_clock::now() - started;
-    other.release();
-
-    REQUIRE_FALSE(locked_while_other_holds);
-    REQUIRE_FALSE(locked_during_cooldown);
-    REQUIRE(cooldown_wait < 4000ms);
-    // Once the cool-down passes, the lock the other holder released is taken again.
-    std::this_thread::sleep_for(400ms);
-    InstanceLock lock(path);
-    REQUIRE(lock.locked());
-}

tests/libslic3r/test_preset_bundle_loading.cpp

@@ -5,7 +5,6 @@
 #include <fstream>
 
 #include "libslic3r/PresetBundle.hpp"
-#include "libslic3r/ParallelResolve.hpp"
 #include "libslic3r/AppConfig.hpp"
 #include "libslic3r/Model.hpp"
 #include "libslic3r/TriangleMesh.hpp"
@@ -18,10 +17,6 @@
 #include <iostream>
 #include <initializer_list>
 
-#ifndef _WIN32
-#include <unistd.h> // geteuid
-#endif
-
 using namespace Slic3r;
 
 namespace {
@@ -5689,214 +5684,3 @@ TEST_CASE("A project saved with pressure advance per filament applies it to ever
     check_double_vector(pla.opt<ConfigOptionFloatsNullable>("filament_flow_ratio")->values, { 0.95, 0.96 });
     check_double_vector(petg.opt<ConfigOptionFloatsNullable>("filament_flow_ratio")->values, { 0.97 });
 }
-
-TEST_CASE("A user preset saved over by another instance while its directory loads is read again under the lock", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.config.option<ConfigOptionFloat>("layer_height", true)->value = 0.24;
-    parent.is_system = true;
-
-    // Fewer than one batch, so every file is read before the first one is committed.
-    constexpr int  children = 8;
-    static_assert(children <= int(resolve_batch_size));
-    const fs::path dir      = temp_dir.path() / PRESET_PRINT_NAME;
-    for (int i = 0; i < children; ++ i)
-        write_minimal_child(dir / ("Child " + std::to_string(i) + ".json"), "Child " + std::to_string(i), "Parent Process");
-
-    // The first commit stands in for another instance saving every other preset after
-    // they were read and before they are committed.
-    std::string first;
-    auto save_the_others = [&](Preset &preset) {
-        if (! first.empty())
-            return;
-        first = preset.name;
-        for (int i = 0; i < children; ++ i) {
-            const std::string name = "Child " + std::to_string(i);
-            if (name != first)
-                std::ofstream((dir / (name + ".json")).string())
-                    << R"({"type":"process","name":")" << name
-                    << R"(","from":"User","version":"1.0.0","inherits":"Parent Process","layer_height":"0.3"})";
-        }
-    };
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable, save_the_others);
-
-    REQUIRE_FALSE(first.empty());
-    CHECK(coll.error_count() == 0);
-    for (int i = 0; i < children; ++ i) {
-        const std::string name  = "Child " + std::to_string(i);
-        const Preset     *child = coll.find_preset(name);
-        REQUIRE(child != nullptr);
-        CHECK_THAT(child->config.opt_float("layer_height"), Catch::Matchers::WithinAbs(name == first ? 0.24 : 0.3, 1e-9));
-    }
-}
-
-TEST_CASE("A user preset removed by another instance while its directory loads is not installed", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-
-    // Fewer than one batch, so every file is read before the first one is committed.
-    constexpr int  children = 8;
-    static_assert(children <= int(resolve_batch_size));
-    const fs::path dir      = temp_dir.path() / PRESET_PRINT_NAME;
-    for (int i = 0; i < children; ++ i)
-        write_minimal_child(dir / ("Child " + std::to_string(i) + ".json"), "Child " + std::to_string(i), "Parent Process");
-
-    std::string first;
-    auto remove_the_others = [&](Preset &preset) {
-        if (! first.empty())
-            return;
-        first = preset.name;
-        for (int i = 0; i < children; ++ i)
-            if (const std::string name = "Child " + std::to_string(i); name != first)
-                fs::remove(dir / (name + ".json"));
-    };
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable, remove_the_others);
-
-    REQUIRE_FALSE(first.empty());
-    CHECK(coll.error_count() == 0);
-    CHECK(coll.size() == 3); // the default preset, the parent and the first child
-    CHECK(coll.find_preset(first) != nullptr);
-}
-
-TEST_CASE("Without the instance lock a user preset loads but its file is not written back", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir temp_dir;
-    ScopedDataDir      data_dir_scope(temp_dir.path());
-    // A directory where the lock file belongs, so the lock cannot be taken.
-    fs::create_directories(fs::path(user_presets_lock_path()));
-    PresetBundle       bundle;
-    const fs::path     file = temp_dir.path() / PRESET_FILAMENT_NAME / "My PLA @Test Printer.json";
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-    const std::string before = read_file(file);
-
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == std::vector<std::string>{ "Test Printer" });
-    CHECK(read_file(file) == before);
-}
-
-#ifndef _WIN32
-// File permissions stop reading only on POSIX.
-TEST_CASE("A user preset that cannot be read under the instance lock is counted and removed", "[Preset][Bundle][InstanceLock]")
-{
-    if (::geteuid() == 0)
-        SKIP("file permissions do not stop root");
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-    const fs::path file = temp_dir.path() / PRESET_PRINT_NAME / "Unreadable.json";
-    write_minimal_child(file, "Unreadable", "Parent Process");
-    fs::permissions(file, fs::no_perms);
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable);
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Unreadable") == nullptr);
-    CHECK_FALSE(fs::exists(file));
-}
-#endif
-
-#ifndef _WIN32
-// Creating a symlink needs no privilege only on POSIX.
-TEST_CASE("A user preset that is a symlink to itself is counted and removed while the rest still load", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    Preset &parent = add_inmemory_preset(coll, "Parent Process");
-    parent.is_system = true;
-    const fs::path dir = temp_dir.path() / PRESET_PRINT_NAME;
-    write_minimal_child(dir / "Good A.json", "Good A", "Parent Process");
-    write_minimal_child(dir / "Good B.json", "Good B", "Parent Process");
-    const fs::path loop = dir / "Loop.json";
-    fs::create_symlink(loop.filename(), loop);
-
-    PresetsConfigSubstitutions substitutions;
-    REQUIRE_NOTHROW(coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                                      ForwardCompatibilitySubstitutionRule::Disable));
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Good A") != nullptr);
-    CHECK(coll.find_preset("Good B") != nullptr);
-    CHECK(coll.find_preset("Loop") == nullptr);
-    boost::system::error_code ec;
-    CHECK(fs::symlink_status(loop, ec).type() == fs::file_not_found);
-}
-#endif
-
-#ifndef _WIN32
-// Creating a symlink needs no privilege only on POSIX.
-TEST_CASE("A user preset symlinked to a missing target is counted but the link is kept", "[Preset][Bundle][InstanceLock]")
-{
-    ScopedTemporaryDir   temp_dir;
-    ScopedDataDir        data_dir_scope(temp_dir.path());
-    RenameTestCollection coll;
-
-    const fs::path dir  = temp_dir.path() / PRESET_PRINT_NAME;
-    const fs::path link = dir / "Linked.json";
-    fs::create_directories(dir);
-    fs::create_symlink(temp_dir.path() / "unmounted" / "Linked.json", link);
-
-    PresetsConfigSubstitutions substitutions;
-    coll.load_presets(temp_dir.path().string(), PRESET_PRINT_NAME, substitutions,
-                      ForwardCompatibilitySubstitutionRule::Disable);
-
-    CHECK(coll.error_count() == 1);
-    CHECK(coll.find_preset("Linked") == nullptr);
-    boost::system::error_code ec;
-    CHECK(fs::symlink_status(link, ec).type() == fs::symlink_file);
-}
-#endif
-
-#ifndef _WIN32
-// The read-only bit on a directory stops file creation only on POSIX.
-TEST_CASE("A user filament whose derived compatible printer cannot be written back still loads with the failure counted", "[Preset][Bundle]")
-{
-    if (::geteuid() == 0)
-        SKIP("a read-only directory does not stop root");
-    ScopedTemporaryDir temp_dir;
-    PresetBundle       bundle;
-    const fs::path     dir  = temp_dir.path() / PRESET_FILAMENT_NAME;
-    const fs::path     file = dir / "My PLA @Test Printer.json";
-    write_preset_with_inherits(bundle.filaments.default_preset().config, file, "My PLA @Test Printer", std::string());
-    const std::string before = read_file(file);
-
-    fs::permissions(file, fs::owner_read);
-    fs::permissions(dir, fs::owner_read | fs::owner_exe);
-    PresetsConfigSubstitutions substitutions;
-    bundle.filaments.load_presets(temp_dir.path().string(), PRESET_FILAMENT_NAME, substitutions,
-                                  ForwardCompatibilitySubstitutionRule::EnableSilent);
-    // Restored before any assertion, so a failure never leaves an unremovable directory behind.
-    fs::permissions(dir, fs::owner_all);
-    fs::permissions(file, fs::owner_read | fs::owner_write);
-
-    const Preset *preset = bundle.filaments.find_preset("My PLA @Test Printer");
-    REQUIRE(preset != nullptr);
-    CHECK(preset->config.option<ConfigOptionStrings>("compatible_printers")->values == std::vector<std::string>{ "Test Printer" });
-    CHECK(bundle.filaments.error_count() == 1);
-    CHECK(read_file(file) == before);
-}
-#endif

tests/libslic3r/test_utils.cpp

@@ -10,8 +10,6 @@
 #include <cctype>
 #include <fstream>
 #include <string>
-#include <thread>
-#include <system_error>
 
 #ifndef _WIN32
 #include <unistd.h>     // getuid
@@ -64,113 +62,6 @@ TEST_CASE("per-user temp root is unchanged on Windows, isolated elsewhere", "[ut
 #endif
 }
 
-TEST_CASE("write_file_atomically replaces the target and leaves no temporary file", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "preset.json";
-
-    REQUIRE_FALSE(write_file_atomically(target.string(), "first"));
-    REQUIRE_FALSE(write_file_atomically(target.string(), "second"));
-
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "second");
-    size_t entries = 0;
-    for (auto &entry : boost::filesystem::directory_iterator(dir.path())) {
-        (void) entry;
-        ++entries;
-    }
-    REQUIRE(entries == 1);
-}
-
-TEST_CASE("write_file_atomically reports a missing directory and writes nothing", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "missing" / "preset.json";
-
-    const std::error_code ec = write_file_atomically(target.string(), "x");
-    REQUIRE(ec == std::errc::no_such_file_or_directory);
-    REQUIRE_FALSE(boost::filesystem::exists(target));
-}
-
-#ifndef _WIN32
-// The read-only bit on a directory stops file creation only on POSIX.
-TEST_CASE("write_file_atomically writes in place when no temporary can be created beside an existing target", "[utils]") {
-    if (::geteuid() == 0)
-        SKIP("a read-only directory does not stop root");
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "preset.json";
-    REQUIRE_FALSE(write_file_atomically(target.string(), "first"));
-    boost::filesystem::permissions(dir.path(), boost::filesystem::owner_read | boost::filesystem::owner_exe);
-
-    const std::error_code replaced = write_file_atomically(target.string(), "second");
-    const std::error_code created  = write_file_atomically((dir.path() / "new.json").string(), "x");
-    // Restored before any assertion, so a failure never leaves an unremovable directory behind.
-    boost::filesystem::permissions(dir.path(), boost::filesystem::owner_all);
-
-    REQUIRE_FALSE(replaced);
-    REQUIRE(created == std::errc::permission_denied);
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "second");
-}
-#endif
-
-TEST_CASE("write_file_atomically keeps bytes intact in binary mode", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "blob.bin";
-    const std::string bytes("a\r\nb\0c", 6);
-
-    REQUIRE_FALSE(write_file_atomically(target.string(), bytes, /*binary=*/true));
-    REQUIRE(boost::filesystem::file_size(target) == bytes.size());
-}
-
-#ifndef _WIN32
-TEST_CASE("write_file_atomically writes through a symlink and keeps the target's permissions", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path real = dir.path() / "real.json";
-    const boost::filesystem::path link = dir.path() / "link.json";
-    REQUIRE_FALSE(write_file_atomically(real.string(), "first"));
-    boost::filesystem::permissions(real, boost::filesystem::owner_read | boost::filesystem::owner_write);
-    boost::filesystem::create_symlink(real, link);
-
-    REQUIRE_FALSE(write_file_atomically(link.string(), "second"));
-
-    REQUIRE(boost::filesystem::is_symlink(boost::filesystem::symlink_status(link)));
-    std::string content;
-    load_string_file(real, content);
-    REQUIRE(content == "second");
-
-    REQUIRE_FALSE(write_file_atomically(real.string(), "third"));
-    const auto perms = boost::filesystem::status(real).permissions() & boost::filesystem::all_all;
-    REQUIRE(perms == (boost::filesystem::owner_read | boost::filesystem::owner_write));
-}
-#endif
-
-TEST_CASE("write_file_atomically survives two threads writing one target", "[utils]") {
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "shared.json";
-    const std::string a(20000, 'a'), b(20000, 'b');
-
-    std::thread other([&] {
-        for (int i = 0; i < 50; ++i)
-            write_file_atomically(target.string(), a);
-    });
-    for (int i = 0; i < 50; ++i)
-        write_file_atomically(target.string(), b);
-    other.join();
-
-    std::string content;
-    load_string_file(target, content);
-    const bool whole = content == a || content == b;
-    REQUIRE(whole);
-    // No temporary may be left; a scanner on Windows may briefly hold the old
-    // file under another name, so only the temporaries are counted.
-    size_t temporaries = 0;
-    for (auto &entry : boost::filesystem::directory_iterator(dir.path()))
-        if (entry.path().extension() == ".tmp")
-            ++temporaries;
-    REQUIRE(temporaries == 0);
-}
-
 TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") {
     ScopedTemporaryFile source(".txt");
     {

tests/libslic3r/test_vendor_cache.cpp

@@ -1804,6 +1804,24 @@ TEST_CASE("a header claiming more body than the file holds is rejected", "[Vendo
     REQUIRE_FALSE(bundle.load_vendor_cache(cache, "Bounded", Semver(1, 0, 0)));
 }
 
+TEST_CASE("a failed write leaves the previous cache in place", "[VendorCache]")
+{
+    TempDir tmp;
+    const std::string cache = (tmp.path / "Durable.opc").string();
+    REQUIRE(save_one_vendor(cache, one_vendor("Durable"), "Durable", "1.0.0"));
+    const std::string before = slurp(cache);
+
+    // A directory where the temp file wants to go: the write cannot complete,
+    // and must not have destroyed what was already there to find that out.
+    const fs::path blocker = fs::path(cache + "." + std::to_string(get_current_pid()) + ".tmp");
+    fs::create_directories(blocker);
+
+    REQUIRE_FALSE(save_one_vendor(cache, one_vendor("Durable"), "Durable", "2.0.0"));
+    CHECK(slurp(cache) == before);
+
+    fs::remove_all(blocker);
+}
+
 TEST_CASE("a cache written by another build's option ordering still loads", "[VendorCache]")
 {
     // The regression the fingerprint used to prevent by refusing the file