mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-05 06:41:10 +00:00
Lock Config and Preset Files Across Instances and Write Them Atomically (#15861)
* Lock Config and Preset Files Across Instances and Write Them Atomically Every running instance shares one OrcaSlicer.conf and one user preset tree, and nothing kept their writers apart. Two instances saving at the same moment, or the cloud preset sync thread writing while the GUI thread saved, could interleave, and a reader in another instance could open a preset JSON or .info file between truncate and close and get a partial file, dropping that preset for the session with a parse error. Add InstanceLock, a scoped guard that serialises the threads of one process through a recursive mutex and other processes through an advisory OS file lock: flock on POSIX, held on the guard's own descriptor so no other close in the process can drop it, and LockFileEx on Windows. The outermost guard opens the lock file and closes it on release, so nothing stays open between saves and a data dir can be removed once nothing is saving into it; the file itself is kept, since deleting it would let a third instance lock a fresh file while the second still holds the old one. It is best effort: when the lock file cannot be opened or locked, or another instance still holds it after a second, the guard logs once and lets the write proceed, then leaves the file alone for ten seconds, so a hung instance never blocks every other one and a holder stuck in a debugger does not cost a stall per save. The guard sits at the leaf readers and writers: set_sync_info_and_save() calls save_info() under the preset collection mutex, so a batch lock around save_user_presets() would invert the order against the sync thread. The user preset scan reads its files on worker threads without the guard, since the mutex would serialise them, and takes it per file in the serial commit step, so a save never waits for the whole scan. Each read keeps the bytes of the preset and its .info as they were before parsing; commit compares them with the disk under the guard and reads a file that changed again, so it never deletes or writes back over another instance's newer save, nor installs a .json and .info from two different saves; a preset another instance removed in the meantime is not installed. Without the guard, in a cool-down, the scan still loads the presets but leaves their files alone: an unreadable file stays for the next scan, and a derived compatible printer is not written back. Read-only scans, which is what the CLI does, take no lock and create no lock file. AppConfig holds OrcaSlicer.conf.lock in load() and save(); load is included because the Windows path restores from the .bak copy. Every user preset writer and reader holds user.lock: Preset::save(), which writes no .info when the preset itself could not be written, since an .info without its preset reads as a cloud deletion request, save_info(), reload() and remove_files(), each preset the scan commits, the bundle metadata reads and write, the .info removal after a cloud-confirmed delete, the orphaned-.info scan on the sync thread, the bundle folder removal on unsubscribe and the physical printer writers and delete paths. A bundle import extracts under cache/ into a folder per process and per import, where no scan reads. Preset JSON, .info, bundle metadata, physical printer and config files, and the caches and state files that already used a temporary by hand, now go through write_file_atomically(), which writes <file>.<pid>.<n>.tmp beside the target and renames it over, so a reader that never waits sees a complete old or new file. A symlink is followed; a target that is not a regular file is written in place; and when no temporary can be created beside an existing target, or the rename itself is refused, by a Windows reader holding the file open or a mount that cannot replace in one step, the helper writes in place as before, since losing the save is worse than a torn read. On POSIX the rename replaces the target atomically where the old code removed it first and left a window with no file at all; only a mount that refuses a one-step replace gets the old remove-then-rename. A crash between temporary and rename leaves the temporary behind, which no scan reads. Preset::save() returns whether it wrote the preset, so the scan counts a compatible printer it could not write back as an error. A failed config write keeps the config dirty, and the idle handler waits ten seconds before retrying while an explicit save always tries. * Run the Cross-Process Lock Test on Every Platform The test that checks the guard yields to a lock held elsewhere forked a child to hold it, so it was left out on Windows. The OS lock belongs to the handle on Windows and to the open file description elsewhere, so a second handle in the same process is refused like another instance would be. The test now holds the lock that way and runs everywhere.
This commit is contained in:
+48
-66
@@ -5,6 +5,7 @@
|
||||
//BBS
|
||||
#include "Preset.hpp"
|
||||
#include "Exception.hpp"
|
||||
#include "InstanceLock.hpp"
|
||||
#include "LocalesUtils.hpp"
|
||||
#include "Thread.hpp"
|
||||
#include "format.hpp"
|
||||
@@ -743,10 +744,13 @@ static bool verify_config_file_checksum(boost::nowide::ifstream &ifs)
|
||||
|
||||
|
||||
#ifdef USE_JSON_CONFIG
|
||||
std::string AppConfig::load()
|
||||
std::string AppConfig::load(bool read_only)
|
||||
{
|
||||
json j;
|
||||
|
||||
// Keep another instance from replacing or restoring the file mid-read.
|
||||
InstanceLock instance_lock(read_only ? std::string() : lock_path());
|
||||
|
||||
// 1) Read the complete config file into a boost::property_tree.
|
||||
namespace pt = boost::property_tree;
|
||||
pt::ptree tree;
|
||||
@@ -996,7 +1000,6 @@ 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;
|
||||
|
||||
@@ -1126,43 +1129,18 @@ void AppConfig::save()
|
||||
|
||||
j["local_machines"][local_machine.first] = m_json;
|
||||
}
|
||||
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;
|
||||
const std::string config_str = j.dump(1, '\t');
|
||||
if (write_config_file(path, config_str + "\n", config_str))
|
||||
m_dirty = false;
|
||||
}
|
||||
|
||||
#else
|
||||
|
||||
std::string AppConfig::load()
|
||||
std::string AppConfig::load(bool read_only)
|
||||
{
|
||||
// Keep another instance from replacing or restoring the file mid-read.
|
||||
InstanceLock instance_lock(read_only ? std::string() : lock_path());
|
||||
|
||||
// 1) Read the complete config file into a boost::property_tree.
|
||||
namespace pt = boost::property_tree;
|
||||
pt::ptree tree;
|
||||
@@ -1300,7 +1278,6 @@ 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)
|
||||
@@ -1336,39 +1313,39 @@ void AppConfig::save()
|
||||
// One empty line before the MD5 sum.
|
||||
config_ss << std::endl;
|
||||
|
||||
std::string config_str = config_ss.str();
|
||||
boost::nowide::ofstream c;
|
||||
c.open(path_pid, std::ios::out | std::ios::trunc);
|
||||
c << config_str;
|
||||
#ifdef WIN32
|
||||
// WIN32 specific: The final "rename_file()" call is not safe in case of an application crash, there is no atomic "rename file" API
|
||||
// provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
|
||||
// we save the config file into a backup first before moving it to the final destination.
|
||||
c << appconfig_md5_hash_line(config_str);
|
||||
#endif
|
||||
c.close();
|
||||
if (c.fail()) {
|
||||
BOOST_LOG_TRIVIAL(error) << "Failed to write new configuration to " << path_pid << "; aborting attempt to overwrite original configuration";
|
||||
return;
|
||||
}
|
||||
|
||||
#ifdef WIN32
|
||||
// Make a backup of the configuration file before copying it to the final destination.
|
||||
std::string error_message;
|
||||
std::string backup_path = (boost::format("%1%.bak") % path).str();
|
||||
// Copy configuration file with PID suffix into the configuration file with "bak" suffix.
|
||||
if (copy_file(path_pid, backup_path, error_message, false) != SUCCESS)
|
||||
BOOST_LOG_TRIVIAL(error) << "Copying from " << path_pid << " to " << backup_path << " failed. Failed to create a backup configuration.";
|
||||
#endif
|
||||
|
||||
// Rename the config atomically.
|
||||
// On Windows, the rename is likely NOT atomic, thus it may fail if PrusaSlicer crashes on another thread in the meanwhile.
|
||||
// To cope with that, we already made a backup of the config on Windows.
|
||||
rename_file(path_pid, path);
|
||||
m_dirty = false;
|
||||
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());
|
||||
#ifdef WIN32
|
||||
// WIN32 specific: the final replace is not safe in case of an application crash, there is no atomic "rename file" API
|
||||
// provided by Windows (sic!). Therefore we save a MD5 checksum to be able to verify file corruption. In addition,
|
||||
// we save the config file into a backup first before moving it to the final destination.
|
||||
body += appconfig_md5_hash_line(checksum_source);
|
||||
#endif
|
||||
// Not flushed to the device: the idle handler saves on the GUI thread after
|
||||
// any change, and the rename already gives a complete old or new file.
|
||||
if (const std::error_code ec = write_file_atomically(path, body)) {
|
||||
BOOST_LOG_TRIVIAL(error) << "Failed to write the configuration " << path << ": " << ec.message() << "; trying again in 10 s";
|
||||
m_retry_save_at = std::chrono::steady_clock::now() + std::chrono::seconds(10);
|
||||
return false;
|
||||
}
|
||||
m_retry_save_at = {};
|
||||
#ifdef WIN32
|
||||
// Written after the config, so the backup never holds a state that was not confirmed written.
|
||||
const std::string backup_path = (boost::format("%1%.bak") % path).str();
|
||||
if (const std::error_code ec = write_file_atomically(backup_path, body))
|
||||
BOOST_LOG_TRIVIAL(error) << "Failed to write the backup configuration " << backup_path << ": " << ec.message();
|
||||
#endif
|
||||
return true;
|
||||
}
|
||||
|
||||
bool AppConfig::get_variant(const std::string &vendor, const std::string &model, const std::string &variant) const
|
||||
{
|
||||
const auto it_v = m_vendors.find(vendor);
|
||||
@@ -1857,6 +1834,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
|
||||
@@ -1893,7 +1875,7 @@ bool AppConfig::exists()
|
||||
|
||||
std::string AppConfig::load_if_exists()
|
||||
{
|
||||
return boost::filesystem::exists(loading_path()) ? load() : std::string();
|
||||
return boost::filesystem::exists(loading_path()) ? load(/*read_only=*/true) : std::string();
|
||||
}
|
||||
|
||||
}; // namespace Slic3r
|
||||
|
||||
Reference in New Issue
Block a user