From bc7edfbdca5a0aff631e63b4af7d92cbf9c0fe8f Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 31 Aug 2023 10:56:30 +0100 Subject: [PATCH] use same logic for all platforms for locking and unlocking the file --- CHANGELOG.md | 2 +- src/realm/util/file.cpp | 17 +++++++++-------- src/realm/util/file.hpp | 8 +++++++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dbdd3a2907..0cf7706c7e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) * Add a distinct error code for timeouts (SyncConnectTimeout) rather than using the same one as for less transient failures ([PR #6932](https://github.com/realm/realm-core/pull/6932)). -* Throw an exception if `File::unlock` has failed, in order to inform the SDK that we are likely hitting some limitation on the OS filesystem, instead of crashing the application.([PR #6926](https://github.com/realm/realm-core/pull/6926)) +* Throw an exception if `File::unlock` has failed, in order to inform the SDK that we are likely hitting some limitation on the OS filesystem, instead of crashing the application and use the same file locking logic for all the platforms.([PR #6926](https://github.com/realm/realm-core/pull/6926)) ### Fixed diff --git a/src/realm/util/file.cpp b/src/realm/util/file.cpp index d70cd98d43c..86ff68bf269 100644 --- a/src/realm/util/file.cpp +++ b/src/realm/util/file.cpp @@ -507,6 +507,7 @@ void File::open_internal(const std::string& path, AccessMode a, CreateMode c, in int fd = ::open(path.c_str(), flags2, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); if (0 <= fd) { m_fd = fd; + m_have_lock = false; if (success) *success = true; return; @@ -562,7 +563,8 @@ void File::close() noexcept if (m_fd < 0) return; - unlock(); + if (m_have_lock) + unlock(); int r = ::close(m_fd); REALM_ASSERT_RELEASE(r == 0); m_fd = -1; @@ -1212,9 +1214,9 @@ bool File::rw_lock(bool exclusive, bool non_blocking) bool File::lock(bool exclusive, bool non_blocking) { REALM_ASSERT_RELEASE(is_attached()); + REALM_ASSERT_RELEASE(!m_have_lock); #ifdef _WIN32 // Windows version - REALM_ASSERT_RELEASE(!m_have_lock); // Under Windows a file lock must be explicitely released before // the file is closed. It will eventually be released by the @@ -1259,8 +1261,10 @@ bool File::lock(bool exclusive, bool non_blocking) if (non_blocking) operation |= LOCK_NB; do { - if (flock(m_fd, operation) == 0) + if (flock(m_fd, operation) == 0) { + m_have_lock = true; return true; + } } while (errno == EINTR); int err = errno; // Eliminate any risk of clobbering if (err == EWOULDBLOCK) @@ -1271,10 +1275,10 @@ bool File::lock(bool exclusive, bool non_blocking) void File::unlock() noexcept { -#ifdef _WIN32 // Windows version if (!m_have_lock) return; +#ifdef _WIN32 // Windows version OVERLAPPED overlapped; overlapped.hEvent = 0; overlapped.OffsetHigh = 0; @@ -1285,11 +1289,8 @@ void File::unlock() noexcept REALM_ASSERT_RELEASE(r); m_have_lock = false; #else - // The Linux man page for flock() does not state explicitely that - // unlocking is idempotent, however, we will assume it since there - // is no mention of the error that would be reported if a - // non-locked file were unlocked. _unlock(m_fd); + m_have_lock = false; #endif } diff --git a/src/realm/util/file.hpp b/src/realm/util/file.hpp index 6c38f6f175c..b7314feb333 100644 --- a/src/realm/util/file.hpp +++ b/src/realm/util/file.hpp @@ -627,9 +627,9 @@ class File { class Streambuf; private: + bool m_have_lock = false; // Only valid when m_fd is not null #ifdef _WIN32 HANDLE m_fd = nullptr; - bool m_have_lock = false; // Only valid when m_fd is not null #else int m_fd = -1; #ifdef REALM_FILELOCK_EMULATION @@ -983,9 +983,12 @@ inline File::File(File&& f) noexcept #ifdef _WIN32 m_fd = f.m_fd; m_have_lock = f.m_have_lock; + f.m_have_lock = false; f.m_fd = nullptr; #else m_fd = f.m_fd; + m_have_lock = f.m_have_lock; + f.m_have_lock = false; #ifdef REALM_FILELOCK_EMULATION m_pipe_fd = f.m_pipe_fd; m_has_exclusive_lock = f.m_has_exclusive_lock; @@ -1003,10 +1006,13 @@ inline File& File::operator=(File&& f) noexcept #ifdef _WIN32 m_fd = f.m_fd; m_have_lock = f.m_have_lock; + f.m_have_lock = false; f.m_fd = nullptr; #else m_fd = f.m_fd; f.m_fd = -1; + m_have_lock = f.m_have_lock; + f.m_have_lock = false; #ifdef REALM_FILELOCK_EMULATION m_pipe_fd = f.m_pipe_fd; f.m_pipe_fd = -1;