Keep the Scarf Seam Free of Sub-Millimetre Segments

A scarf joint split the loop at exactly the scarf length, so the remainder
of the segment the split landed in became the first flat segment, often a
fraction of a millimetre. The seam insertion also leaves segments of a few
micrometres at both ends of the loop, which a scarf extrudes through where a
plain loop would start or stop. With junction deviation the planner treats
such short moves as tight corners, limited by the Z axis acceleration at the
end of the ramp, and slows to a third of the wall speed or nearly halts at
the seam.

Extend the ramp to the next vertex when the remainder would be shorter than
half a scarf step, capped at a millimetre, beyond which planners treat a move
as ordinary; the scarf only grows, never shrinks. Drop the vertex next to the
seam point at either end when that segment is shorter than an eighth of a
common line width, so the loop still starts and ends at the seam; a trimmed
path loses its arc fitting and prints as line segments. Clamp the scarf
length to the trimmed loop so a scarf covering a whole loop still ends at
full flow. The descending pass reuses the same path, so both wedges stay
consistent, and a flat part that would collapse to a single point is dropped.

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"
@@ -731,17 +730,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());
-    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;
     pt::ptree tree;
@@ -991,6 +983,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;
 
@@ -1120,22 +1113,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))
-        m_dirty = false;
+    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());
-    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;
     pt::ptree tree;
@@ -1273,6 +1287,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)
@@ -1308,40 +1323,38 @@ 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))
-        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());
+    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 replace is not safe in case of an application crash, there is no atomic "rename file" API
+    // 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.
-    body += appconfig_md5_hash_line(checksum_source);
+    c << appconfig_md5_hash_line(config_str);
 #endif
-    // The config was always replaced, never written in place, so a read-only one is replaced still.
-    if (const std::error_code ec = write_file_atomically(path, body, false, /*replace_read_only=*/true)) {
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the configuration " << path << ": " << ec.message()
-                                 << "; trying again in " << m_retry_save_after.count() << " s";
-        m_retry_save_at    = std::chrono::steady_clock::now() + m_retry_save_after;
-        m_retry_save_after = std::min(m_retry_save_after * 2, std::chrono::seconds(300));
-        return false;
+    c.close();
+    if (c.fail()) {
+      BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
+      return;
     }
-    m_retry_save_at    = {};
-    m_retry_save_after = std::chrono::seconds(10);
+
 #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, false, /*replace_read_only=*/true))
-        BOOST_LOG_TRIVIAL(error) << "Failed to write the backup configuration " << backup_path << ": " << ec.message();
+    // 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
-    return true;
+
+    // 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::get_variant(const std::string &vendor, const std::string &model, const std::string &variant) const
 {
@@ -1831,11 +1844,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
@@ -1872,7 +1880,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"
@@ -122,18 +121,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; }
@@ -343,8 +338,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; }
@@ -455,17 +448,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 until this point, which moves out
-	// ten seconds, then twenty, up to five minutes, for every failure in a row.
-	std::chrono::steady_clock::time_point						m_retry_save_at{};
-	std::chrono::seconds										m_retry_save_after{10};
 	// 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

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

@@ -1522,10 +1522,12 @@ 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_LOG_TRIVIAL(info) << __FUNCTION__ << ":" <<__LINE__ << boost::format(", saved config to %1%\n")%file;
+    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;
 }
 
 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/ExtrusionEntity.cpp

@@ -423,6 +423,74 @@ bool ExtrusionLoop::is_smooth(double angle_threshold, double min_arm_length) con
     return true;
 }
 
