Retry a Failed Lock File, Keep Special Targets in Place and Sweep Stale Temporaries

A lock file that failed to open once stayed unopened for the rest of the
process, so a scanner holding the fresh file for a moment on Windows
silently disabled the lock for the session, and a lock call that throws,
as it does on a share without a lock service, was re-attempted and logged
by every guard. Both are now left alone for the cool-down and then retried.

Renaming a fresh file over the target turned a symlinked preset into a
plain file, reset its permissions to the umask default, and could not
work at all for a settings export to a device or pipe, since a temporary
cannot be created beside /dev/stdout. A target that is not a regular file
is now written in place, an existing target keeps its permissions, and a
directory that refuses the temporary but not the file falls back too.

The inline PhysicalPrinter::save() overload was the one physical printer
writer without the guard. load_info() took the lock redundantly under the
scan guard and, in read-only mode, created user.lock per file; the bundle
metadata reader now honours read_only too. The plugin config, the setup
guide's profile cache and the vendor preset cache wrote through a
temporary by hand; they use the helper, with a binary mode for the cache.
A crash between temporary and rename left a <name>.<pid>.tmp behind for
good; AppConfig::load() and the preset scan remove stale ones while they
hold the lock, and AppConfig::save() names its temporary the same way.

src/libslic3r/AppConfig.cpp

@@ -737,6 +737,10 @@ std::string AppConfig::load()
 
     // Keep another instance from replacing or restoring the file mid-read.
     InstanceLock instance_lock(lock_path());
+    if (instance_lock.locked()) {
+        const boost::filesystem::path conf(config_path());
+        remove_stale_temp_files(conf.parent_path(), conf.filename().string());
+    }
 
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
@@ -988,7 +992,7 @@ void AppConfig::save()
     // 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::string path_pid = (boost::format("%1%.%2%.tmp") % path % get_current_pid()).str();
 
     json j;
 
@@ -1158,6 +1162,10 @@ std::string AppConfig::load()
 {
     // Keep another instance from replacing or restoring the file mid-read.
     InstanceLock instance_lock(lock_path());
+    if (instance_lock.locked()) {
+        const boost::filesystem::path conf(config_path());
+        remove_stale_temp_files(conf.parent_path(), conf.filename().string());
+    }
 
     // 1) Read the complete config file into a boost::property_tree.
     namespace pt = boost::property_tree;
@@ -1297,7 +1305,7 @@ void AppConfig::save()
     // 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::string path_pid = (boost::format("%1%.%2%.tmp") % path % get_current_pid()).str();
 
     std::stringstream config_ss;
     if (m_mode == EAppMode::Editor)

src/libslic3r/InstanceLock.cpp

@@ -21,12 +21,15 @@ namespace Slic3r {
 struct InstanceLock::Slot
 {
     std::recursive_mutex mutex;
-    // Absent when the lock file could not be created or opened.
+    // Absent until the lock file has been created and 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{};
+    // After a failed open or a failing lock call, guards skip the file lock
+    // entirely until this point.
+    std::chrono::steady_clock::time_point retry_at{};
 };
 
 InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path)
@@ -38,32 +41,42 @@ InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path)
 
     std::lock_guard<std::mutex> guard(registry_mutex);
     std::unique_ptr<Slot> &slot = (*registry)[lock_file_path];
-    if (! slot) {
+    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;
 }
 
