folkslinux
/
mamba
mirror of https://github.com/mamba-org/mamba.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Changed LockFile to be a non-throwing checkable type, no pointers use. 

This changes how we use LockFile. See documentation/comments in the
type's header `util.hpp`.
This commit is contained in:
Joël Lamotte (Klaim) 2022-10-11 19:01:29 +02:00 committed by Joël Lamotte (Klaim)
parent 7e939f5096
commit 2b7b230613
12 changed files with 215 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
libmamba/include/mamba/core/error_handling.hpp
View File

@ -27,6 +27,7 @@ namespace mamba
env_lockfile_parsing_failed,
openssl_failed,
internal_failure,
lockfile_failure,
};
class mamba_error : public std::runtime_error

95
libmamba/include/mamba/core/util.hpp
View File

@ -12,6 +12,7 @@
#include "mamba/core/util_string.hpp"
#include "nlohmann/json.hpp"
#include "tl/expected.hpp"
#include <array>
#include <limits>
@ -47,6 +48,14 @@ namespace mamba
#error "no supported OS detected"
#endif
// Used when we want a callback which does nothing.
struct no_op
{
void operator()() const noexcept
{
}
};
bool is_package_file(const std::string_view& fn);
bool lexists(const fs::u8path& p);
@ -103,9 +112,62 @@ namespace mamba
class LockFileOwner;
// This is a non-throwing file-locking mechanism.
// It can be used on a file or directory path. In the case of a directory path a file will be
// created to be locked. The locking will be implemented using the OS's filesystem locking
// capabilities, if available.
//
// Once constructed, use `is_locked()` or `operator bool` to check if the lock did happen
// successfully. When locking fails because of an error, the error can be retrieved using
// `error()`. When attempting to lock a path which is already locked by another process, the
// attempt will fail and `is_locked()` will return false.
//
// When the same process attempts to lock the same path more than once (multiple instances of
// `LockFile` target the same path), creating a new `LockFile` for that path will always succeed
// and increment the lock owner count which can be retrieved using `count_lock_owners()`.
// Basically, all instacnes of `LockFile` locking the same path are sharing the lock, which will
// only be released once there is no instance alive.
//
// Use `Context::instance().use_lockfiles = false` to never have locking happen, in which case
// the created `LockFile` instance will not be locked (`is_locked()` will return false) but will
// have no error either (`error()` will return `noopt`).
//
// Example:
//
// LockFile some_work_on(some_path)
// {
// LockFile lock{ some_path, timeout };
// if(lock) // make sure the locking happened
// {
// print("locked file {}, locking counts: {}", some_path,
// lock.count_lock_owners()); // success might mean we are locking the same path
// from multiple threads do_something(som_path); // locking was a success
// }
// else // locking didnt succeed for some reason
// {
// if(auto error = lock.error) print(error); // some error happened while
// attempting the lock, maybe some other process already locks the path else
// print("didn't attempt locking {}") // locking didn't happen for some other
// reason, maybe a configuration option
// }
// some_more_work(some_path); // do this that the lock failed or not
// return lock; // The locking ownership can be transfered to another function if
// necessary
// }
//
class LockFile
{
public:
// Non-throwing constructors, attempting lock on the provided path, file or directory.
// In case of a directory, a lock-file will be created, located at `this->lockfile_path()`
// and `this->is_locked()` (and `if(*this))` will always return true (unless this instance
// is moved-from). If the lock acquisition failed or `Context::instance().use_lockfiles ==
// false` and until re-assigned:
// - `this->is_locked() == false` and `if(*this) ...` will go in the `false` branch.
// - accessors will throw, except `is_locked()`, `count_lock_owners()`, and `error()`
LockFile(const fs::u8path& path);
LockFile(const fs::u8path& path, const std::chrono::seconds& timeout);
~LockFile();
LockFile(const LockFile&) = delete;
@ -114,15 +176,13 @@ namespace mamba
LockFile(LockFile&&);
LockFile& operator=(LockFile&&);
static std::unique_ptr<LockFile> create_lock(const fs::u8path& path);
static std::unique_ptr<LockFile> try_lock(const fs::u8path& path) noexcept;
// Returns true if this LockFile is currently maintaining a lock on the target path.
// Returns false if this instance have been moved-from without being re-assigned,
// or if the lock acquisition failed.
bool is_locked() const
{
return impl ? true: false;
return impl.has_value() // we have a owner
&& (impl.value() ? true : false); // it's not null
}
// Convenient operator to check if a lockfile is actually locking a path.
@ -131,13 +191,13 @@ namespace mamba
return is_locked();
}
// Returns the fd of the path being locked if `is_locked() == true`.
// Returns the fd of the path being locked, throws if `is_locked() == false`.
int fd() const;
// Returns the path being locked if `is_locked() == true`.
// Returns the path being locked, throws if `is_locked() == false`.
fs::u8path path() const;
// Returns the path of the lock-file being locked if `is_locked() == true`.
// Returns the path of the lock-file being locked, throws if `is_locked() == false`.
fs::u8path lockfile_path() const;
// Returns the count of LockFile instances which are currently locking
@ -145,7 +205,7 @@ namespace mamba
// Returns 0 if `is_locked() == false`.
std::size_t count_lock_owners() const
{
return impl.use_count();
return impl.has_value() ? impl.value().use_count() : 0;
}
#ifdef _WIN32
@ -158,18 +218,25 @@ namespace mamba
static bool is_locked(const LockFile& lockfile)
{
return lockfile.is_locked() &&
#ifdef _WIN32
return is_locked(lockfile.lockfile_path());
is_locked(lockfile.lockfile_path());
#else
// Opening a new file descriptor on Unix would clear locks
return is_locked(lockfile.fd());
// Opening a new file descriptor on Unix would clear locks
is_locked(lockfile.fd());
#endif
}
std::optional<mamba_error> error() const
{
if (impl.has_value())
return {};
else
return impl.error();
}
private:
LockFile(const fs::u8path& path);
LockFile(const fs::u8path& path, const std::chrono::seconds& timeout);
std::shared_ptr<LockFileOwner> impl;
tl::expected<std::shared_ptr<LockFileOwner>, mamba_error> impl;
};

