diff --git a/src/libslic3r/AppConfig.cpp b/src/libslic3r/AppConfig.cpp index c8a2c4984d..09eacac87d 100644 --- a/src/libslic3r/AppConfig.cpp +++ b/src/libslic3r/AppConfig.cpp @@ -5,6 +5,7 @@ //BBS #include "Preset.hpp" #include "Exception.hpp" +#include "InstanceLock.hpp" #include "LocalesUtils.hpp" #include "Thread.hpp" #include "format.hpp" @@ -734,6 +735,9 @@ std::string AppConfig::load() { json j; + // Keep another instance from replacing or restoring the file mid-read. + InstanceLock instance_lock(lock_path()); + // 1) Read the complete config file into a boost::property_tree. namespace pt = boost::property_tree; pt::ptree tree; @@ -983,6 +987,7 @@ void AppConfig::save() // The config is first written to a file with a PID suffix and then moved // to avoid race conditions with multiple instances of Slic3r const auto path = config_path(); + InstanceLock instance_lock(lock_path()); std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str(); json j; @@ -1142,7 +1147,8 @@ void AppConfig::save() // Rename the config atomically. // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile. // To cope with that, we already made a backup of the config on Windows. - rename_file(path_pid, path); + if (const std::error_code ec = rename_file(path_pid, path)) + BOOST_LOG_TRIVIAL(error) << "Failed to replace " << path << " with the new configuration: " << ec.message(); m_dirty = false; } @@ -1150,6 +1156,9 @@ void AppConfig::save() std::string AppConfig::load() { + // Keep another instance from replacing or restoring the file mid-read. + InstanceLock instance_lock(lock_path()); + // 1) Read the complete config file into a boost::property_tree. namespace pt = boost::property_tree; pt::ptree tree; @@ -1287,6 +1296,7 @@ void AppConfig::save() // The config is first written to a file with a PID suffix and then moved // to avoid race conditions with multiple instances of Slic3r const auto path = config_path(); + InstanceLock instance_lock(lock_path()); std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str(); std::stringstream config_ss; @@ -1351,7 +1361,8 @@ void AppConfig::save() // Rename the config atomically. // On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile. // To cope with that, we already made a backup of the config on Windows. - rename_file(path_pid, path); + if (const std::error_code ec = rename_file(path_pid, path)) + BOOST_LOG_TRIVIAL(error) << "Failed to replace " << path << " with the new configuration: " << ec.message(); m_dirty = false; } #endif @@ -1844,6 +1855,11 @@ void AppConfig::reset_selections() } } +std::string AppConfig::lock_path() +{ + return Slic3r::data_dir().empty() ? std::string() : config_path() + ".lock"; +} + std::string AppConfig::config_path() { #ifdef USE_JSON_CONFIG diff --git a/src/libslic3r/AppConfig.hpp b/src/libslic3r/AppConfig.hpp index 24a4c2069b..4c71aa9c03 100644 --- a/src/libslic3r/AppConfig.hpp +++ b/src/libslic3r/AppConfig.hpp @@ -338,6 +338,8 @@ public: // Get the default config path from Slic3r::data_dir(). std::string config_path(); + // Lock file guarding config_path() against other running instances; empty without a data dir. + std::string lock_path(); // Returns true if the user's data directory comes from before Slic3r 1.40.0 (no updating) bool legacy_datadir() const { return m_legacy_datadir; } diff --git a/src/libslic3r/CMakeLists.txt b/src/libslic3r/CMakeLists.txt index e734c036fa..6d5f850239 100644 --- a/src/libslic3r/CMakeLists.txt +++ b/src/libslic3r/CMakeLists.txt @@ -304,6 +304,8 @@ set(lisbslic3r_sources Geometry/VoronoiUtils.cpp Geometry/VoronoiUtils.hpp Geometry/VoronoiVisualUtils.hpp + InstanceLock.cpp + InstanceLock.hpp Int128.hpp KDTreeIndirect.hpp Layer.cpp diff --git a/src/libslic3r/Config.cpp b/src/libslic3r/Config.cpp index c8d816b3a4..177cd63c66 100644 --- a/src/libslic3r/Config.cpp +++ b/src/libslic3r/Config.cpp @@ -1522,12 +1522,10 @@ void ConfigBase::save_to_json(const std::string &file, const std::string &name, // Serialize first: if that throws (invalid UTF-8), the existing file stays untouched. std::ostringstream ss; this->save_to_json(ss, name, from, version); - boost::nowide::ofstream c; - 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; + 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; } 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 diff --git a/src/libslic3r/InstanceLock.cpp b/src/libslic3r/InstanceLock.cpp new file mode 100644 index 0000000000..f0d55077dc --- /dev/null +++ b/src/libslic3r/InstanceLock.cpp @@ -0,0 +1,101 @@ +#include "InstanceLock.hpp" + +#include +#include +#include +#include + +#include +#include +#include +#ifdef _WIN32 +#include +#endif + +namespace Slic3r { + +// One slot per lock file, shared by every guard in the process. A POSIX fcntl +// lock is owned by the process, not the thread or descriptor, so a single +// file_lock per path behind a mutex is what makes the guard re-entrant and +// safe to use from the preset sync thread and the GUI thread at once. +struct InstanceLock::Slot +{ + std::recursive_mutex mutex; + // Absent when the lock file could not be created or opened. + std::unique_ptr file_lock; + int depth{0}; + bool file_locked{false}; +}; + +InstanceLock::Slot &InstanceLock::slot_for(const std::string &lock_file_path) +{ + static std::mutex registry_mutex; + // Never freed: the slots keep the lock files open for as long as anything + // in the process may still save, including during static destruction. + static auto *registry = new std::map>(); + + std::lock_guard guard(registry_mutex); + std::unique_ptr &slot = (*registry)[lock_file_path]; + if (! slot) { + slot = std::make_unique(); + try { + // file_lock only opens existing files. + boost::nowide::ofstream(lock_file_path, std::ios::app).close(); +#ifdef _WIN32 + slot->file_lock = std::make_unique(boost::nowide::widen(lock_file_path).c_str()); +#else + slot->file_lock = std::make_unique(lock_file_path.c_str()); +#endif + } catch (const std::exception &e) { + BOOST_LOG_TRIVIAL(warning) << "Cannot open lock file " << lock_file_path << ": " << e.what() + << "; other instances are not excluded from writing"; + } + } + return *slot; +} + +InstanceLock::InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout) +{ + if (lock_file_path.empty()) + return; + m_slot = &slot_for(lock_file_path); + m_slot->mutex.lock(); + if (m_slot->depth++ == 0 && m_slot->file_lock) { + const auto deadline = std::chrono::steady_clock::now() + timeout; + for (;;) { + try { + if (m_slot->file_lock->try_lock()) { + m_slot->file_locked = true; + break; + } + } catch (const std::exception &e) { + BOOST_LOG_TRIVIAL(warning) << "Cannot lock " << lock_file_path << ": " << e.what(); + break; + } + if (std::chrono::steady_clock::now() >= deadline) { + BOOST_LOG_TRIVIAL(warning) << "Another instance has held " << lock_file_path << " for over " + << timeout.count() << " ms; writing without the lock"; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + } + m_locked = m_slot->file_locked; +} + +InstanceLock::~InstanceLock() +{ + if (m_slot == nullptr) + return; + if (--m_slot->depth == 0 && m_slot->file_locked) { + try { + m_slot->file_lock->unlock(); + } catch (const std::exception &e) { + BOOST_LOG_TRIVIAL(warning) << "Cannot unlock instance lock: " << e.what(); + } + m_slot->file_locked = false; + } + m_slot->mutex.unlock(); +} + +} // namespace Slic3r diff --git a/src/libslic3r/InstanceLock.hpp b/src/libslic3r/InstanceLock.hpp new file mode 100644 index 0000000000..936cafdac9 --- /dev/null +++ b/src/libslic3r/InstanceLock.hpp @@ -0,0 +1,42 @@ +#pragma once + +#include +#include + +namespace Slic3r { + +// Scoped write lock on a file shared by every running instance of the +// application, such as the app config or the user preset directory. Threads +// of this process are serialised through a recursive mutex, other processes +// through an advisory OS file lock on `lock_file_path`, created on first use. +// The OS releases the file lock when its holder exits, so a crashed instance +// never leaves a stale lock behind. +// +// The lock is best effort: when the lock file cannot be created, or another +// instance still holds it after `timeout`, the guard keeps only the in-process +// mutex and locked() reports false. Writes then proceed unprotected rather than +// letting one hung instance block every other one from saving. +class InstanceLock +{ +public: + static constexpr std::chrono::milliseconds default_timeout{2000}; + + // An empty path makes the guard a no-op. + explicit InstanceLock(const std::string &lock_file_path, std::chrono::milliseconds timeout = default_timeout); + ~InstanceLock(); + + InstanceLock(const InstanceLock &) = delete; + InstanceLock &operator=(const InstanceLock &) = delete; + + // True while this process holds the cross-process file lock. + bool locked() const { return m_locked; } + +private: + struct Slot; + static Slot &slot_for(const std::string &lock_file_path); + + Slot *m_slot{nullptr}; + bool m_locked{false}; +}; + +} // namespace Slic3r diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index 1002f5be89..b38c878cdb 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -48,6 +48,9 @@ #include "libslic3r.h" #include "Utils.hpp" +#include "InstanceLock.hpp" + +#include #include "Time.hpp" #include "PlaceholderParser.hpp" #include "libslic3r/GCode/Thumbnails.hpp" @@ -104,6 +107,11 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const } } +std::string user_presets_lock_path() +{ + return data_dir().empty() ? std::string() : (fs::path(data_dir()) / (PRESET_USER_DIR ".lock")).string(); +} + std::string get_preset_bare_name(const std::string &canonical_name) { const auto pos = canonical_name.find_last_of('/'); @@ -603,6 +611,7 @@ Preset::Type Preset::get_type_from_string(std::string type_str) void Preset::load_info(const std::string& file) { + InstanceLock instance_lock(user_presets_lock_path()); try { boost::property_tree::ptree tree; boost::nowide::ifstream ifs(file); @@ -646,18 +655,20 @@ void Preset::save_info(std::string file) file = idx_file.string(); } - boost::nowide::ofstream c; - c.open(file, std::ios::out | std::ios::trunc); std::string sync_info_to_save; //BBS: hold is used for stop requesting to server this time if (this->sync_info.compare("hold") != 0) sync_info_to_save = this->sync_info; + std::ostringstream c; c << "sync_info" << " = " << sync_info_to_save << std::endl; c << "user_id" << " = " << this->user_id << std::endl; c << "setting_id" << " = " << this->setting_id << std::endl; c << "base_id" << " = " << this->base_id << std::endl; c << "updated_time" << " = " << std::to_string(this->updated_time) << std::endl; - c.close(); + + InstanceLock instance_lock(user_presets_lock_path()); + if (const std::error_code ec = write_file_atomically(file, c.str())) + BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << file << ": " << ec.message(); } void Preset::remove_files(bool cloud_already_deleted) @@ -666,6 +677,7 @@ void Preset::remove_files(bool cloud_already_deleted) if (this->is_project_embedded) { return; } + InstanceLock instance_lock(user_presets_lock_path()); // Erase the preset file. boost::nowide::remove(this->file.c_str()); fs::path idx_path(this->file); @@ -702,6 +714,7 @@ void Preset::save(DynamicPrintConfig* parent_config) else from_str = std::string("Default"); + InstanceLock instance_lock(user_presets_lock_path()); boost::filesystem::create_directories(fs::path(this->file).parent_path()); const std::string bare_name = get_preset_bare_name(this->name); @@ -1704,6 +1717,8 @@ void PresetCollection::load_presets( std::set *key_set1 = nullptr, *key_set2 = nullptr; Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2); + // Held across the scan so no instance replaces or removes a file mid-read. + InstanceLock instance_lock(user_presets_lock_path()); //BBS: change to json format for (auto &dir_entry : boost::filesystem::directory_iterator(dir)) { diff --git a/src/libslic3r/Preset.hpp b/src/libslic3r/Preset.hpp index c66518e29d..aef97021b7 100644 --- a/src/libslic3r/Preset.hpp +++ b/src/libslic3r/Preset.hpp @@ -484,6 +484,10 @@ std::string get_preset_canonical_name(const std::string &preset_bare_name, const // Tail segment of a canonical name — what's written to the bundle's .json filename and JSON "name" field. std::string get_preset_bare_name(const std::string &canonical_name); +// Lock file guarding every user preset file under data_dir() against other +// running instances and the preset sync thread; empty without a data dir. +std::string user_presets_lock_path(); + // Resolve an origin from a directory path when the caller passes Kind::Auto. PresetOrigin detect_origin_from_path(const boost::filesystem::path &path, const PresetOrigin &explicit_origin = PresetOrigin()); diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index dc731b63c4..013e55fcd6 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -12,6 +12,7 @@ #include "libslic3r.h" #include "I18N.hpp" #include "Utils.hpp" +#include "InstanceLock.hpp" #include "LocalesUtils.hpp" #include "Model.hpp" #include "TriangleSelector.hpp" @@ -2231,6 +2232,7 @@ void PresetBundle::remove_user_presets_directory(const std::string preset_folder return; } BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(" enter, delete directory : %1%") % dir_user_presets; + InstanceLock instance_lock(user_presets_lock_path()); fs::path folder(dir_user_presets); if (fs::exists(folder)) { fs::remove_all(folder); @@ -7867,6 +7869,7 @@ bool PresetBundle::check_duplicate_filament_subtypes() const // Orca: BundleMetadata method implementations bool BundleMetadata::load_from_json(const std::string& path) { + InstanceLock instance_lock(user_presets_lock_path()); try { boost::nowide::ifstream ifs(path); if (!ifs.good()) @@ -7928,13 +7931,16 @@ bool BundleMetadata::save_to_json(const std::string& path) const j["imported_time"] = this->imported_time; j["updated_time"] = this->updated_time; + InstanceLock instance_lock(user_presets_lock_path()); j["print_presets"] = strip_prefix(this->print_presets); j["filament_presets"] = strip_prefix(this->filament_presets); j["printer_presets"] = strip_prefix(this->printer_presets); - boost::nowide::ofstream ofs(path); - ofs << j.dump(4); - return ofs.good(); + if (const std::error_code ec = write_file_atomically(path, j.dump(4))) { + BOOST_LOG_TRIVIAL(error) << "Failed to save bundle metadata to " << path << ": " << ec.message(); + return false; + } + return true; } catch (const std::exception& e) { BOOST_LOG_TRIVIAL(error) << "Failed to save bundle metadata to " << path << ": " << e.what(); return false; diff --git a/src/libslic3r/Utils.hpp b/src/libslic3r/Utils.hpp index b21da72fc8..64950a64f5 100644 --- a/src/libslic3r/Utils.hpp +++ b/src/libslic3r/Utils.hpp @@ -224,6 +224,10 @@ extern std::vector split_string(const std::string &str, char delimi // On Windows, the file explorer (or anti-virus or whatever else) often locks the file // for a short while, so the file may not be movable. Retry while we see recoverable errors. extern std::error_code rename_file(const std::string &from, const std::string &to); +// Write `content` to `path` through a temporary file beside it that is then renamed +// over the target, so a concurrent reader sees the old or the new file, never a +// partial one. The temporary file is removed on failure. +extern std::error_code write_file_atomically(const std::string &path, const std::string &content); enum CopyFileResult { SUCCESS = 0, diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 9def5dad17..97c619c517 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -704,11 +705,30 @@ std::error_code rename_file(const std::string &from, const std::string &to) #ifdef _WIN32 return WindowsSupport::rename(from, to); #else - boost::nowide::remove(to.c_str()); - return std::make_error_code(static_cast(boost::nowide::rename(from.c_str(), to.c_str()))); + // rename(2) replaces an existing target atomically; removing it first would + // leave a window in which the file does not exist at all. + return std::make_error_code(static_cast(boost::nowide::rename(from.c_str(), to.c_str()) == 0 ? 0 : errno)); #endif } +std::error_code write_file_atomically(const std::string &path, const std::string &content) +{ + const std::string tmp_path = path + "." + std::to_string(get_current_pid()) + ".tmp"; + { + boost::nowide::ofstream out(tmp_path, std::ios::out | std::ios::trunc); + out << content; + out.close(); + if (out.fail()) { + boost::nowide::remove(tmp_path.c_str()); + return std::make_error_code(std::errc::io_error); + } + } + std::error_code ec = rename_file(tmp_path, path); + if (ec) + boost::nowide::remove(tmp_path.c_str()); + return ec; +} + #ifdef __linux__ // Copied from boost::filesystem. // Called by copy_file_linux() in case linux sendfile() API is not supported. diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 48bd58b1a5..2df8dca3b4 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -85,6 +85,7 @@ #include "libslic3r/Model.hpp" #include "libslic3r/I18N.hpp" #include "libslic3r/PresetBundle.hpp" +#include "libslic3r/InstanceLock.hpp" #include "libslic3r/Thread.hpp" #include "libslic3r/miniz_extension.hpp" #include "libslic3r/Utils.hpp" @@ -7624,7 +7625,10 @@ void GUI_App::start_sync_user_preset(bool with_progress_dlg) // Delete the bundle folder and bundle fs::path bundle_folder = fs::path(bundle.path.c_str()).parent_path(); boost::system::error_code ec; - boost::filesystem::remove_all(bundle_folder, ec); + { + InstanceLock instance_lock(user_presets_lock_path()); + boost::filesystem::remove_all(bundle_folder, ec); + } preset_bundle->bundles.WriteLock(); preset_bundle->bundles.m_bundles.erase(bundle.id); @@ -8889,6 +8893,7 @@ void GUI_App::preset_deleted_from_cloud(std::string setting_id) // Delete the .info file after cloud deletion is confirmed if (!preset_file_path.empty() && fs::exists(fs::path(preset_file_path))) { + InstanceLock instance_lock(user_presets_lock_path()); boost::nowide::remove(preset_file_path.c_str()); BOOST_LOG_TRIVIAL(info) << "Deleted .info file after cloud confirmation: " << preset_file_path; } diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index 185dce37da..c064d9e5e3 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -49,6 +49,7 @@ add_executable(${_TEST_NAME}_tests test_ordering_strategies.cpp # test_png_io.cpp test_indexed_triangle_set.cpp + test_instance_lock.cpp ../libnest2d/printer_parts.cpp ) diff --git a/tests/libslic3r/test_instance_lock.cpp b/tests/libslic3r/test_instance_lock.cpp new file mode 100644 index 0000000000..e28fa3420c --- /dev/null +++ b/tests/libslic3r/test_instance_lock.cpp @@ -0,0 +1,135 @@ +#include + +#include +#include +#include + +#include + +#include "libslic3r/InstanceLock.hpp" +#include "test_utils.hpp" + +#ifndef _WIN32 +#include +#include +#include +#endif + +using namespace Slic3r; +using namespace std::chrono_literals; + +TEST_CASE("InstanceLock creates its lock file and holds it for the guard's scope", "[InstanceLock]") +{ + ScopedTemporaryDir dir; + const std::string path = (dir.path() / "shared.lock").string(); + + { + InstanceLock lock(path); + REQUIRE(lock.locked()); + REQUIRE(boost::filesystem::exists(path)); + } + // Released: a fresh guard gets the lock at once instead of waiting out a timeout. + const auto started = std::chrono::steady_clock::now(); + InstanceLock again(path, 5000ms); + REQUIRE(again.locked()); + REQUIRE(std::chrono::steady_clock::now() - started < 1000ms); +} + +TEST_CASE("InstanceLock nests within one thread", "[InstanceLock]") +{ + ScopedTemporaryDir dir; + const std::string path = (dir.path() / "shared.lock").string(); + + InstanceLock outer(path); + { + InstanceLock inner(path, 100ms); + REQUIRE(inner.locked()); + } + // The inner guard leaving does not release the outer one. + REQUIRE(outer.locked()); +} + +TEST_CASE("InstanceLock is a no-op for an empty path and survives an unwritable one", "[InstanceLock]") +{ + ScopedTemporaryDir dir; + + InstanceLock none(""); + REQUIRE_FALSE(none.locked()); + + // The directory does not exist, so the lock file cannot be created; the + // guard still constructs and the write it guards can go ahead. + InstanceLock unwritable((dir.path() / "missing" / "shared.lock").string(), 100ms); + REQUIRE_FALSE(unwritable.locked()); +} + +TEST_CASE("InstanceLock serialises the threads of one process", "[InstanceLock]") +{ + ScopedTemporaryDir dir; + const std::string path = (dir.path() / "shared.lock").string(); + + std::atomic holder_ready{false}; + std::atomic holder_released{false}; + std::thread holder([&] { + InstanceLock lock(path); + holder_ready = true; + std::this_thread::sleep_for(150ms); + holder_released = true; + }); + while (! holder_ready) + std::this_thread::yield(); + + bool released_before_acquire = false; + { + InstanceLock lock(path); + released_before_acquire = holder_released; + } + holder.join(); + REQUIRE(released_before_acquire); +} + +#ifndef _WIN32 +// The cross-process side of the lock is a POSIX fcntl write lock, which a +// child process takes here directly; the same primitive backs the guard on +// Windows through LockFileEx, but spawning a child there is not worth a test. +TEST_CASE("InstanceLock yields to another process and reports it", "[InstanceLock]") +{ + ScopedTemporaryDir dir; + const std::string path = (dir.path() / "shared.lock").string(); + + int child_holds[2], child_may_exit[2]; + REQUIRE(::pipe(child_holds) == 0); + REQUIRE(::pipe(child_may_exit) == 0); + + const pid_t child = ::fork(); + REQUIRE(child >= 0); + if (child == 0) { + int fd = ::open(path.c_str(), O_RDWR | O_CREAT, 0644); + struct flock lock{}; + lock.l_type = F_WRLCK; + lock.l_whence = SEEK_SET; + char byte = ::fcntl(fd, F_SETLK, &lock) == 0 ? '1' : '0'; + if (::write(child_holds[1], &byte, 1) != 1 || ::read(child_may_exit[0], &byte, 1) != 1) + ::_exit(1); + ::_exit(0); + } + + char byte = '0'; + REQUIRE(::read(child_holds[0], &byte, 1) == 1); + REQUIRE(byte == '1'); + + bool locked_while_child_holds; + { + InstanceLock lock(path, 100ms); + locked_while_child_holds = lock.locked(); + } + 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); + InstanceLock lock(path); + REQUIRE(lock.locked()); +} +#endif diff --git a/tests/libslic3r/test_utils.cpp b/tests/libslic3r/test_utils.cpp index 7880b783f1..36065dbd50 100644 --- a/tests/libslic3r/test_utils.cpp +++ b/tests/libslic3r/test_utils.cpp @@ -62,6 +62,32 @@ 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"; + + REQUIRE(write_file_atomically(target.string(), "x")); + REQUIRE_FALSE(boost::filesystem::exists(target)); +} + TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") { ScopedTemporaryFile source(".txt"); {