commit e6dbedd: [Rework] Split locked and unlocked files, as mmap does not need flock normally
Vsevolod Stakhov
vsevolod at rspamd.com
Sat Oct 15 12:35:03 UTC 2022
Author: Vsevolod Stakhov
Date: 2022-10-15 13:33:55 +0100
URL: https://github.com/rspamd/rspamd/commit/e6dbedd6904d30e4a8d65d511f449665cae1f70b (HEAD -> master)
[Rework] Split locked and unlocked files, as mmap does not need flock normally
---
src/libutil/cxx/locked_file.cxx | 76 ++++++++++++++++++-----------
src/libutil/cxx/locked_file.hxx | 105 ++++++++++++++++++++++++++++++----------
2 files changed, 127 insertions(+), 54 deletions(-)
diff --git a/src/libutil/cxx/locked_file.cxx b/src/libutil/cxx/locked_file.cxx
index bd25b0fc7..b4d865626 100644
--- a/src/libutil/cxx/locked_file.cxx
+++ b/src/libutil/cxx/locked_file.cxx
@@ -24,7 +24,7 @@
namespace rspamd::util {
-auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::open(const char *fname, int flags) -> tl::expected<raii_file, std::string>
{
int oflags = flags;
#ifdef O_CLOEXEC
@@ -46,7 +46,7 @@ auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_l
return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
}
- auto ret = raii_locked_file{fname, fd, false};
+ auto ret = raii_file{fname, fd, false};
if (fstat(ret.fd, &ret.st) == -1) {
return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -55,18 +55,7 @@ auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_l
return ret;
}
-raii_locked_file::~raii_locked_file()
-{
- if (fd != -1) {
- if (temp) {
- (void)unlink(fname.c_str());
- }
- (void) rspamd_file_unlock(fd, FALSE);
- close(fd);
- }
-}
-
-auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::create(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>
{
int oflags = flags;
#ifdef O_CLOEXEC
@@ -88,7 +77,7 @@ auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::ex
return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
}
- auto ret = raii_locked_file{fname, fd, false};
+ auto ret = raii_file{fname, fd, false};
if (fstat(ret.fd, &ret.st) == -1) {
return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -97,7 +86,7 @@ auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::ex
return ret;
}
-auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>
{
int oflags = flags;
#ifdef O_CLOEXEC
@@ -114,11 +103,12 @@ auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> t
}
if (!rspamd_file_lock(fd, TRUE)) {
+ unlink(fname);
close(fd);
return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
}
- auto ret = raii_locked_file{fname, fd, true};
+ auto ret = raii_file{fname, fd, true};
if (fstat(ret.fd, &ret.st) == -1) {
return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -127,7 +117,7 @@ auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> t
return ret;
}
-auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_file, std::string>
{
int oflags = flags;
#ifdef O_CLOEXEC
@@ -151,7 +141,7 @@ auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl:
return tl::make_unexpected(fmt::format("cannot lock file {}: {}", pattern, ::strerror(errno)));
}
- auto ret = raii_locked_file{mutable_pattern.c_str(), fd, true};
+ auto ret = raii_file{mutable_pattern.c_str(), fd, true};
if (fstat(ret.fd, &ret.st) == -1) {
return tl::make_unexpected(fmt::format("cannot stat file {}: {}",
@@ -161,13 +151,41 @@ auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl:
return ret;
}
-raii_mmaped_locked_file::raii_mmaped_locked_file(raii_locked_file &&_file, void *_map)
+raii_file::~raii_file() noexcept
+{
+ if (fd != -1) {
+ if (temp) {
+ (void)unlink(fname.c_str());
+ }
+ (void) rspamd_file_unlock(fd, FALSE);
+ close(fd);
+ }
+}
+
+
+raii_locked_file::~raii_locked_file() noexcept
+{
+ if (fd != -1) {
+ (void) rspamd_file_unlock(fd, FALSE);
+ }
+}
+
+auto raii_locked_file::lock_raii_file(raii_file &&unlocked) -> tl::expected<raii_locked_file, std::string>
+{
+ if (!rspamd_file_lock(unlocked.get_fd(), TRUE)) {
+ return tl::make_unexpected(fmt::format("cannot lock file {}: {}", unlocked.get_name(), ::strerror(errno)));
+ }
+
+ return raii_locked_file{std::move(unlocked)};
+}
+
+raii_mmaped_file::raii_mmaped_file(raii_file &&_file, void *_map)
: file(std::move(_file)), map(_map)
{
}
-auto raii_mmaped_locked_file::mmap_shared(raii_locked_file &&file,
- int flags) -> tl::expected<raii_mmaped_locked_file, std::string>
+auto raii_mmaped_file::mmap_shared(raii_file &&file,
+ int flags) -> tl::expected<raii_mmaped_file, std::string>
{
void *map;
@@ -179,29 +197,29 @@ auto raii_mmaped_locked_file::mmap_shared(raii_locked_file &&file,
}
- return raii_mmaped_locked_file{std::move(file), map};
+ return raii_mmaped_file{std::move(file), map};
}
-auto raii_mmaped_locked_file::mmap_shared(const char *fname, int open_flags,
- int mmap_flags) -> tl::expected<raii_mmaped_locked_file, std::string>
+auto raii_mmaped_file::mmap_shared(const char *fname, int open_flags,
+ int mmap_flags) -> tl::expected<raii_mmaped_file, std::string>
{
- auto file = raii_locked_file::open(fname, open_flags);
+ auto file = raii_file::open(fname, open_flags);
if (!file.has_value()) {
return tl::make_unexpected(file.error());
}
- return raii_mmaped_locked_file::mmap_shared(std::move(file.value()), mmap_flags);
+ return raii_mmaped_file::mmap_shared(std::move(file.value()), mmap_flags);
}
-raii_mmaped_locked_file::~raii_mmaped_locked_file()
+raii_mmaped_file::~raii_mmaped_file()
{
if (map != nullptr) {
munmap(map, file.get_stat().st_size);
}
}
-raii_mmaped_locked_file::raii_mmaped_locked_file(raii_mmaped_locked_file &&other) noexcept
+raii_mmaped_file::raii_mmaped_file(raii_mmaped_file &&other) noexcept
: file(std::move(other.file))
{
std::swap(map, other.map);
diff --git a/src/libutil/cxx/locked_file.hxx b/src/libutil/cxx/locked_file.hxx
index 94561a2d4..8690cfb64 100644
--- a/src/libutil/cxx/locked_file.hxx
+++ b/src/libutil/cxx/locked_file.hxx
@@ -24,16 +24,16 @@
namespace rspamd::util {
/**
- * A simple RAII object to contain a file descriptor with an flock wrap
+ * A simple RAII object to contain a move only file descriptor
* A file is unlocked and closed when not needed
*/
-struct raii_locked_file final {
- ~raii_locked_file();
+struct raii_file {
+ virtual ~raii_file() noexcept;
- static auto open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string>;
- static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
- static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
- static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
+ static auto open(const char *fname, int flags) -> tl::expected<raii_file, std::string>;
+ static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>;
+ static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>;
+ static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_file, std::string>;
auto get_fd() const -> int {
return fd;
@@ -79,7 +79,7 @@ struct raii_locked_file final {
}
}
- raii_locked_file& operator=(raii_locked_file &&other) noexcept {
+ raii_file& operator=(raii_file &&other) noexcept {
std::swap(fd, other.fd);
std::swap(temp, other.temp);
std::swap(fname, other.fname);
@@ -88,30 +88,85 @@ struct raii_locked_file final {
return *this;
}
- raii_locked_file(raii_locked_file &&other) noexcept {
+ raii_file(raii_file &&other) noexcept {
*this = std::move(other);
}
/* Do not allow copy/default ctor */
- const raii_locked_file& operator=(const raii_locked_file &other) = delete;
- raii_locked_file() = delete;
- raii_locked_file(const raii_locked_file &other) = delete;
-private:
+ const raii_file& operator=(const raii_file &other) = delete;
+ raii_file() = delete;
+ raii_file(const raii_file &other) = delete;
+protected:
int fd = -1;
bool temp;
std::string fname;
struct stat st;
- explicit raii_locked_file(const char *_fname, int _fd, bool _temp) : fd(_fd), temp(_temp), fname(_fname) {}
+ explicit raii_file(const char *fname, int fd, bool temp) : fd(fd), temp(temp), fname(fname) {}
+};
+/**
+ * A simple RAII object to contain a file descriptor with an flock wrap
+ * A file is unlocked and closed when not needed
+ */
+struct raii_locked_file final : public raii_file {
+ ~raii_locked_file() noexcept override;
+
+ static auto open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string> {
+ auto locked = raii_file::open(fname, flags).and_then([]<class T>(T &&file) {
+ return lock_raii_file(std::forward<T>(file));
+ });
+
+ return locked;
+ }
+ static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+ auto locked = raii_file::create(fname, flags, perms).and_then([]<class T>(T &&file) {
+ return lock_raii_file(std::forward<T>(file));
+ });
+
+ return locked;
+ }
+ static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+ auto locked = raii_file::create_temp(fname, flags, perms).and_then([]<class T>(T &&file) {
+ return lock_raii_file(std::forward<T>(file));
+ });
+
+ return locked;
+ }
+ static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+ auto locked = raii_file::mkstemp(pattern, flags, perms).and_then([]<class T>(T &&file) {
+ return lock_raii_file(std::forward<T>(file));
+ });
+
+ return locked;
+ }
+
+ raii_locked_file& operator=(raii_locked_file &&other) noexcept {
+ std::swap(fd, other.fd);
+ std::swap(temp, other.temp);
+ std::swap(fname, other.fname);
+ std::swap(st, other.st);
+
+ return *this;
+ }
+
+ raii_locked_file(raii_locked_file &&other) noexcept : raii_file(static_cast<raii_file &&>(std::move(other))) {}
+ /* Do not allow copy/default ctor */
+ const raii_locked_file& operator=(const raii_locked_file &other) = delete;
+ raii_locked_file() = delete;
+ raii_locked_file(const raii_locked_file &other) = delete;
+private:
+ static auto lock_raii_file(raii_file &&unlocked) -> tl::expected<raii_locked_file, std::string>;
+ raii_locked_file(raii_file &&other) noexcept : raii_file(std::move(other)) {}
+ explicit raii_locked_file(const char *fname, int fd, bool temp) : raii_file(fname, fd, temp) {}
};
/**
* A mmap wrapper on top of a locked file
*/
-struct raii_mmaped_locked_file final {
- ~raii_mmaped_locked_file();
- static auto mmap_shared(raii_locked_file &&file, int flags) -> tl::expected<raii_mmaped_locked_file, std::string>;
- static auto mmap_shared(const char *fname, int open_flags, int mmap_flags) -> tl::expected<raii_mmaped_locked_file, std::string>;
+struct raii_mmaped_file final {
+ ~raii_mmaped_file();
+ static auto mmap_shared(raii_file &&file, int flags) -> tl::expected<raii_mmaped_file, std::string>;
+ static auto mmap_shared(const char *fname, int open_flags, int mmap_flags) -> tl::expected<raii_mmaped_file, std::string>;
// Returns a constant pointer to the underlying map
auto get_map() const -> void* {return map;}
// Passes the ownership of the mmaped memory to the callee
@@ -123,23 +178,23 @@ struct raii_mmaped_locked_file final {
auto get_size() const -> std::size_t { return file.get_stat().st_size; }
- raii_mmaped_locked_file& operator=(raii_mmaped_locked_file &&other) noexcept {
+ raii_mmaped_file& operator=(raii_mmaped_file &&other) noexcept {
std::swap(map, other.map);
file = std::move(other.file);
return *this;
}
- raii_mmaped_locked_file(raii_mmaped_locked_file &&other) noexcept;
+ raii_mmaped_file(raii_mmaped_file &&other) noexcept;
/* Do not allow copy/default ctor */
- const raii_mmaped_locked_file& operator=(const raii_mmaped_locked_file &other) = delete;
- raii_mmaped_locked_file() = delete;
- raii_mmaped_locked_file(const raii_mmaped_locked_file &other) = delete;
+ const raii_mmaped_file& operator=(const raii_mmaped_file &other) = delete;
+ raii_mmaped_file() = delete;
+ raii_mmaped_file(const raii_mmaped_file &other) = delete;
private:
/* Is intended to be used with map_shared */
- explicit raii_mmaped_locked_file(raii_locked_file &&_file, void *_map);
- raii_locked_file file;
+ explicit raii_mmaped_file(raii_file &&_file, void *_map);
+ raii_file file;
void *map = nullptr;
};
More information about the Commits
mailing list