+// The seam is inserted into the loop unless a vertex lies within the G-code resolution of it, so a
+// loop can begin and end with a segment of a few micrometres. A plain loop stops there anyway; a
+// scarf extrudes through both ends, and the planner nearly halts on a block that short. Drop the
+// vertex next to the seam point instead, so the loop still starts and ends at the seam. A trimmed
+// path loses its arc fitting; its geometry is unchanged, it just prints as line segments.
+static void trim_seam_ends(ExtrusionPaths &paths, double tolerance)
+{
+    const auto shorter = [tolerance](const Point3 &a, const Point3 &b) { return (b - a).cast<double>().norm() < tolerance; };
+
+    while (!paths.empty()) {
+        Points3 &points = paths.front().polyline.points;
+        if (points.size() < 2 || !shorter(points[0], points[1]))
+            break;
+        if (points.size() > 2) {
+            points.erase(points.begin() + 1);
+            paths.front().polyline.fitting_result.clear();
+        } else if (paths.size() > 1) {
+            const Point3 seam = points.front();
+            paths.erase(paths.begin());
+            paths.front().polyline.points.front() = seam;
+            paths.front().polyline.fitting_result.clear();
+        } else {
+            break;
+        }
+    }
+
+    while (!paths.empty()) {
+        Points3 &points = paths.back().polyline.points;
+        if (points.size() < 2 || !shorter(points[points.size() - 2], points.back()))
+            break;
+        if (points.size() > 2) {
+            points.erase(points.end() - 2);
+            paths.back().polyline.fitting_result.clear();
+        } else if (paths.size() > 1) {
+            const Point3 seam = points.back();
+            paths.pop_back();
+            paths.back().polyline.points.back() = seam;
+            paths.back().polyline.fitting_result.clear();
+        } else {
+            break;
+        }
+    }
+}
+
+// Split `polyline` where the scarf ramp ends. When the split would leave a remainder shorter
+// than half a slope step before the next vertex, the ramp is extended to that vertex instead:
+// a stub that short makes the motion planner slow down at the end of the ramp. Planners treat
+// moves of a millimetre and more as ordinary, so the ramp never grows by more than that.
+static void split_at_slope_end(const Polyline3 &polyline, double length, double slope_max_segment_length, Polyline3 &slope, Polyline3 &flat)
+{
+    const double snap_distance = std::min(0.5 * slope_max_segment_length, scale_(1.));
+    double acc_length = 0.;
+    size_t line_idx   = 0;
+    for (const Line3 &line : polyline.lines()) {
+        const double end_length = acc_length + line.length();
+        if (end_length >= length) {
+            if (end_length - length < snap_distance) {
+                polyline.split_at_index(line_idx + 1, &slope, &flat);
+                return;
+            }
+            break;
+        }
+        acc_length = end_length;
+        ++line_idx;
+    }
+    polyline.split_at_length(length, &slope, &flat);
+}
+
 ExtrusionLoopSloped::ExtrusionLoopSloped(ExtrusionPaths&   original_paths,
                                          double            seam_gap,
                                          double            slope_min_length,
@@ -431,6 +499,14 @@ ExtrusionLoopSloped::ExtrusionLoopSloped(ExtrusionPaths&   original_paths,
                                          ExtrusionLoopRole role)
     : ExtrusionLoop(role)
 {
+    // An eighth of a common line width: the path moves by less than that at the seam.
+    trim_seam_ends(original_paths, scale_(0.05));
+    // The caller measured the loop before the trim; a scarf that covers the whole loop must still end at 1.
+    double trimmed_length = 0.;
+    for (const ExtrusionPath &path : original_paths)
+        trimmed_length += unscale_(path.length());
+    slope_min_length = std::min(slope_min_length, trimmed_length);
+
     // create slopes
     const auto add_slop = [this, slope_max_segment_length, seam_gap](const ExtrusionPath &path, const Polyline3 &poly, double ratio_begin, double ratio_end) {
         if (poly.empty()) { return; }
@@ -487,12 +563,13 @@ ExtrusionLoopSloped::ExtrusionLoopSloped(ExtrusionPaths&   original_paths,
             // Split current path into slope and non-slope part
             Polyline3 slope_path;
             Polyline3 flat_path;
-            path->polyline.split_at_length(scale_(remaining_length), &slope_path, &flat_path);
+            split_at_slope_end(path->polyline, scale_(remaining_length), slope_max_segment_length, slope_path, flat_path);
 
             add_slop(*path, slope_path, start_ratio, 1);
             start_ratio = 1;
 
-            paths.emplace_back(std::move(flat_path), *path);
+            if (flat_path.size() > 1)
+                paths.emplace_back(std::move(flat_path), *path);
             remaining_length = 0;
         } else {
             remaining_length -= path_len;

src/libslic3r/InstanceLock.cpp

@@ -1,186 +0,0 @@
-#include "InstanceLock.hpp"
-
-#include <algorithm>
-#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_CLOEXEC))
-    {
-        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;
-        if (errno == EWOULDBLOCK)
-            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.
-struct InstanceLock::Slot
-{
-    std::recursive_mutex mutex;
-    std::unique_ptr<NativeFileLock> file_lock;
-    int  depth{0};
-    bool file_locked{false};
-    // After a timed-out wait, guards skip waiting until this point; each wait
-    // that times out in a row doubles the next cool-down, up to a few minutes.
-    std::chrono::steady_clock::time_point skip_waiting_until{};
-    int consecutive_timeouts{0};
-    // After a failed open or a failing lock call, guards skip the file lock
-    // entirely until this point, again doubling for failures in a row.
-    std::chrono::steady_clock::time_point retry_at{};
-    int consecutive_failures{0};
-};
-
-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;
-}
-
-static std::chrono::milliseconds backoff_for(int failures_in_a_row, std::chrono::milliseconds base)
-{
-    return std::min(base * (1 << std::min(failures_in_a_row, 5)), std::chrono::milliseconds(std::chrono::minutes(5)));
-}
-
-// 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 {
-        // The lock opens an existing file read-write, and every user sharing
-        // the data dir has to be able to open it.
-        boost::nowide::ofstream(lock_file_path, std::ios::app).close();
-        boost::system::error_code ec;
-        boost::filesystem::permissions(lock_file_path, boost::filesystem::owner_read | boost::filesystem::owner_write |
-                                                       boost::filesystem::group_read | boost::filesystem::group_write |
-                                                       boost::filesystem::others_read | boost::filesystem::others_write, ec);
-#ifdef _WIN32
-        slot.file_lock = std::make_unique<NativeFileLock>(boost::nowide::widen(lock_file_path).c_str());
-#else
-        slot.file_lock = std::make_unique<NativeFileLock>(lock_file_path.c_str());
-#endif
-        return true;
-    } catch (const std::exception &e) {
-        const auto backoff = backoff_for(slot.consecutive_failures ++, cooldown);
-        slot.retry_at      = std::chrono::steady_clock::now() + backoff;
-        BOOST_LOG_TRIVIAL(warning) << "Cannot open lock file " << lock_file_path << ": " << e.what()
-                                   << " (check its owner and permissions); other instances are not excluded from writing for the next "
-                                   << backoff.count() << " ms";
-        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->retry_at && open_lock_file(*m_slot, lock_file_path)) {
-        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;
-                    m_slot->consecutive_timeouts = 0;
-                    m_slot->consecutive_failures = 0;
-                    m_slot->skip_waiting_until   = {};
-                    break;
-                }
-            } catch (const std::exception &e) {
-                const auto backoff = backoff_for(m_slot->consecutive_failures ++, cooldown);
-                m_slot->retry_at   = std::chrono::steady_clock::now() + backoff;
-                BOOST_LOG_TRIVIAL(warning) << "Cannot lock " << lock_file_path << ": " << e.what()
-                                           << "; proceeding without the lock for the next " << backoff.count() << " ms";
-                break;
-            }
-            if (! wait)
-                break;
-            if (std::chrono::steady_clock::now() >= deadline) {
-                const auto backoff = backoff_for(m_slot->consecutive_timeouts ++, cooldown);
-                m_slot->skip_waiting_until = deadline + backoff;
-                BOOST_LOG_TRIVIAL(warning) << "Another instance has held " << lock_file_path << " for over "
-                                           << timeout.count() << " ms; proceeding without the lock for the next "
-                                           << backoff.count() << " ms";
-                break;
-            }
-            std::this_thread::sleep_for(std::chrono::milliseconds(5));
-        }
-        if (! m_slot->file_locked)
-            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_locked;
-}
-
-InstanceLock::~InstanceLock()
-{
-    if (m_slot == nullptr)
-        return;
-    if (-- m_slot->depth == 0 && m_slot->file_lock) {
-        if (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->file_lock.reset();
-    }
-}
-
-} // namespace Slic3r

