Files
OrcaSlicer/tests/libslic3r/test_instance_lock.cpp
T
Hanif Koh 5181a7fe26 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, which the OS releases when its holder dies. It is best
effort: when the lock file cannot be created, or another instance still
holds it after two seconds, the guard logs a warning and lets the write
proceed rather than letting a hung instance block every other one.

AppConfig::save() and load() hold OrcaSlicer.conf.lock; load is included
because the Windows path restores from the .bak copy, and because Windows
cannot replace a file another process has open, so an unlocked reader
there made the other instance's save fail. Every user preset writer and
reader holds user.lock: Preset::save(), save_info(), load_info() and
remove_files(), the whole directory scan in load_presets(), the bundle
metadata file, the .info removal after a cloud-confirmed delete and the
bundle folder removal on unsubscribe. The guard sits at the leaf writers
on purpose: set_sync_info_and_save() calls save_info() under the preset
collection mutex, so a batch lock around save_user_presets(), which takes
that mutex through delete_preset(), would invert the order.

Write preset JSON, .info and bundle metadata files through a temporary
file beside the target that is renamed over it, so a reader that never
waits still sees a complete old or new file. On Linux and macOS
rename_file() deleted the target before renaming, leaving a window in
which the file did not exist at all, and returned a meaningless error
code on failure; rename() replaces atomically, so call it directly.
AppConfig::save() now logs a failed final rename instead of dropping the
save silently.
2026-09-24 02:36:05 +08:00

136 lines
4.0 KiB
C++

#include <catch2/catch_all.hpp>
#include <atomic>
#include <chrono>
#include <thread>
#include <boost/filesystem.hpp>
#include "libslic3r/InstanceLock.hpp"
#include "test_utils.hpp"
#ifndef _WIN32
#include <fcntl.h>
#include <sys/wait.h>
#include <unistd.h>
#endif
using namespace Slic3r;
using namespace std::chrono_literals;
TEST_CASE("InstanceLock creates its lock file and holds it for the guard's scope", "[InstanceLock]")
{
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<bool> holder_ready{false};
std::atomic<bool> holder_released{false};
std::thread holder([&] {
InstanceLock lock(path);
holder_ready = true;
std::this_thread::sleep_for(150ms);
holder_released = true;
});
while (! holder_ready)
std::this_thread::yield();
bool released_before_acquire = false;
{
InstanceLock lock(path);
released_before_acquire = holder_released;
}
holder.join();
REQUIRE(released_before_acquire);
}
#ifndef _WIN32
// The cross-process side of the lock is a POSIX fcntl write lock, which a
// child process takes here directly; the same primitive backs the guard on
// Windows through LockFileEx, but spawning a child there is not worth a test.
TEST_CASE("InstanceLock yields to another process and reports it", "[InstanceLock]")
{
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