Keep a Save From Being Lost to an Open Reader and Cap the Lock Wait

Windows refuses to replace a file that another process holds open without
FILE_SHARE_DELETE, which is how the C runtime opens files for reading, so
the atomic write could fail against a concurrent reader and drop the save
where the old in-place write had succeeded. Lock the readers that were
still outside the guard, Preset::reload() and the physical printer loader
and writer, and when the rename still fails that way, log it and write in
place as before; losing the save is worse than a torn read. Report the OS
error from a failed write instead of a generic I/O error.

Each leaf guard paid the full two-second wait on its own, so a bulk save
against an instance holding the lock for a long scan stalled once per
preset while holding the preset collection mutex. After a timed-out wait
the same lock file is not waited on again for ten seconds. Read-only scans
never rewrite or delete and many CLI jobs may share one data dir, so they
take no lock and no longer queue behind each other or log about writing.

Take the bundle metadata guard beside the write rather than while the JSON
is built, state the lock-order rule in the header, and give the tests a
temporary file rather than a directory, since the process keeps the lock
file open and a directory holding it cannot be removed on Windows.

src/libslic3r/AppConfig.cpp

@@ -5,6 +5,7 @@
 //BBS
 #include "Preset.hpp"
 #include "Exception.hpp"
+#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Thread.hpp"
 #include "format.hpp"
@@ -734,6 +735,9 @@ std::string AppConfig::load()
 {
     json j;
 
+    // Keep another instance from replacing or restoring the file mid-read.
+    InstanceLock instance_lock(lock_path());
+
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -983,6 +987,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();
+    InstanceLock instance_lock(lock_path());
     std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     json j;
@@ -1142,7 +1147,8 @@ void AppConfig::save()
     // 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);
+    if (const std::error_code ec = rename_file(path_pid, path))
+        BOOST_LOG_TRIVIAL(error) << "Failed to replace " << path << " with the new configuration: " << ec.message();
     m_dirty = false;
 }
 
@@ -1150,6 +1156,9 @@ void AppConfig::save()
 
 std::string AppConfig::load()
 {
+    // Keep another instance from replacing or restoring the file mid-read.
+    InstanceLock instance_lock(lock_path());
+
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
     pt::ptree tree;
@@ -1287,6 +1296,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();
+    InstanceLock instance_lock(lock_path());
     std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     std::stringstream config_ss;
@@ -1351,7 +1361,8 @@ void AppConfig::save()
     // 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);
+    if (const std::error_code ec = rename_file(path_pid, path))
+        BOOST_LOG_TRIVIAL(error) << "Failed to replace " << path << " with the new configuration: " << ec.message();
     m_dirty = false;
 }
 #endif
@@ -1844,6 +1855,11 @@ 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

src/libslic3r/AppConfig.hpp

@@ -338,6 +338,8 @@ 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; }

src/libslic3r/CMakeLists.txt

@@ -304,6 +304,8 @@ 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

@@ -1522,12 +1522,10 @@ 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);
-    boost::nowide::ofstream c;
+    if (const std::error_code ec = write_file_atomically(file, ss.str()))
-    c.open(file, std::ios::out | std::ios::trunc);
+        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(": failed to save config to %1%: %2%") % file % ec.message();
-    c << ss.str();
+    else
-    c.close();
+        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ":" <<__LINE__ << boost::format(", saved config to %1%\n")%file;
-
-    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ":" <<__LINE__ << boost::format(", saved config to %1%\n")%file;
 }
 
 void ConfigBase::save_to_json(std::ostream &os, const std::string &name, const std::string &from, const std::string &version, bool replace_invalid_utf8) const

src/libslic3r/InstanceLock.cpp