src/libslic3r/InstanceLock.hpp

@@ -1,72 +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` (flock on POSIX, tied to
-// the guard's own open file so nothing else in the process can drop it by
-// opening and closing the file; LockFileEx on Windows). The file is created on
-// first use and kept, since the lock state lives in the kernel on the open
-// file and deleting the file on release would let a third instance lock a
-// fresh file while the second still holds the old one; it is opened by the
-// outermost guard and closed when that guard goes, so whatever is at the path
-// is what gets locked and nothing stays open between saves. 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` (a second by default), 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`, doubling
-// for every further timeout in a row up to a few minutes, so a batch of saves
-// pays the wait once rather than once per file and a holder that never lets go
-// does not cost a stall every cool-down for good; 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`, doubling the same way.
-// A wait for the in-process mutex is bounded only by the longest critical
-// section, which is why every guard covers a few file operations and nothing
-// slower, such as removing a directory tree.
-//
-// 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:
-    // 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};
-    // 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);
-    ~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);
-
-    Slot *m_slot{nullptr};
-    std::unique_lock<std::recursive_mutex> m_slot_guard;
-    bool  m_locked{false};
-};
-
-} // namespace Slic3r

src/libslic3r/Preset.cpp

@@ -48,9 +48,6 @@
 
 #include "libslic3r.h"
 #include "Utils.hpp"
-#include "InstanceLock.hpp"
-
-#include <sstream>
 #include "Time.hpp"
 #include "PlaceholderParser.hpp"
 #include "libslic3r/GCode/Thumbnails.hpp"
@@ -107,43 +104,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.
-static void remove_preset_files(const std::string &preset_file, bool read_only)
-{
-    if (read_only)
-        return;
-    InstanceLock instance_lock(user_presets_lock_path());
-    boost::system::error_code ec;
-    fs::path file_path(preset_file);
-    fs::remove(file_path, ec);
-    file_path.replace_extension(".info");
-    fs::remove(file_path, ec);
-}
-
-void remove_directory_tree_locked(const boost::filesystem::path &dir)
-{
-    boost::system::error_code ec;
-    fs::path doomed = dir;
-    doomed += "." + std::to_string(get_current_pid()) + ".removing";
-    {
-        InstanceLock instance_lock(user_presets_lock_path());
-        if (! fs::exists(dir, ec))
-            return;
-        fs::rename(dir, doomed, ec);
-        if (ec) {
-            // Cannot be set aside: the slow way, still under the lock.
-            fs::remove_all(dir, ec);
-            return;
-        }
-    }
-    fs::remove_all(doomed, ec);
-}
-
 std::string get_preset_bare_name(const std::string &canonical_name)
 {
     const auto pos = canonical_name.find_last_of('/');
@@ -686,20 +646,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)
@@ -708,7 +666,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);
@@ -745,16 +702,12 @@ void 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;
@@ -790,22 +743,15 @@ void 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;
+        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());
     }
-
-    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()))
-        BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << this->file << ": " << ec.message();
-    else
-        BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " save config for: " << this->name << " and filament_id: " << filament_id << " and base_id: " << this->base_id;
+    BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << " save config for: " << this->name << " and filament_id: " << filament_id << " and base_id: " << this->base_id;
 
     // Bundle presets are synced via bundle_id and don't need individual .info files.
     if (! this->is_from_bundle()) {
@@ -824,7 +770,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));
@@ -1759,11 +1704,6 @@ 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);
 
-    {
-        InstanceLock instance_lock(user_presets_lock_path(read_only));
-        if (instance_lock.locked())
-            remove_stale_temp_files(dir);
-    }
     //BBS: change to json format
     for (auto &dir_entry : boost::filesystem::directory_iterator(dir))
     {
@@ -1785,31 +1725,30 @@ void PresetCollection::load_presets(
                 preset.file = dir_entry.path().string();
                 // Load the preset file, apply preset values on top of defaults.
                 try {
+                    fs::path idx_path(preset.file);
+                    idx_path.replace_extension(".info");
+                    if (fs::exists(idx_path)) {
+                        preset.load_info(idx_path.string());
+                    }
                     DynamicPrintConfig config;
+                    //BBS: change to json format
+                    //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
                     std::map<std::string, std::string> key_values;
                     std::string reason;
-                    ConfigSubstitutions config_substitutions;
-                    {
-                        // Per file, so no instance replaces or removes it mid-read, and a
-                        // save on another thread never waits for the whole scan.
-                        InstanceLock instance_lock(user_presets_lock_path(read_only));
-                        fs::path idx_path(preset.file);
-                        idx_path.replace_extension(".info");
-                        if (fs::exists(idx_path)) {
-                            preset.load_info(idx_path.string());
-                        }
-                        //BBS: change to json format
-                        //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule);
-                        config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
-                        if (!reason.empty()) {
-                            remove_preset_files(preset.file, read_only);
-                            BOOST_LOG_TRIVIAL(error) << boost::format("parse config %1% failed")%preset.file;
-                            ++m_errors;
-                            continue;
-                        }
-                    }
+                    ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason);
                     if (! config_substitutions.empty())
                         substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) });