2
libmamba/src/core/package_handling.cpp
View File

@ -305,7 +305,7 @@ namespace mamba
archive_write_disk_set_options(ext, flags);
archive_write_disk_set_standard_lookup(ext);
auto lock = LockFile::try_lock(file);
auto lock = LockFile(file);
r = archive_read_open_filename(a, file.string().c_str(), 10240);
if (r != ARCHIVE_OK)

4
libmamba/src/core/repo.cpp
View File

@ -317,7 +317,7 @@ namespace mamba
if (is_solv)
{
auto lock = LockFile::try_lock(m_solv_file);
auto lock = LockFile(m_solv_file);
#ifdef _WIN32
auto fp = _wfopen(m_solv_file.wstring().c_str(), L"rb");
#else
@ -393,7 +393,7 @@ namespace mamba
fclose(fp);
}
auto lock = LockFile::try_lock(m_json_file);
auto lock = LockFile(m_json_file);
#ifdef _WIN32
auto fp = _wfopen(m_json_file.wstring().c_str(), L"r");
#else

10
libmamba/src/core/subdirdata.cpp
View File

@ -309,7 +309,7 @@ namespace mamba
if (!fs::exists(json_file, ec))
continue;
auto lock = LockFile::try_lock(cache_path / "cache");
auto lock = LockFile(cache_path / "cache");
auto cache_age = check_cache(json_file, now);
if (cache_age != fs::file_time_type::duration::max() && !forbid_cache())
@ -480,7 +480,7 @@ namespace mamba
LOG_DEBUG << "Copying repodata cache files from '" << m_expired_cache_path.string()
<< "' to '" << m_writable_pkgs_dir.string() << "'";
fs::u8path writable_cache_dir = create_cache_dir(m_writable_pkgs_dir);
auto lock = LockFile::create_lock(writable_cache_dir);
auto lock = LockFile(writable_cache_dir);
auto copied_json_file = writable_cache_dir / m_json_fn;
if (fs::exists(copied_json_file))
@ -502,13 +502,13 @@ namespace mamba
{
LOG_TRACE << "Refreshing '" << json_file.string() << "'";
auto lock = LockFile::create_lock(json_file);
auto lock = LockFile(json_file);
fs::last_write_time(json_file, now);
}
if (fs::exists(solv_file) && solv_age.count() <= json_age.count())
{
LOG_TRACE << "Refreshing '" << solv_file.string() << "'";
auto lock = LockFile::create_lock(solv_file);
auto lock = LockFile(solv_file);
fs::last_write_time(solv_file, now);
m_solv_cache_valid = true;
}
@ -545,7 +545,7 @@ namespace mamba
fs::u8path writable_cache_dir = create_cache_dir(m_writable_pkgs_dir);
json_file = writable_cache_dir / m_json_fn;
auto lock = LockFile::create_lock(writable_cache_dir);
auto lock = LockFile(writable_cache_dir);
m_mod_etag.clear();
m_mod_etag["_url"] = m_repodata_url;