@@ -0,0 +1,109 @@
+#include "InstanceLock.hpp"
+
+#include <map>
+#include <memory>
+#include <mutex>
+#include <thread>
+
+#include <boost/interprocess/sync/file_lock.hpp>
+#include <boost/log/trivial.hpp>
+#include <boost/nowide/fstream.hpp>
+#ifdef _WIN32
+#include <boost/nowide/convert.hpp>
+#endif
+
+namespace Slic3r {
+
+// One slot per lock file, shared by every guard in the process. A POSIX fcntl
+// lock is owned by the process, not the thread or descriptor, so a single
+// file_lock 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.
+struct InstanceLock::Slot
+{
+    std::recursive_mutex mutex;
+    // Absent when the lock file could not be created or opened.
+    std::unique_ptr<boost::interprocess::file_lock> file_lock;
+    int  depth{0};
+    bool file_locked{false};
+    // After a timed-out wait, guards skip waiting until this point.
+    std::chrono::steady_clock::time_point skip_waiting_until{};
+};
+
+InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path)
+{
+    static std::mutex registry_mutex;
+    // Never freed: the slots keep the lock files open for as long as anything
+    // in the process may still save, including during static destruction.
+    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>();
+        try {
+            // file_lock only opens existing files.
+            boost::nowide::ofstream(lock_file_path, std::ios::app).close();
+#ifdef _WIN32
+            slot->file_lock = std::make_unique<boost::interprocess::file_lock>(boost::nowide::widen(lock_file_path).c_str());
+#else
+            slot->file_lock = std::make_unique<boost::interprocess::file_lock>(lock_file_path.c_str());
+#endif
+        } catch (const std::exception &e) {
+            BOOST_LOG_TRIVIAL(warning) << "Cannot open lock file " << lock_file_path << ": " << e.what()
+                                       << "; other instances are not excluded from writing";
+        }
+    }
+    return *slot;
+}
+
+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->mutex.lock();
+    if (m_slot->depth++ == 0 && m_slot->file_lock) {
+        const auto now      = std::chrono::steady_clock::now();
+        const bool wait     = now >= m_slot->skip_waiting_until;
+        const auto deadline = now + timeout;
+        for (;;) {
+            try {
+                if (m_slot->file_lock->try_lock()) {
+                    m_slot->file_locked = true;
+                    break;
+                }
+            } catch (const std::exception &e) {
+                BOOST_LOG_TRIVIAL(warning) << "Cannot lock " << lock_file_path << ": " << e.what();
+                break;
+            }
+            if (! wait)
+                break;
+            if (std::chrono::steady_clock::now() >= deadline) {
+                m_slot->skip_waiting_until = deadline + cooldown;
+                BOOST_LOG_TRIVIAL(warning) << "Another instance has held " << lock_file_path << " for over "
+                                           << timeout.count() << " ms; proceeding without the lock for the next "
+                                           << cooldown.count() << " ms";
+                break;
+            }
+            std::this_thread::sleep_for(std::chrono::milliseconds(5));
+        }
+    }
+    m_locked = m_slot->file_locked;
+}
+
+InstanceLock::~InstanceLock()
+{
+    if (m_slot == nullptr)
+        return;
+    if (--m_slot->depth == 0 && m_slot->file_locked) {
+        try {
+            m_slot->file_lock->unlock();
+        } catch (const std::exception &e) {
+            BOOST_LOG_TRIVIAL(warning) << "Cannot unlock instance lock: " << e.what();
+        }
+        m_slot->file_locked = false;
+    }
+    m_slot->mutex.unlock();
+}
+
+} // namespace Slic3r

src/libslic3r/InstanceLock.hpp

@@ -0,0 +1,53 @@
+#pragma once
+
+#include <chrono>
+#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`, created on first use
+// and kept: the lock state lives in the kernel on the open file, so deleting
+// the file on release would let a third instance lock a fresh file while the
+// second still holds the old one. The OS releases the lock when its holder
+// exits, so a crashed instance never leaves a stale lock behind.
+//
+// The lock is best effort: when the lock file cannot be created, or another
+// instance still holds it after `timeout`, the guard keeps only the in-process
+// mutex and locked() reports false. Writes then proceed unprotected rather than
+// letting one hung instance block every other one from saving. After such a
+// timeout the same lock file is not waited on again for `cooldown`, so a batch
+// of saves pays the wait once rather than once per file.
+//
+// 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
+// batch-level guard around save_user_presets(), which takes the collection
+// mutex through delete_preset(), must not be added.
+class InstanceLock
+{
+public:
+    static constexpr std::chrono::milliseconds default_timeout{2000};
+    static constexpr 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);
+
+    Slot *m_slot{nullptr};
+    bool  m_locked{false};
+};
+
+} // namespace Slic3r

src/libslic3r/Preset.cpp

@@ -48,6 +48,9 @@
 
 #include "libslic3r.h"
 #include "Utils.hpp"
+#include "InstanceLock.hpp"
+
+#include <sstream>
 #include "Time.hpp"
 #include "PlaceholderParser.hpp"
 #include "libslic3r/GCode/Thumbnails.hpp"
@@ -104,6 +107,11 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const
     }
 }
 