+                    if (!reason.empty()) {
+                        fs::path file_path(preset.file);
+                        if (!read_only && fs::exists(file_path))
+                            fs::remove(file_path);
+                        file_path.replace_extension(".info");
+                        if (!read_only && fs::exists(file_path))
+                            fs::remove(file_path);
+                        BOOST_LOG_TRIVIAL(error) << boost::format("parse config %1% failed")%preset.file;
+                        ++m_errors;
+                        continue;
+                    }
 
                     std::string version_str = key_values[BBL_JSON_KEY_VERSION];
                     boost::optional<Semver> version = Semver::parse(version_str);
@@ -1893,13 +1832,23 @@ void PresetCollection::load_presets(
                 } catch (const std::ifstream::failure &err) {
                     ++m_errors;
                     BOOST_LOG_TRIVIAL(error) << boost::format("The user-config cannot be loaded: %1%. Reason: %2%")%preset.file %err.what();
-                    remove_preset_files(preset.file, read_only);
+                    fs::path file_path(preset.file);
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
+                    file_path.replace_extension(".info");
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
                     //throw Slic3r::RuntimeError(std::string("The selected preset cannot be loaded: ") + preset.file + "\n\tReason: " + err.what());
                 } catch (const std::runtime_error &err) {
                     ++m_errors;
                     BOOST_LOG_TRIVIAL(error) << boost::format("Failed loading the user-config file: %1%. Reason: %2%")%preset.file %err.what();
                     //throw Slic3r::RuntimeError(std::string("Failed loading the preset file: ") + preset.file + "\n\tReason: " + err.what());
-                    remove_preset_files(preset.file, read_only);
+                    fs::path file_path(preset.file);
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
+                    file_path.replace_extension(".info");
+                    if (!read_only && fs::exists(file_path))
+                        fs::remove(file_path);
                 }
 
                 if (preset_loaded_fn != nullptr)
@@ -4252,15 +4201,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;
@@ -4392,7 +4334,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.
@@ -4585,10 +4526,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());
-    }
+    boost::nowide::remove(printer.file.c_str());
     m_printers.erase(it);
     return true;
 }
@@ -4600,10 +4538,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());
-    }
+    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

@@ -484,18 +484,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);
-
-// Removes a directory tree under the user preset lock without holding the lock
-// for the removal itself: the tree is renamed aside under the lock, in one
-// step, and deleted afterwards, so a save waiting on the lock waits
-// milliseconds rather than for a tree of files to go.
-void remove_directory_tree_locked(const boost::filesystem::path &dir);
-
 // 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());
 
@@ -1065,7 +1053,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

@@ -12,7 +12,6 @@
 #include "libslic3r.h"
 #include "I18N.hpp"
 #include "Utils.hpp"
-#include "InstanceLock.hpp"
 #include "LocalesUtils.hpp"
 #include "Model.hpp"
 #include "TriangleSelector.hpp"