2
libmamba/src/core/transaction.cpp
View File

@ -921,7 +921,7 @@ namespace mamba
return true;
}
auto lf = LockFile::create_lock(ctx.target_prefix / "conda-meta");
auto lf = LockFile(ctx.target_prefix / "conda-meta");
clean_trash_files(ctx.target_prefix, false);
Console::stream() << "\nTransaction starting";

104
libmamba/src/core/util.cpp
View File

@ -52,7 +52,7 @@ extern "C"
#include "mamba/core/fsutil.hpp"
#include "mamba/core/url.hpp"
#include "mamba/core/shell_init.hpp"
#include "mamba/core/invoke.hpp"
namespace mamba
{
@ -812,6 +812,18 @@ namespace mamba
int m_fd = -1;
bool m_locked;
bool m_lockfile_existed;
template <typename Func = no_op>
void throw_lock_error(std::string error_message, Func before_throw_task = no_op{}) const
{
auto complete_error_message = fmt::format("LockFile acquisition failed, aborting: {}",
std::move(error_message));
LOG_ERROR << error_message;
safe_invoke(before_throw_task)
.map_error([](const auto& error)
{ LOG_ERROR << "While handling LockFile failure: " << error.what(); });
throw mamba_error(complete_error_message, mamba_error_code::lockfile_failure);
}
};
LockFileOwner::LockFileOwner(const fs::u8path& path, const std::chrono::seconds timeout)
@ -822,8 +834,7 @@ namespace mamba
std::error_code ec;
if (!fs::exists(path, ec))
{
LOG_ERROR << "Could not lock non-existing path '" << path.string() << "'";
throw std::runtime_error("LockFile error. Aborting.");
throw_lock_error(fmt::format("Could not lock non-existing path '{}'", path.string()));
}
if (fs::is_directory(path))
@ -845,9 +856,8 @@ namespace mamba
#endif
if (m_fd <= 0)
{
LOG_ERROR << "Could not open lockfile '" << m_lockfile_path.string() << "'";
unlock();
throw std::runtime_error("LockFile error. Aborting.");
throw_lock_error(fmt::format("Could not open lockfile '{}'", m_lockfile_path.string()),
[this] { unlock(); });
}
else
{
@ -866,11 +876,12 @@ namespace mamba
}
else
{
LOG_ERROR << "LockFile can't be set at '" << m_path.string() << "'\n"
<< "This could be fixed by changing the locks' timeout or "
<< "cleaning your environment from previous runs";
unlock();
throw std::runtime_error("LockFile error. Aborting.");
throw_lock_error(
fmt::format("LockFile can't be set at '{}'\n"
"This could be fixed by changing the locks' timeout or "
"cleaning your environment from previous runs",
m_path.string()),
[this] { unlock(); });
}
}
}
@ -1062,9 +1073,15 @@ namespace mamba
LockedFilesRegistry& operator=(const LockedFilesRegistry&) = delete;
std::shared_ptr<LockFileOwner> acquire_lock(const fs::u8path& file_path,
const std::chrono::seconds timeout)
tl::expected<std::shared_ptr<LockFileOwner>, mamba_error> acquire_lock(
const fs::u8path& file_path, const std::chrono::seconds timeout)
{
if (!Context::instance().use_lockfiles)
{
// No locking allowed, so do nothing.
return std::shared_ptr<LockFileOwner>{};
}
const auto absolute_file_path = fs::absolute(file_path);
std::scoped_lock lock{ mutex };
@ -1079,12 +1096,17 @@ namespace mamba
}
// At this point, we didn't find a lockfile alive, so we create one.
auto lockedfile = std::make_shared<LockFileOwner>(absolute_file_path, timeout);
auto tracker = std::weak_ptr{ lockedfile };
locked_files.insert_or_assign(absolute_file_path, std::move(tracker));
fd_to_locked_path.insert_or_assign(lockedfile->fd(), absolute_file_path);
assert(is_lockfile_locked(*lockedfile));
return lockedfile;
return safe_invoke(
[&]
{
auto lockedfile
= std::make_shared<LockFileOwner>(absolute_file_path, timeout);
auto tracker = std::weak_ptr{ lockedfile };
locked_files.insert_or_assign(absolute_file_path, std::move(tracker));
fd_to_locked_path.insert_or_assign(lockedfile->fd(), absolute_file_path);
assert(is_lockfile_locked(*lockedfile));
return lockedfile;
});
}
// note: the resulting value will be obsolete before returning.
@ -1154,26 +1176,17 @@ namespace mamba
int LockFile::fd() const
{
return impl->fd();
return impl.value()->fd();
}
fs::u8path LockFile::path() const
{
return impl->path();
return impl.value()->path();
}
fs::u8path LockFile::lockfile_path() const
{
return impl->lockfile_path();
}
std::unique_ptr<LockFile> LockFile::create_lock(const fs::u8path& path)
{
if (!Context::instance().use_lockfiles)
return nullptr;
auto ptr = std::unique_ptr<LockFile>(new LockFile(path));
return ptr;
return impl.value()->lockfile_path();
}
#ifdef _WIN32
@ -1182,6 +1195,16 @@ namespace mamba
// Windows locks are isolated between file descriptor
// We can then test if locked by opening a new one
int fd = _wopen(path.wstring().c_str(), O_RDWR | O_CREAT, 0666);
if (fd == -1)
{
if (errno == EACCES)
return true;
// In other cases, something is wrong.
throw mamba_error{ fmt::format("failed to check if path is locked : '{}'",
path.string()),
mamba_error_code::lockfile_failure };
}
_lseek(fd, MAMBA_LOCK_POS, SEEK_SET);
char buffer[1];
bool is_locked = _read(fd, buffer, 1) == -1;
@ -1230,25 +1253,6 @@ namespace mamba
#endif
std::unique_ptr<LockFile> LockFile::try_lock(const fs::u8path& path) noexcept
{
// Don't even lock if the file/directory isn't writable by someone or doesnt exists.
if (!Context::instance().use_lockfiles || !path::is_writable(path))
return nullptr;
try
{
auto ptr = std::unique_ptr<LockFile>(new LockFile(path));
return ptr;
}
catch (...)
{
LOG_WARNING << "LockFile creation for path '" << path.string()
<< "' failed, continuing without it";
}
return nullptr;
}
std::string timestamp(const std::time_t& utc_time)
{
char buf[sizeof("2011-10-08T07:07:09Z")];

71
libmamba/tests/test_lockfile.cpp
View File

@ -52,38 +52,45 @@ namespace mamba
TEST_F(LockDirTest, disable_locking)
{
{
auto _ = on_scope_exit([] { mamba::Context::instance().use_lockfiles = true; });
mamba::Context::instance().use_lockfiles = false;
auto lock = LockFile::create_lock(tempdir_path);
EXPECT_TRUE(lock == nullptr);
auto lock = LockFile(tempdir_path);
EXPECT_FALSE(lock);
}
ASSERT_TRUE(mamba::Context::instance().use_lockfiles);
{
ASSERT_TRUE(mamba::Context::instance().use_lockfiles);
auto lock = LockFile(tempdir_path);
EXPECT_TRUE(lock);
}
}
TEST_F(LockDirTest, same_pid)
{
{
auto lock = LockFile::create_lock(tempdir_path);
EXPECT_TRUE(lock->is_locked());
EXPECT_EQ(lock->count_lock_owners(), 1);
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
auto lock = LockFile(tempdir_path);
EXPECT_TRUE(lock.is_locked());
EXPECT_EQ(lock.count_lock_owners(), 1);
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
{
auto other_lock = LockFile::create_lock(tempdir_path);
EXPECT_TRUE(other_lock->is_locked());
EXPECT_EQ(other_lock->count_lock_owners(), 2);
EXPECT_EQ(lock->count_lock_owners(), 2);
auto other_lock = LockFile(tempdir_path);
EXPECT_TRUE(other_lock.is_locked());
EXPECT_EQ(other_lock.count_lock_owners(), 2);
EXPECT_EQ(lock.count_lock_owners(), 2);
}
EXPECT_EQ(lock->count_lock_owners(), 1);
EXPECT_EQ(lock.count_lock_owners(), 1);
// check the first lock is still locked
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
}
EXPECT_FALSE(fs::exists(tempdir_path / (tempdir_path.filename().string() + ".lock")));
// we can still re-lock afterwards
{
auto lock = LockFile::create_lock(tempdir_path);
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
auto lock = LockFile(tempdir_path);
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
}
}
@ -100,14 +107,14 @@ namespace mamba
#endif
{
auto lock = LockFile::create_lock(tempdir_path);
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
auto lock = LockFile(tempdir_path);
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
// Check lock status
EXPECT_TRUE(mamba::LockFile::is_locked(*lock));
EXPECT_TRUE(mamba::LockFile::is_locked(lock));
// Check lock status from another process
args = { lock_cli, "is-locked", lock->lockfile_path().string() };
args = { lock_cli, "is-locked", lock.lockfile_path().string() };
out.clear();
err.clear();
reproc::run(
@ -186,22 +193,22 @@ namespace mamba
TEST_F(LockFileTest, same_pid)
{
{
auto lock = LockFile::create_lock(tempfile_path);
EXPECT_TRUE(lock->is_locked());
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
EXPECT_EQ(lock->count_lock_owners(), 1);
auto lock = LockFile(tempfile_path);
EXPECT_TRUE(lock.is_locked());
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
EXPECT_EQ(lock.count_lock_owners(), 1);
{
auto other_lock = LockFile::create_lock(tempfile_path);
EXPECT_TRUE(other_lock->is_locked());
EXPECT_EQ(other_lock->count_lock_owners(), 2);
EXPECT_EQ(lock->count_lock_owners(), 2);
auto other_lock = LockFile(tempfile_path);
EXPECT_TRUE(other_lock.is_locked());
EXPECT_EQ(other_lock.count_lock_owners(), 2);
EXPECT_EQ(lock.count_lock_owners(), 2);
}
EXPECT_EQ(lock->count_lock_owners(), 1);
EXPECT_EQ(lock.count_lock_owners(), 1);
// check the first lock is still locked
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
}
EXPECT_FALSE(fs::exists(tempfile_path.string() + ".lock"));
}
@ -219,14 +226,14 @@ namespace mamba
#endif
{
// Create a lock
auto lock = LockFile::create_lock(tempfile_path);
EXPECT_TRUE(fs::exists(lock->lockfile_path()));
auto lock = LockFile(tempfile_path);
EXPECT_TRUE(fs::exists(lock.lockfile_path()));
// Check lock status from current PID
EXPECT_TRUE(mamba::LockFile::is_locked(*lock));
EXPECT_TRUE(mamba::LockFile::is_locked(lock));
// Check lock status from another process
args = { lock_cli, "is-locked", lock->lockfile_path().string() };
args = { lock_cli, "is-locked", lock.lockfile_path().string() };
out.clear();
err.clear();
reproc::run(

7
libmamba/tests/testing/lock.cpp
View File

@ -42,8 +42,11 @@ main(int argc, char** argv)
mamba::Context::instance().lock_timeout = timeout;
try
{
auto lock = mamba::LockFile::create_lock(path);
std::cout << 1;
auto lock = mamba::LockFile(path);
if (lock)
std::cout << 1;
else
std::cout << 0;
}
catch (...)
{

2
libmambapy/src/main.cpp
View File

@ -69,7 +69,7 @@ PYBIND11_MODULE(bindings, m)
{ return fmt::format("fs::u8path[{}]", self.string()); });
py::implicitly_convertible<std::string, fs::u8path>();
py::class_<mamba::LockFile>(m, "LockFile").def(py::init(&mamba::LockFile::create_lock));
py::class_<mamba::LockFile>(m, "LockFile").def(py::init<fs::u8path>());
py::register_exception<mamba_error>(m, "MambaNativeException");

35
micromamba/src/run.cpp
View File

@ -17,6 +17,7 @@
#include "mamba/core/util_os.hpp"
#include "mamba/core/util_random.hpp"
#include "mamba/core/execution.hpp"
#include "mamba/core/error_handling.hpp"
#include <nlohmann/json.hpp>
@ -102,19 +103,31 @@ namespace mamba
return path;
}
std::unique_ptr<LockFile> lock_proc_dir()
LockFile lock_proc_dir()
{
try
const auto proc_dir_path = proc_dir();
auto lockfile = LockFile(proc_dir_path);
if (!lockfile)
{
auto lockfile = LockFile::create_lock(proc_dir());
return lockfile;
}
catch (...)
{
throw std::runtime_error(
fmt::format("'mamba run' failed to lock ({}) or lockfile was not properly deleted",
proc_dir().string()));
if (auto error = lockfile.error())
{
throw mamba_error{
fmt::format(
"'mamba run' failed to lock ({}) or lockfile was not properly deleted - error: {}",
proc_dir_path.string(),
error->what()),
mamba_error_code::lockfile_failure
};
}
else
{
LOG_DEBUG
<< "`mamba run` file locking attempt ignored because locking is disabled - path: "
<< proc_dir_path.string();
}
}
return lockfile;
}
nlohmann::json get_all_running_processes_info(
@ -161,7 +174,7 @@ namespace mamba
public:
ScopedProcFile(const std::string& name,
const std::vector<std::string>& command,
std::unique_ptr<LockFile> proc_dir_lock = lock_proc_dir())
LockFile proc_dir_lock = lock_proc_dir())
: location{ proc_dir() / fmt::format("{}.json", getpid()) }
{
// Lock must be hold for the duraction of this constructor.

2
micromamba/src/run.hpp
View File

@ -10,7 +10,7 @@ namespace mamba
bool is_process_name_running(const std::string& name);
std::string generate_unique_process_name(std::string_view program_name);
const fs::u8path& proc_dir();
std::unique_ptr<LockFile> lock_proc_dir();
LockFile lock_proc_dir();
nlohmann::json get_all_running_processes_info(
const std::function<bool(const nlohmann::json&)>& filter