+// Creates the lock file if needed and opens it. Called with the slot mutex held.
+static bool open_lock_file(std::unique_ptr<boost::interprocess::file_lock> &file_lock, const std::string &lock_file_path)
+{
+    try {
+        // file_lock only opens existing files.
+        boost::nowide::ofstream(lock_file_path, std::ios::app).close();
+#ifdef _WIN32
+        file_lock = std::make_unique<boost::interprocess::file_lock>(boost::nowide::widen(lock_file_path).c_str());
+#else
+        file_lock = std::make_unique<boost::interprocess::file_lock>(lock_file_path.c_str());
+#endif
+        return true;
+    } 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 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->mutex.lock();
-    if (m_slot->depth++ == 0 && m_slot->file_lock) {
-        const auto now      = std::chrono::steady_clock::now();
+    const auto now = std::chrono::steady_clock::now();
+    if (m_slot->depth++ == 0 && now >= m_slot->retry_at) {
+        if (! m_slot->file_lock && ! open_lock_file(m_slot->file_lock, lock_file_path))
+            m_slot->retry_at = now + cooldown;
+    }
+    if (m_slot->depth == 1 && m_slot->file_lock && now >= m_slot->retry_at) {
         const bool wait     = now >= m_slot->skip_waiting_until;
         const auto deadline = now + timeout;
         for (;;) {
@@ -73,7 +86,9 @@ InstanceLock::InstanceLock(const std::string &lock_file_path, std::chrono::milli
                     break;
                 }
             } catch (const std::exception &e) {
-                BOOST_LOG_TRIVIAL(warning) << "Cannot lock " << lock_file_path << ": " << e.what();
+                m_slot->retry_at = std::chrono::steady_clock::now() + cooldown;
+                BOOST_LOG_TRIVIAL(warning) << "Cannot lock " << lock_file_path << ": " << e.what()
+                                           << "; proceeding without the lock for the next " << cooldown.count() << " ms";
                 break;
             }
             if (! wait)

src/libslic3r/InstanceLock.hpp

@@ -19,7 +19,9 @@ namespace Slic3r {
 // 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.
+// of saves pays the wait once rather than once per file; a lock file that could
+// not be opened, or a lock call that fails outright (a share without a lock
+// service), is likewise retried only after `cooldown`.
 //
 // 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.
@@ -30,7 +32,9 @@ class InstanceLock
 {
 public:
     static constexpr std::chrono::milliseconds default_timeout{2000};
-    static constexpr std::chrono::milliseconds cooldown{10000};
+    // How long a lock file is left alone after a timed-out wait, a failed open
+    // or a failing lock call. 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);

src/libslic3r/Preset.cpp

@@ -611,7 +611,6 @@ 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);
@@ -1722,6 +1721,8 @@ void PresetCollection::load_presets(
     // 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());
+    if (instance_lock.locked())
+        remove_stale_temp_files(dir);
     //BBS: change to json format
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
     {
@@ -4219,6 +4220,12 @@ 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());

src/libslic3r/Preset.hpp

@@ -1057,7 +1057,7 @@ public:
 
     //BBS: change to json format
     //void                save() { this->config.save(this->file); }
-    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(DynamicPrintConfig* parent_config);
     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

@@ -1241,7 +1241,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!metadata.load_from_json(metadata_file.string())) continue;
+            if (!metadata.load_from_json(metadata_file.string(), read_only)) continue;
             metadata.print_presets.clear();
             metadata.filament_presets.clear();
             metadata.printer_presets.clear();
@@ -1276,7 +1276,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!metadata.load_from_json(metadata_file.string())) continue;
+            if (!metadata.load_from_json(metadata_file.string(), read_only)) continue;
             metadata.print_presets.clear();
             metadata.filament_presets.clear();
             metadata.printer_presets.clear();
@@ -7867,9 +7867,9 @@ bool PresetBundle::check_duplicate_filament_subtypes() const
 }
 
 // Orca: BundleMetadata method implementations
-bool BundleMetadata::load_from_json(const std::string& path)
+bool BundleMetadata::load_from_json(const std::string& path, bool read_only)
 {
-    InstanceLock instance_lock(user_presets_lock_path());
+    InstanceLock instance_lock(read_only ? std::string() : user_presets_lock_path());
     try {
         boost::nowide::ifstream ifs(path);
         if (!ifs.good())

src/libslic3r/PresetBundle.hpp

@@ -129,7 +129,8 @@ struct BundleMetadata
     bool                            not_found{false};
     bool                            unauthorized{false};
 
-    bool load_from_json(const std::string& path);
+    // A read-only load (the CLI) takes no instance lock.
+    bool load_from_json(const std::string& path, bool read_only = false);
     bool save_to_json(const std::string& path) const;
 };
 

src/libslic3r/PresetCacheFormat.cpp

@@ -400,46 +400,25 @@ 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, 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";
+    // 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.
     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();
-            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);
+        CacheFileHeader fhdr;
+        fhdr.magic     = CACHE_MAGIC;
+        fhdr.version   = CACHE_VERSION;
+        fhdr.data_size = static_cast<uint64_t>(blob.size());
+        fhdr.crc32     = crc.checksum();
+        std::string payload(reinterpret_cast<const char*>(&fhdr), sizeof(fhdr));
+        payload += blob;
+        if (const std::error_code ec = write_file_atomically(path, payload, /*binary=*/true)) {
+            BOOST_LOG_TRIVIAL(warning) << "VendorCacheFile: write failed (" << path << "): " << ec.message();
             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

@@ -226,8 +226,14 @@ extern std::vector<std::string> split_string(const std::string &str, char delimi
 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);
+// partial one. The temporary file is removed on failure and an existing target
+// keeps its permissions. A target that is not a regular file (a symlink, device
+// or pipe) is written in place, since replacing it would change what it is.
+extern std::error_code write_file_atomically(const std::string &path, const std::string &content, bool binary = false);
+// Remove the `<name>.<pid>.tmp` files a crashed write_file_atomically() left in
+// `dir`, restricted to names starting with `name_prefix` when given. Call it only
+// while holding the lock that guards writes into `dir`. Returns how many were removed.
+extern size_t remove_stale_temp_files(const boost::filesystem::path &dir, const std::string &name_prefix = std::string());
 
 enum CopyFileResult {
 	SUCCESS = 0,

src/libslic3r/utils.cpp

@@ -711,10 +711,10 @@ std::error_code rename_file(const std::string &from, const std::string &to)
 #endif
 }
 
-static std::error_code write_whole_file(const std::string &path, const std::string &content)
+static std::error_code write_whole_file(const std::string &path, const std::string &content, bool binary)
 {
 	errno = 0;
-	boost::nowide::ofstream out(path, std::ios::out | std::ios::trunc);
+	boost::nowide::ofstream out(path, std::ios::out | std::ios::trunc | (binary ? std::ios::binary : std::ios::openmode{}));
 	out << content;
 	out.close();
 	if (! out.fail())
@@ -722,13 +722,24 @@ static std::error_code write_whole_file(const std::string &path, const std::stri
 	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)
+std::error_code write_file_atomically(const std::string &path, const std::string &content, 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_regular_file(target))
+		return write_whole_file(path, content, binary);
+
 	const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + ".tmp";
-	if (std::error_code ec = write_whole_file(tmp_path, content)) {
+	if (std::error_code ec = write_whole_file(tmp_path, content, binary)) {
 		boost::nowide::remove(tmp_path.c_str());
+		// A directory that allows writing the file but not creating one beside it.
+		if (target_exists && ec == std::errc::permission_denied)
+			return write_whole_file(path, content, binary);
 		return ec;
 	}
+	if (target_exists)
+		boost::filesystem::permissions(tmp_path, target.permissions(), bec);
 	std::error_code ec = rename_file(tmp_path, path);
 	if (ec)
 		boost::nowide::remove(tmp_path.c_str());
@@ -739,12 +750,41 @@ std::error_code write_file_atomically(const std::string &path, const std::string
 	// 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);
+		ec = write_whole_file(path, content, binary);
 	}
 #endif
 	return ec;
 }
 
+size_t remove_stale_temp_files(const boost::filesystem::path &dir, const std::string &name_prefix)
+{
+	// <anything>.<digits>.tmp
+	auto is_temp_name = [&name_prefix](const std::string &name) {
+		if (name.compare(0, name_prefix.size(), name_prefix) != 0)
+			return false;
+		static const std::string suffix = ".tmp";
+		if (name.size() <= suffix.size() || name.compare(name.size() - suffix.size(), suffix.size(), suffix) != 0)
+			return false;
+		const size_t digits_end = name.size() - suffix.size();
+		const size_t dot        = name.rfind('.', digits_end - 1);
+		if (dot == std::string::npos || dot == 0 || dot + 1 == digits_end)
+			return false;
+		return std::all_of(name.begin() + dot + 1, name.begin() + digits_end, [](char c) { return c >= '0' && c <= '9'; });
+	};
+	size_t removed = 0;
+	boost::system::error_code ec;
+	for (boost::filesystem::directory_iterator it(dir, ec), end; ! ec && it != end; it.increment(ec)) {
+		if (! boost::filesystem::is_regular_file(it->symlink_status()) || ! is_temp_name(it->path().filename().string()))
+			continue;
+		if (boost::filesystem::remove(it->path(), ec)) {
+			BOOST_LOG_TRIVIAL(info) << "Removed stale temporary file " << it->path();
+			++ removed;
+		}
+		ec.clear();
+	}
+	return removed;
+}
+
 #ifdef __linux__
 // Copied from boost::filesystem.
 // Called by copy_file_linux() in case linux sendfile() API is not supported.

src/slic3r/GUI/WebGuideDialog.cpp

@@ -1481,9 +1481,7 @@ 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, 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";
+        // are: half a cache must never be readable.
         try {
             json out;
             out["format"]  = 1;
@@ -1492,18 +1490,9 @@ bool GuideFrame::BuildProfileDataFromVendors()
             for (const char* key : { "model", "machine", "filament", "process" })
                 profile[key] = m_ProfileJson[key];
             boost::filesystem::create_directories(cache_file.parent_path());
-            {
-                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()))
+            if (const std::error_code ec = write_file_atomically(cache_file.string(), out.dump(-1, ' ', false, json::error_handler_t::ignore), /*binary=*/true))
                 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/plugin/PluginConfig.cpp

@@ -249,21 +249,10 @@ bool PluginConfig::save()
         return false;
     }
 
-    // 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();
+    // 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";
         return false;
     }
 

tests/libslic3r/test_instance_lock.cpp

@@ -62,6 +62,35 @@ TEST_CASE("InstanceLock is a no-op for an empty path and survives an unwritable
     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();
+    const auto         saved_cooldown = InstanceLock::cooldown;
+    InstanceLock::cooldown = 50ms;
+
+    bool before_dir, during_cooldown, after_cooldown;
+    {
+        InstanceLock lock(path, 100ms);
+        before_dir = lock.locked();
+    }
+    boost::filesystem::create_directories(dir.path() / "later");
+    {
+        InstanceLock lock(path, 100ms);
+        during_cooldown = lock.locked();
+    }
+    std::this_thread::sleep_for(80ms);
+    {
+        InstanceLock lock(path, 100ms);
+        after_cooldown = lock.locked();
+    }
+    InstanceLock::cooldown = saved_cooldown;
+
+    REQUIRE_FALSE(before_dir);
+    REQUIRE_FALSE(during_cooldown);
+    REQUIRE(after_cooldown);
+}
+
 TEST_CASE("InstanceLock serialises the threads of one process", "[InstanceLock]")
 {
     ScopedTemporaryFile lock_file(".lock");

tests/libslic3r/test_utils.cpp

@@ -90,6 +90,58 @@ TEST_CASE("write_file_atomically reports a missing directory and writes nothing"
     REQUIRE_FALSE(boost::filesystem::exists(target));
 }
 
+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("remove_stale_temp_files removes only <name>.<pid>.tmp files", "[utils]") {
+    ScopedTemporaryDir dir;
+    for (const char *name : { "a.json.123.tmp", "b.info.4.tmp", "c.json", "d.tmp", "e.json.x.tmp", "f.json..tmp" })
+        REQUIRE_FALSE(write_file_atomically((dir.path() / name).string(), "x"));
+
+    SECTION("with a name prefix only matching names go") {
+        REQUIRE(remove_stale_temp_files(dir.path(), "a.json") == 1);
+        REQUIRE_FALSE(boost::filesystem::exists(dir.path() / "a.json.123.tmp"));
+        REQUIRE(boost::filesystem::exists(dir.path() / "b.info.4.tmp"));
+    }
+    SECTION("without a prefix every stale temporary goes and nothing else") {
+        REQUIRE(remove_stale_temp_files(dir.path()) == 2);
+        size_t entries = 0;
+        for (auto &entry : boost::filesystem::directory_iterator(dir.path())) {
+            (void) entry;
+            ++entries;
+        }
+        REQUIRE(entries == 4);
+    }
+}
+
 TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") {
     ScopedTemporaryFile source(".txt");
     {