@@ -1228,15 +1227,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 &bundle_dir, const fs::path &metadata_file, BundleMetadata &metadata) {
-        InstanceLock instance_lock(user_presets_lock_path(read_only));
-        if (instance_lock.locked())
-            remove_stale_temp_files(bundle_dir);
-        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)) {
@@ -1250,7 +1240,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!load_bundle_metadata(entry.path(), 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();
@@ -1285,7 +1275,7 @@ PresetsConfigSubstitutions PresetBundle::load_user_presets(std::string user, For
             if (!fs::exists(metadata_file)) continue;
 
             BundleMetadata metadata;
-            if (!load_bundle_metadata(entry.path(), 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();
@@ -1619,8 +1609,7 @@ 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";
-            // Per process, so two instances importing at once do not clear each other's extraction.
-            fs::path temp_folder(configs_folder / ("temp_" + std::to_string(get_current_pid())));
+            fs::path temp_folder(configs_folder / "temp");
             std::string user_default_temp_dir = temp_folder.make_preferred().string();
             if (fs::exists(temp_folder)) fs::remove_all(temp_folder);
             fs::create_directory(temp_folder, ec);
@@ -2242,7 +2231,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;
-    remove_directory_tree_locked(fs::path(dir_user_presets));
+    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)
@@ -7940,13 +7932,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());
-        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();
+        {
+            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);
             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,26 +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. A target this process may not write is refused,
-// as an in-place write would be, unless `replace_read_only` is set for a file
-// that was always replaced rather than written, such as the app config. 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, and so is a target whose
-// replace the filesystem refuses or beside which no temporary can be created.
-// A successful write into a directory under data_dir() also sweeps stale
-// leftovers there.
-extern std::error_code write_file_atomically(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary = false, bool replace_read_only = false);
-inline std::error_code write_file_atomically(const std::string &path, const std::string &content, bool binary = false, bool replace_read_only = false)
-	{ return write_file_atomically(path, { std::string_view(content) }, binary, replace_read_only); }
-// Remove the `<name>.<pid>.<n>.tmp` files a crashed write_file_atomically() left
-// in `dir` at least an hour ago, only names starting with `name_prefix` when it is
-// given, and log the `<name>.<pid>.old` files a crashed rename_file() left, which
-// may be a last copy or a since-deleted file and so stay for the user. Runs at
-// most once an hour per directory and prefix. 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

@@ -9,13 +9,6 @@
 #include <stdio.h>
 #include <filesystem>
 #include <sstream>
-#include <cerrno>
-#include <chrono>
-#include <map>
-#include <set>
-#include <mutex>
-#include <thread>
-#include <cstring>
 #include <iomanip>
 #include <algorithm>
 #include <cmath>
@@ -41,7 +34,6 @@
 #else
 	#include <unistd.h>
 	#include <sys/types.h>
-	#include <sys/stat.h>
 	#include <sys/param.h>
     #include <sys/resource.h>
 	#ifdef BSD
@@ -52,6 +44,7 @@
 		#include <libproc.h>
 	#endif
 	#ifdef __linux__
+		#include <sys/stat.h>
 		#include <fcntl.h>
 		#include <sys/sendfile.h>
 		#include <dirent.h>
@@ -459,12 +452,6 @@ boost::filesystem::path get_log_file_name()
 
 #ifdef _WIN32
 // The following helpers are borrowed from the LLVM project https://github.com/llvm
-// Names a file object by volume and file index.
-static std::string file_identity_of(const BY_HANDLE_FILE_INFORMATION &info)
-{
-	return std::to_string(info.dwVolumeSerialNumber) + ":" + std::to_string((static_cast<uint64_t>(info.nFileIndexHigh) << 32) | info.nFileIndexLow);
-}
-
 namespace WindowsSupport
 {
 	template <typename HandleTraits>
@@ -686,7 +673,7 @@ namespace WindowsSupport
 						BY_HANDLE_FILE_INFORMATION FI2;
 						if (! ::GetFileInformationByHandle(to_handle2, &FI2))
 							return map_windows_error(GetLastError());
-						if (file_identity_of(FI) != file_identity_of(FI2))
+						if (FI.nFileIndexHigh != FI2.nFileIndexHigh || FI.nFileIndexLow != FI2.nFileIndexLow || FI.dwVolumeSerialNumber != FI2.dwVolumeSerialNumber)
 							break;
 						continue;
 					}
@@ -715,209 +702,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 and report it as one of these; move the
-	// target aside and try again there, and put it back if that fails too, so
-	// a refusal that was really about the source never costs the target.
-	const bool replace_refused = err == EPERM || err == EACCES || err == EBUSY || err == ENOTSUP || err == EOPNOTSUPP;
-	if (replace_refused) {
-		// Named so remove_stale_temp_files() can clear it after a crash in between.
-		const std::string aside = to + "." + std::to_string(get_current_pid()) + ".old";
-		if (boost::nowide::rename(to.c_str(), aside.c_str()) == 0) {
-			if (boost::nowide::rename(from.c_str(), to.c_str()) == 0) {
-				boost::nowide::remove(aside.c_str());
-				return {};
-			}
-			if (boost::nowide::rename(aside.c_str(), to.c_str()) != 0)
-				BOOST_LOG_TRIVIAL(error) << "Could not put " << to << " back after a failed replace; its previous content is at " << aside;
-		}
-	}
-	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
 }
 
-// Whether this process may open `path` for writing.
-static bool is_writable(const std::string &path)
-{
-#ifdef _WIN32
-	// _waccess() sees only the read-only attribute; an open for writing sees
-	// ACLs too. Another process merely holding the file open is not a refusal:
-	// the rename that follows moves an open destination aside.
-	HANDLE handle = ::CreateFileW(boost::nowide::widen(path).c_str(), GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
-	if (handle == INVALID_HANDLE_VALUE) {
-		const DWORD err = ::GetLastError();
-		return err == ERROR_SHARING_VIOLATION || err == ERROR_LOCK_VIOLATION;
-	}
-	::CloseHandle(handle);
-	return true;
-#else
-	return ::access(path.c_str(), W_OK) == 0;
-#endif
-}
-
-static std::error_code write_whole_file(const std::string &path, std::initializer_list<std::string_view> chunks, bool binary)
-{
-	errno = 0;
-	boost::nowide::ofstream out(path, std::ios::out | std::ios::trunc | (binary ? std::ios::binary : std::ios::openmode{}));
-	for (const std::string_view chunk : chunks)
-		out.write(chunk.data(), static_cast<std::streamsize>(chunk.size()));
-	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, std::initializer_list<std::string_view> chunks, bool binary, bool replace_read_only)
-{
-	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, chunks, binary);
-	// Replacing needs only a writable directory, so a file this process may
-	// not write has to be refused here, as the in-place write used to be.
-	if (target_exists && ! is_writable(path)) {
-		if (! replace_read_only)
-			return std::make_error_code(std::errc::permission_denied);
-#ifdef _WIN32
-		// Nothing replaces a read-only file on Windows, so the attribute goes first.
-		boost::filesystem::permissions(path, boost::filesystem::add_perms | boost::filesystem::owner_write, bec);
-#endif
-	}
-
-	// 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 (std::error_code ec = write_whole_file(tmp_path, chunks, binary)) {
-		boost::nowide::remove(tmp_path.c_str());
-		// A directory that allows writing the file but not creating one beside
-		// it, or a name the longer temporary pushes past the path limit.
-		const bool only_the_temporary_failed = (target_exists && (ec == std::errc::permission_denied || ec == std::errc::no_such_file_or_directory)) ||
-		                                       ec == std::errc::filename_too_long;
-		if (only_the_temporary_failed) {
-			BOOST_LOG_TRIVIAL(warning) << "Cannot create a temporary beside " << path << " (" << ec.message() << "); writing in place";
-			return write_whole_file(path, chunks, binary);
-		}
-		return ec;
-	}
-#ifndef _WIN32
-	// Only here: on Windows the target was refused above unless writable, and
-	// 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.
-		// Once per file at warning level, so a mount that never replaces does
-		// not fill the log, and the loss of the atomic path is still on record.
-		static std::mutex warned_mutex;
-		static std::set<std::string> warned;
-		bool first;
-		{
-			std::lock_guard<std::mutex> guard(warned_mutex);
-			first = warned.insert(path).second;
-		}
-		if (first)
-			BOOST_LOG_TRIVIAL(warning) << "Cannot replace " << path << " (" << ec.message() << "); writing in place, here and for later saves of this file";
-		else
-			BOOST_LOG_TRIVIAL(info) << "Cannot replace " << path << " (" << ec.message() << "); writing in place";
-		return write_whole_file(path, chunks, binary);
-	}
-	// Crash leftovers of earlier writes into this directory, for the files the
-	// application owns; the sweep throttles itself per directory.
-	const boost::filesystem::path dir = boost::filesystem::path(path).parent_path();
-	if (! data_dir().empty() && boost::algorithm::starts_with(dir.generic_string(), boost::filesystem::path(data_dir()).generic_string()))
-		remove_stale_temp_files(dir);
-	return {};
-}
-
-size_t remove_stale_temp_files(const boost::filesystem::path &dir, const std::string &name_prefix)
-{
-	// Once an hour per directory: a batch of saves or a load followed by a
-	// save must not read the same directory over and over.
-	{
-		static std::mutex swept_mutex;
-		static std::map<std::string, std::chrono::steady_clock::time_point> swept_at;
-		const auto now = std::chrono::steady_clock::now();
-		std::lock_guard<std::mutex> guard(swept_mutex);
-		auto &last = swept_at[dir.string() + "|" + name_prefix];
-		if (last != std::chrono::steady_clock::time_point{} && now - last < std::chrono::hours(1))
-			return 0;
-		last = now;
-	}
-	// <name_prefix>...<pid>.<n>.tmp, exactly the shape write_file_atomically()
-	// makes, or <name_prefix>...<pid>.old, the shape rename_file() moves a
-	// target aside under.
-	auto digit_segments_before = [](const std::string &name, size_t end, int count) {
-		for (int segment = 0; segment < count; ++ segment) {
-			const size_t dot = name.rfind('.', end - 1);
-			if (dot == std::string::npos || dot == 0 || dot + 1 == end ||
-			    ! std::all_of(name.begin() + dot + 1, name.begin() + end, [](char c) { return c >= '0' && c <= '9'; }))
-				return false;
-			end = dot;
-		}
-		return true;
-	};
-	auto is_temp_name = [&](const std::string &name) {
-		if (name.compare(0, name_prefix.size(), name_prefix) != 0)
-			return false;
-		static const std::string tmp_suffix = ".tmp", old_suffix = ".old";
-		if (name.size() > tmp_suffix.size() && name.compare(name.size() - tmp_suffix.size(), tmp_suffix.size(), tmp_suffix) == 0)
-			return digit_segments_before(name, name.size() - tmp_suffix.size(), 2);
-		if (name.size() > old_suffix.size() && name.compare(name.size() - old_suffix.size(), old_suffix.size(), old_suffix) == 0)
-			return digit_segments_before(name, name.size() - old_suffix.size(), 1);
-		return false;
-	};
-	// An instance that gave up waiting for the lock writes unlocked by design,
-	// so a temporary this young may still be in flight, and hosts sharing a
-	// data dir may disagree on the time by minutes; a crash leftover is old.
-	constexpr std::time_t stale_age = 60 * 60;
-	const std::time_t     now       = std::time(nullptr);
-	size_t removed = 0;
-	boost::system::error_code ec;
-	for (boost::filesystem::directory_iterator it(dir, ec), end; ! ec && it != end; it.increment(ec)) {
-		// The name test first: it is free, the stat is not.
-		if (! is_temp_name(it->path().filename().string()))
-			continue;
-		boost::system::error_code entry_ec;
-		if (! boost::filesystem::is_regular_file(it->symlink_status(entry_ec)))
-			continue;
-		const std::time_t written = boost::filesystem::last_write_time(it->path(), entry_ec);
-		if (entry_ec || now - written < stale_age)
-			continue;
-		const std::string name = it->path().filename().string();
-		if (name.size() > 4 && name.compare(name.size() - 4, 4, ".old") == 0) {
-			// A target rename_file() moved aside and never put back or removed.
-			// It may be the last copy of a file, or a file since deleted on
-			// purpose; neither removing nor restoring it is safe unasked.
-			static std::mutex warned_mutex;
-			static std::set<std::string> warned;
-			std::lock_guard<std::mutex> guard(warned_mutex);
-			if (warned.insert(it->path().string()).second)
-				BOOST_LOG_TRIVIAL(warning) << it->path() << " was set aside by an interrupted save; restore or delete it by hand";
-			continue;
-		}
-		if (boost::filesystem::remove(it->path(), entry_ec)) {
-			BOOST_LOG_TRIVIAL(info) << "Removed stale temporary file " << it->path();
-			++ removed;
-		}
-	}
-	return removed;
-}
-
 #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,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"
