Skip to content

Commit

Permalink
use same logic for all platforms for locking and unlocking the file
Browse files Browse the repository at this point in the history
  • Loading branch information
nicola-cab committed Aug 31, 2023
1 parent 7a71337 commit bc7edfb
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
### Enhancements
* <New feature description> (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
Expand Down
17 changes: 9 additions & 8 deletions src/realm/util/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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
}

Expand Down
8 changes: 7 additions & 1 deletion src/realm/util/file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit bc7edfb

Please sign in to comment.