+std::string user_presets_lock_path()
+{
+    return data_dir().empty() ? std::string() : (fs::path(data_dir()) / (PRESET_USER_DIR ".lock")).string();
+}
+
 std::string get_preset_bare_name(const std::string &canonical_name)
 {
     const auto pos = canonical_name.find_last_of('/');
@@ -603,6 +611,7 @@ Preset::Type Preset::get_type_from_string(std::string type_str)
 
 void Preset::load_info(const std::string& file)
 {
+    InstanceLock instance_lock(user_presets_lock_path());
     try {
         boost::property_tree::ptree tree;
         boost::nowide::ifstream ifs(file);
@@ -646,18 +655,20 @@ 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;
-    c.close();
+
+    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();
 }
 
 void Preset::remove_files(bool cloud_already_deleted)
@@ -666,6 +677,7 @@ 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);
@@ -702,6 +714,7 @@ void Preset::save(DynamicPrintConfig* parent_config)
     else
         from_str = std::string("Default");
 
+    InstanceLock instance_lock(user_presets_lock_path());
     boost::filesystem::create_directories(fs::path(this->file).parent_path());
     const std::string bare_name = get_preset_bare_name(this->name);
 
@@ -770,6 +783,7 @@ 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));
@@ -1704,6 +1718,10 @@ void PresetCollection::load_presets(
     std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
     Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
 
+    // Held across the scan so no instance replaces or removes a file mid-read.
+    // A read-only scan (the CLI) never rewrites or deletes, and many CLI jobs
+    // may share one data dir, so it does not queue behind the others.
+    InstanceLock instance_lock(read_only ? std::string() : user_presets_lock_path());
     //BBS: change to json format
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
     {
@@ -4203,6 +4221,7 @@ void PhysicalPrinter::update_preset_names_in_config()
 
 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;
@@ -4319,6 +4338,7 @@ void PhysicalPrinterCollection::load_printers(
     std::string errors_cummulative;
     // Store the loaded printers into a new vector, otherwise the binary search for already existing presets would be broken.
     std::deque<PhysicalPrinter> printers_loaded;
+    InstanceLock instance_lock(user_presets_lock_path());
     //BBS: change to json format
     for (auto& dir_entry : boost::filesystem::directory_iterator(dir))
     {

src/libslic3r/Preset.hpp

@@ -484,6 +484,10 @@ 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.
+std::string user_presets_lock_path();
+
 // 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());
 

src/libslic3r/PresetBundle.cpp

@@ -12,6 +12,7 @@
 #include "libslic3r.h"
 #include "I18N.hpp"
 #include "Utils.hpp"
+#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Model.hpp"
 #include "TriangleSelector.hpp"
@@ -2231,6 +2232,7 @@ 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;
+    InstanceLock instance_lock(user_presets_lock_path());
     fs::path folder(dir_user_presets);
     if (fs::exists(folder)) {
         fs::remove_all(folder);
@@ -7867,6 +7869,7 @@ bool PresetBundle::check_duplicate_filament_subtypes() const
 // Orca: BundleMetadata method implementations
 bool BundleMetadata::load_from_json(const std::string& path)
 {
+    InstanceLock instance_lock(user_presets_lock_path());
     try {
         boost::nowide::ifstream ifs(path);
         if (!ifs.good())
@@ -7932,9 +7935,13 @@ 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);
 
-        boost::nowide::ofstream ofs(path);
+        const std::string content = j.dump(4);
-        ofs << j.dump(4);
+        InstanceLock instance_lock(user_presets_lock_path());
-        return ofs.good();
+        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;
     } catch (const std::exception& e) {
         BOOST_LOG_TRIVIAL(error) << "Failed to save bundle metadata to " << path << ": " << e.what();
         return false;

src/libslic3r/Utils.hpp

@@ -224,6 +224,10 @@ 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 `content` 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 file is removed on failure.
+extern std::error_code write_file_atomically(const std::string &path, const std::string &content);
 
 enum CopyFileResult {
 	SUCCESS = 0,

src/libslic3r/utils.cpp

@@ -9,6 +9,7 @@
 #include <stdio.h>
 #include <filesystem>
 #include <sstream>
+#include <cerrno>
 #include <iomanip>
 #include <algorithm>
 #include <cmath>
@@ -704,11 +705,46 @@ std::error_code rename_file(const std::string &from, const std::string &to)
 #ifdef _WIN32
 	return WindowsSupport::rename(from, to);
 #else
-	boost::nowide::remove(to.c_str());
+	// rename(2) replaces an existing target atomically; removing it first would
-	return std::make_error_code(static_cast<std::errc>(boost::nowide::rename(from.c_str(), to.c_str())));
+	// leave a window in which the file does not exist at all.
+	return std::make_error_code(static_cast<std::errc>(boost::nowide::rename(from.c_str(), to.c_str()) == 0 ? 0 : errno));
 #endif
 }
 
+static std::error_code write_whole_file(const std::string &path, const std::string &content)
+{
+	errno = 0;
+	boost::nowide::ofstream out(path, std::ios::out | std::ios::trunc);
+	out << content;
+	out.close();
+	if (! out.fail())
+		return {};
+	return std::make_error_code(errno != 0 ? static_cast<std::errc>(errno) : std::errc::io_error);
+}
+
+std::error_code write_file_atomically(const std::string &path, const std::string &content)
+{
+	const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + ".tmp";
+	if (std::error_code ec = write_whole_file(tmp_path, content)) {
+		boost::nowide::remove(tmp_path.c_str());
+		return ec;
+	}
+	std::error_code ec = rename_file(tmp_path, path);
+	if (ec)
+		boost::nowide::remove(tmp_path.c_str());
+#ifdef _WIN32
+	// Windows refuses to replace a file another process holds open without
+	// FILE_SHARE_DELETE, which is how the C runtime opens files for reading.
+	// 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.
+	if (ec == std::errc::permission_denied) {
+		BOOST_LOG_TRIVIAL(warning) << "Cannot replace " << path << " while another process holds it open; writing in place";
+		ec = write_whole_file(path, content);
+	}
+#endif
+	return ec;
+}
+
 #ifdef __linux__
 // Copied from boost::filesystem.
 // Called by copy_file_linux() in case linux sendfile() API is not supported.

src/slic3r/GUI/GUI_App.cpp

@@ -85,6 +85,7 @@
 #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"
@@ -7624,7 +7625,10 @@ 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;
-                            boost::filesystem::remove_all(bundle_folder, 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);
@@ -8889,6 +8893,7 @@ 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;
     }

tests/libslic3r/CMakeLists.txt

@@ -49,6 +49,7 @@ 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

@@ -0,0 +1,146 @@
+#include <catch2/catch_all.hpp>
+
+#include <atomic>
+#include <chrono>
+#include <thread>
+
+#include <boost/filesystem.hpp>
+
+#include "libslic3r/InstanceLock.hpp"
+#include "test_utils.hpp"
+
+#ifndef _WIN32
+#include <fcntl.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#endif
+
+using namespace Slic3r;
+using namespace std::chrono_literals;
+
+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());
+    REQUIRE(std::chrono::steady_clock::now() - started < 1000ms);
+}
+
+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 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);
+}
+
+#ifndef _WIN32
+// The cross-process side of the lock is a POSIX fcntl write lock, which a
+// child process takes here directly; the same primitive backs the guard on
+// Windows through LockFileEx, but spawning a child there is not worth a test.
+TEST_CASE("InstanceLock yields to another process and reports it", "[InstanceLock]")
+{
+    ScopedTemporaryFile lock_file(".lock");
+    const std::string   path = lock_file.string();
+
+    int child_holds[2], child_may_exit[2];
+    REQUIRE(::pipe(child_holds) == 0);
+    REQUIRE(::pipe(child_may_exit) == 0);
+
+    const pid_t child = ::fork();
+    REQUIRE(child >= 0);
+    if (child == 0) {
+        int fd = ::open(path.c_str(), O_RDWR | O_CREAT, 0644);
+        struct flock lock{};
+        lock.l_type   = F_WRLCK;
+        lock.l_whence = SEEK_SET;
+        char byte = ::fcntl(fd, F_SETLK, &lock) == 0 ? '1' : '0';
+        if (::write(child_holds[1], &byte, 1) != 1 || ::read(child_may_exit[0], &byte, 1) != 1)
+            ::_exit(1);
+        ::_exit(0);
+    }
+
+    char byte = '0';
+    REQUIRE(::read(child_holds[0], &byte, 1) == 1);
+    REQUIRE(byte == '1');
+
+    bool locked_while_child_holds;
+    {
+        InstanceLock lock(path, 100ms);
+        locked_while_child_holds = lock.locked();
+    }
+    // The timed-out wait starts a cool-down: the next guard does not wait again.
+    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;
+    REQUIRE(::write(child_may_exit[1], "x", 1) == 1);
+    int status = 0;
+    REQUIRE(::waitpid(child, &status, 0) == child);
+    for (int fd : {child_holds[0], child_holds[1], child_may_exit[0], child_may_exit[1]})
+        ::close(fd);
+
+    REQUIRE_FALSE(locked_while_child_holds);
+    REQUIRE_FALSE(locked_during_cooldown);
+    REQUIRE(cooldown_wait < 1000ms);
+    // A guard inside the cool-down still takes the lock when it is free.
+    InstanceLock lock(path);
+    REQUIRE(lock.locked());
+}
+#endif

tests/libslic3r/test_utils.cpp

@@ -10,6 +10,7 @@
 #include <cctype>
 #include <fstream>
 #include <string>
+#include <system_error>
 
 #ifndef _WIN32
 #include <unistd.h>     // getuid
@@ -62,6 +63,33 @@ 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));
+}
+
 TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") {
     ScopedTemporaryFile source(".txt");
     {