@@ -3532,7 +3531,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();
 
     });
@@ -7624,7 +7623,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();
-                            remove_directory_tree_locked(bundle_folder);
+                            boost::system::error_code ec;
+                            boost::filesystem::remove_all(bundle_folder, ec);
 
                             preset_bundle->bundles.WriteLock();
                             preset_bundle->bundles.m_bundles.erase(bundle.id);
@@ -8889,7 +8889,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;
     }
@@ -8952,19 +8951,15 @@ 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 (!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 .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)

src/slic3r/GUI/WebGuideDialog.cpp

@@ -1481,7 +1481,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;
@@ -1490,9 +1492,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
-            stored = true;
+        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/fff_print/test_extrusion_entity.cpp

@@ -84,3 +84,104 @@ SCENARIO("Polygon flattening", "[ExtrusionEntity]") {
         }
     }
 }
+
+static ExtrusionPaths straight_path(const std::vector<double> &xs)
+{
+    ExtrusionPath path{erExternalPerimeter, 1.0, 0.45f, 0.2f};
+    for (double x : xs)
+        path.polyline.append(Point3::new_scale(x, 0., 0.));
+    return {path};
+}
+
+TEST_CASE("Scarf ramp ends on the next loop vertex instead of leaving a short stub", "[ExtrusionEntity]")
+{
+    using Catch::Matchers::WithinAbs;
+    // A 20 mm scarf in 10 steps: a remainder shorter than half a 2 mm step is snapped forward.
+    const double slope_length = 20.;
+    const double max_segment  = scale_(slope_length / 10);
+
+    SECTION("a 0.09 mm remainder extends the ramp to the vertex") {
+        ExtrusionPaths     paths = straight_path({0., 5., 10., 15., 20.09, 25., 30.});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        REQUIRE(loop.ends.size() == 1);
+        REQUIRE(loop.paths.size() == 1);
+        CHECK_THAT(unscale_(loop.starts.front().polyline.last_point().x()), WithinAbs(20.09, 1e-3));
+        CHECK_THAT(unscale_(loop.ends.front().polyline.last_point().x()), WithinAbs(20.09, 1e-3));
+        CHECK_THAT(unscale_(loop.paths.front().polyline.first_point().x()), WithinAbs(20.09, 1e-3));
+        CHECK_THAT(unscale_(loop.paths.front().polyline.lines().front().length()), WithinAbs(4.91, 1e-3));
+    }
+
+    SECTION("a remainder longer than half a step keeps the exact scarf length") {
+        ExtrusionPaths     paths = straight_path({0., 5., 10., 15., 21.5, 25., 30.});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        REQUIRE(loop.paths.size() == 1);
+        CHECK_THAT(unscale_(loop.starts.front().polyline.last_point().x()), WithinAbs(20., 1e-3));
+        CHECK_THAT(unscale_(loop.paths.front().polyline.first_point().x()), WithinAbs(20., 1e-3));
+        CHECK_THAT(unscale_(loop.paths.front().polyline.lines().front().length()), WithinAbs(1.5, 1e-3));
+    }
+
+    SECTION("the ramp never grows by more than a millimetre, whatever the step size") {
+        ExtrusionPaths     paths = straight_path({0., 5., 10., 15., 21.5, 25., 30.});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, scale_(slope_length), 0.); // a single 20 mm step
+        REQUIRE(loop.paths.size() == 1);
+        CHECK_THAT(unscale_(loop.starts.front().polyline.last_point().x()), WithinAbs(20., 1e-3));
+    }
+
+    SECTION("snapping onto the path's last vertex leaves no single-point flat path") {
+        ExtrusionPaths     paths = straight_path({0., 5., 10., 15., 20.5});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        CHECK(loop.paths.empty());
+        CHECK_THAT(unscale_(loop.starts.front().polyline.last_point().x()), WithinAbs(20.5, 1e-3));
+    }
+}
+
+TEST_CASE("Scarf loop drops the micro segments the seam insertion leaves at both ends", "[ExtrusionEntity]")
+{
+    using Catch::Matchers::WithinAbs;
+    const double slope_length = 20.;
+    const double max_segment  = scale_(slope_length / 10);
+
+    SECTION("a 3 um segment at each end of a single path is removed, the seam point stays") {
+        ExtrusionPaths     paths = straight_path({0., 0.003, 5., 10., 15., 21.5, 25., 29.997, 30.});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        REQUIRE(loop.paths.size() == 1);
+        const Polyline3 &start = loop.starts.front().polyline;
+        CHECK_THAT(unscale_(start.first_point().x()), WithinAbs(0., 1e-4));
+        CHECK_THAT(unscale_(start.lines().front().length()), WithinAbs(1.25, 1e-3)); // 5 mm halved twice
+        const Polyline3 &flat = loop.paths.front().polyline;
+        CHECK_THAT(unscale_(flat.last_point().x()), WithinAbs(30., 1e-4));
+        CHECK_THAT(unscale_(flat.lines().back().length()), WithinAbs(5., 1e-3));
+    }
+
+    SECTION("a micro path of its own is dropped and the neighbour ends at the seam point") {
+        ExtrusionPaths paths = straight_path({0., 0.003});
+        ExtrusionPaths rest  = straight_path({0.003, 5., 10., 15., 21.5, 25., 30.});
+        paths.push_back(rest.front());
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        CHECK_THAT(unscale_(loop.starts.front().polyline.first_point().x()), WithinAbs(0., 1e-4));
+        CHECK_THAT(unscale_(loop.starts.front().polyline.lines().front().length()), WithinAbs(1.25, 1e-3));
+    }
+
+    SECTION("a scarf covering the whole loop still ends at full flow after a trim") {
+        // The caller sizes the scarf from the untrimmed loop: 10.003 mm here, 10 mm after the trim.
+        ExtrusionPaths     paths = straight_path({0., 0.003, 5., 10.});
+        ExtrusionLoopSloped loop(paths, 0., 10.003, max_segment, 0.);
+        REQUIRE(loop.starts.size() == 1);
+        CHECK(loop.paths.empty());
+        CHECK_THAT(loop.starts.back().slope_end.e_ratio, WithinAbs(1., 1e-9));
+        CHECK_THAT(unscale_(loop.starts.back().polyline.last_point().x()), WithinAbs(10., 1e-4));
+    }
+
+    SECTION("segments longer than the tolerance are kept") {
+        ExtrusionPaths     paths = straight_path({0., 0.3, 5., 10., 15., 21.5, 25., 29.7, 30.});
+        ExtrusionLoopSloped loop(paths, 0., slope_length, max_segment, 0.);
+        REQUIRE(loop.paths.size() == 1);
+        CHECK_THAT(unscale_(loop.starts.front().polyline.lines().front().length()), WithinAbs(0.3, 1e-3));
+        CHECK_THAT(unscale_(loop.paths.front().polyline.lines().back().length()), WithinAbs(0.3, 1e-3));
+    }
+}

tests/libslic3r/CMakeLists.txt

@@ -49,7 +49,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,202 +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"
-
-#ifndef _WIN32
-#include <fcntl.h>
-#include <sys/file.h>
-#include <sys/wait.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);
-}
-
-#ifndef _WIN32
-// The cross-process side of the lock is a POSIX flock, which a child process
-// takes here directly; LockFileEx backs the guard on Windows, 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);
-        char byte = ::flock(fd, LOCK_EX | LOCK_NB) == 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 < 4000ms);
-    // 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

@@ -8,11 +8,8 @@
 
 #include <algorithm>
 #include <cctype>
-#include <ctime>
 #include <fstream>
 #include <string>
-#include <thread>
-#include <system_error>
 
 #ifndef _WIN32
 #include <unistd.h>     // getuid
@@ -65,173 +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));
-}
-
-TEST_CASE("write_file_atomically refuses a read-only target and leaves it untouched", "[utils]") {
-#ifndef _WIN32
-    if (::geteuid() == 0)
-        SKIP("a read-only file does not stop root");
-#endif
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "pinned.json";
-    REQUIRE_FALSE(write_file_atomically(target.string(), "pinned"));
-    boost::filesystem::permissions(target, boost::filesystem::owner_read | boost::filesystem::group_read | boost::filesystem::others_read);
-
-    const std::error_code ec = write_file_atomically(target.string(), "replaced");
-    boost::filesystem::permissions(target, boost::filesystem::owner_read | boost::filesystem::owner_write | boost::filesystem::group_read | boost::filesystem::others_read);
-
-    REQUIRE(ec == std::errc::permission_denied);
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "pinned");
-}
-
-TEST_CASE("write_file_atomically replaces a read-only target when asked to", "[utils]") {
-#ifndef _WIN32
-    if (::geteuid() == 0)
-        SKIP("a read-only file does not stop root");
-#endif
-    ScopedTemporaryDir dir;
-    const boost::filesystem::path target = dir.path() / "pinned.json";
-    REQUIRE_FALSE(write_file_atomically(target.string(), "pinned"));
-    boost::filesystem::permissions(target, boost::filesystem::owner_read | boost::filesystem::group_read | boost::filesystem::others_read);
-
-    const std::error_code ec = write_file_atomically(target.string(), "replaced", false, /*replace_read_only=*/true);
-    boost::filesystem::permissions(target, boost::filesystem::owner_read | boost::filesystem::owner_write | boost::filesystem::group_read | boost::filesystem::others_read);
-
-    REQUIRE_FALSE(ec);
-    std::string content;
-    load_string_file(target, content);
-    REQUIRE(content == "replaced");
-}
-
-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("remove_stale_temp_files leaves a moved-aside file to the user", "[utils]") {
-    ScopedTemporaryDir dir;
-    REQUIRE_FALSE(write_file_atomically((dir.path() / "lost.json.4242.old").string(), "maybe the last copy"));
-    boost::filesystem::last_write_time(dir.path() / "lost.json.4242.old", std::time(nullptr) - 7200);
-
-    REQUIRE(remove_stale_temp_files(dir.path()) == 0);
-    REQUIRE(boost::filesystem::exists(dir.path() / "lost.json.4242.old"));
-    REQUIRE_FALSE(boost::filesystem::exists(dir.path() / "lost.json"));
-}
-
-TEST_CASE("remove_stale_temp_files removes only old <name>.<pid>.<n>.tmp files", "[utils]") {
-    ScopedTemporaryDir dir;
-    // a.json exists, so its .old is a leftover; a moved-aside file without its original is a different case.
-    for (const char *name : { "a.json", "a.json.123.7.tmp", "b.info.4.0.tmp", "c.json", "d.tmp", "e.json.x.1.tmp", "f.json..tmp", "a.json.99", "a.json.12.tmp", "a.json.123.old", "h.json.old" }) {
-        REQUIRE_FALSE(write_file_atomically((dir.path() / name).string(), "x"));
-        // An hour old: long past the age below which a temporary may still be in flight.
-        boost::filesystem::last_write_time(dir.path() / name, std::time(nullptr) - 3600);
-    }
-    // Just written: possibly another instance's in-flight save, so it stays.
-    REQUIRE_FALSE(write_file_atomically((dir.path() / "g.json.7.2.tmp").string(), "x"));
-
-    SECTION("with a name prefix only matching names go; a numbered backup, a one-segment name or an .old is not removed") {
-        REQUIRE(remove_stale_temp_files(dir.path(), "a.json") == 1);
-        REQUIRE_FALSE(boost::filesystem::exists(dir.path() / "a.json.123.7.tmp"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json.123.old"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json.99"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json.12.tmp"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "b.info.4.0.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 == 10);
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json.123.old"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "h.json.old"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "a.json.99"));
-        REQUIRE(boost::filesystem::exists(dir.path() / "g.json.7.2.tmp"));
-    }
-}
-
 TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") {
     ScopedTemporaryFile source(".txt");
     {

tests/libslic3r/test_vendor_cache.cpp

@@ -1,9 +1,5 @@
 #include <catch2/catch_all.hpp>
 
-#ifndef _WIN32
-#include <unistd.h>
-#endif
-
 #include <boost/filesystem.hpp>
 #include <boost/crc.hpp>
 #include <cereal/archives/binary.hpp>
@@ -1362,23 +1358,20 @@ TEST_CASE("a header claiming more body than the file holds is rejected", "[Vendo
 
 TEST_CASE("a failed write leaves the previous cache in place", "[VendorCache]")
 {
-#ifndef _WIN32
-    if (::geteuid() == 0)
-        SKIP("a read-only file does not stop root");
-#endif
     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 read-only cache (the read-only attribute on Windows) is refused before
-    // anything is written, so what was there must survive the attempt.
-    fs::permissions(cache, fs::owner_read | fs::group_read | fs::others_read);
+    // 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"));
-
-    fs::permissions(cache, fs::owner_read | fs::owner_write | fs::group_read | fs::others_read);
     CHECK(slurp(cache) == before);
+
+    fs::remove_all(blocker);
 }
 
 TEST_CASE("a cache written by another build's option ordering still loads", "[VendorCache]")