diff --git a/CHANGELOG.md b/CHANGELOG.md index 81045741c7..947d4fb053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,11 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* Using an empty KeyPath in C API would result in no filtering being done ([#7805](https://github.com/realm/realm-core/issues/7805), since 13.24.0) -* Filtering notifications with backlink columns as last element could sometimes give wrong results ([#7530](https://github.com/realm/realm-core/issues/7530), since 11.1.0) +* Using an empty KeyPath in C API would result in no filtering being done ([#7805](https://github.com/realm/realm-core/issues/7805), since v13.24.0) +* Filtering notifications with backlink columns as last element could sometimes give wrong results ([#7530](https://github.com/realm/realm-core/issues/7530), since v11.1.0) * Fix crash during client app shutdown when Logger log level is set higher than Info. ([#7969](https://github.com/realm/realm-core/issues/7969), since v13.23.3) +* If File::rw_lock() fails to open a file the exception message does not contain the filename ([#7999](https://github.com/realm/realm-core/issues/7999), since v6.0.21) +* Fallback to hashed filename will fail if length of basename is between 240 and 250 ([#8007](https://github.com/realm/realm-core/issues/8007), since v10.0.0) ### Breaking changes * None. diff --git a/src/realm/object-store/sync/impl/sync_file.hpp b/src/realm/object-store/sync/impl/sync_file.hpp index c5191149e2..ecb9f10864 100644 --- a/src/realm/object-store/sync/impl/sync_file.hpp +++ b/src/realm/object-store/sync/impl/sync_file.hpp @@ -133,7 +133,7 @@ class SyncFileManager { static constexpr const char c_metadata_realm[] = "sync_metadata.realm"; static constexpr const char c_realm_file_suffix[] = ".realm"; static constexpr const char c_realm_file_test_suffix[] = - ".rtest"; // Must have same length as c_realm_file_suffix. + ".rtest.management"; // Must have same length as the biggest path name we might use. static constexpr const char c_legacy_sync_directory[] = "realm-object-server"; std::string get_special_directory(std::string directory_name) const; diff --git a/src/realm/util/file.cpp b/src/realm/util/file.cpp index 3e32d53008..3dcde11428 100644 --- a/src/realm/util/file.cpp +++ b/src/realm/util/file.cpp @@ -1152,6 +1152,11 @@ bool File::rw_lock(bool exclusive, bool non_blocking) // or the OS will deadlock when un-suspending the app. int mode = exclusive ? O_WRONLY | O_NONBLOCK : O_RDWR | O_NONBLOCK; + auto report_err = [this](int err, const char* msg) { + auto message = util::format("%1: %2, path = '%3'", msg, std::system_category().message(err), m_fifo_path); + throw RuntimeError(ErrorCodes::FileOperationFailed, message); // LCOV_EXCL_LINE + }; + // Optimistically try to open the fifo. This may fail due to the fifo not // existing, but since it usually exists this is faster than trying to create // it first. @@ -1168,7 +1173,7 @@ bool File::rw_lock(bool exclusive, bool non_blocking) } // Otherwise we got an unexpected error - throw std::system_error(err, std::system_category(), "opening lock fifo for writing failed"); + report_err(err, "opening lock fifo for writing failed"); } if (err == ENOENT) { @@ -1178,7 +1183,7 @@ bool File::rw_lock(bool exclusive, bool non_blocking) try_make_dir(m_fifo_dir_path); status = mkfifo(m_fifo_path.c_str(), 0666); if (status != 0) - throw std::system_error(errno, std::system_category(), "creating lock fifo for reading failed"); + report_err(errno, "creating lock fifo for reading failed"); // Try again to open the fifo now that it exists fd = ::open(m_fifo_path.c_str(), mode); @@ -1186,7 +1191,7 @@ bool File::rw_lock(bool exclusive, bool non_blocking) } if (fd == -1) - throw std::system_error(err, std::system_category(), "opening lock fifo for reading failed"); + report_err(err, "opening lock fifo for reading failed"); } // We successfully opened the pipe. If we're trying to acquire an exclusive diff --git a/test/object-store/sync/file.cpp b/test/object-store/sync/file.cpp index 1ef56f4819..623573d12e 100644 --- a/test/object-store/sync/file.cpp +++ b/test/object-store/sync/file.cpp @@ -162,7 +162,7 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { SECTION("deleting a Realm for a valid user") { manager.realm_file_path(identity, legacy_identities, relative_path, partition); // Create the required files - REQUIRE(create_dummy_realm(expected_paths.current_preferred_path)); + REQUIRE_NOTHROW(create_dummy_realm(expected_paths.current_preferred_path)); REQUIRE(File::exists(expected_paths.current_preferred_path)); REQUIRE(File::exists(expected_paths.current_preferred_path + ".lock")); REQUIRE_DIR_EXISTS(expected_paths.current_preferred_path + ".management"); @@ -183,7 +183,7 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { REQUIRE(!File::exists(expected_paths.fallback_hashed_path)); REQUIRE(!File::exists(expected_paths.current_preferred_path)); - REQUIRE(create_dummy_realm(expected_paths.fallback_hashed_path)); + REQUIRE_NOTHROW(create_dummy_realm(expected_paths.fallback_hashed_path)); REQUIRE(File::exists(expected_paths.fallback_hashed_path)); REQUIRE(!File::exists(expected_paths.current_preferred_path)); auto actual = manager.realm_file_path(identity, legacy_identities, relative_path, partition); @@ -197,7 +197,7 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { util::try_make_dir((manager_path / local_identity).string()); REQUIRE(!File::exists(expected_paths.legacy_local_id_path)); REQUIRE(!File::exists(expected_paths.current_preferred_path)); - REQUIRE(create_dummy_realm(expected_paths.legacy_local_id_path)); + REQUIRE_NOTHROW(create_dummy_realm(expected_paths.legacy_local_id_path)); REQUIRE(File::exists(expected_paths.legacy_local_id_path)); REQUIRE(!File::exists(expected_paths.current_preferred_path)); @@ -217,7 +217,7 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { util::try_make_dir(manager_path.string()); util::try_make_dir((manager_path / local_identity_2).string()); - REQUIRE(create_dummy_realm(expected_paths_2.legacy_local_id_path)); + REQUIRE_NOTHROW(create_dummy_realm(expected_paths_2.legacy_local_id_path)); // Note: intentionally not legacy_identities_2. We're passing both // in and validating that it'll open the second one. @@ -233,7 +233,7 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { for (auto& dir : expected_paths.legacy_sync_directories_to_make) { util::try_make_dir(dir); } - REQUIRE(create_dummy_realm(expected_paths.legacy_sync_path)); + REQUIRE_NOTHROW(create_dummy_realm(expected_paths.legacy_sync_path)); REQUIRE(File::exists(expected_paths.legacy_sync_path)); REQUIRE(!File::exists(expected_paths.current_preferred_path)); auto actual = manager.realm_file_path(identity, legacy_identities, relative_path, partition); @@ -247,7 +247,13 @@ TEST_CASE("sync_file: SyncFileManager APIs", "[sync][file]") { REQUIRE(long_path_name.length() > 255); // linux name length limit auto actual = manager.realm_file_path(identity, legacy_identities, long_path_name, partition); REQUIRE(actual.length() < 500); - REQUIRE(create_dummy_realm(actual)); + REQUIRE_NOTHROW(create_dummy_realm(actual)); + } + + SECTION("paths have a fallbach hashed location if the preferred length is 240 > length < 255") { + const std::string long_path_name = std::string(241, 'a'); + auto actual = manager.realm_file_path(identity, legacy_identities, long_path_name, partition); + REQUIRE_NOTHROW(create_dummy_realm(actual)); REQUIRE(File::exists(actual)); } } diff --git a/test/object-store/util/test_utils.cpp b/test/object-store/util/test_utils.cpp index 550748c569..a70eefb05e 100644 --- a/test/object-store/util/test_utils.cpp +++ b/test/object-store/util/test_utils.cpp @@ -78,20 +78,14 @@ std::ostream& operator<<(std::ostream& os, const Exception& e) return os; } -bool create_dummy_realm(std::string path, std::shared_ptr* out) +void create_dummy_realm(std::string path, std::shared_ptr* out) { Realm::Config config; config.path = path; - try { - auto realm = _impl::RealmCoordinator::get_coordinator(path)->get_realm(config, none); - REQUIRE_REALM_EXISTS(path); - if (out) { - *out = std::move(realm); - } - return true; - } - catch (std::exception&) { - return false; + auto realm = _impl::RealmCoordinator::get_coordinator(path)->get_realm(config, none); + REQUIRE_REALM_EXISTS(path); + if (out) { + *out = std::move(realm); } } diff --git a/test/object-store/util/test_utils.hpp b/test/object-store/util/test_utils.hpp index c413daf619..63807438d6 100644 --- a/test/object-store/util/test_utils.hpp +++ b/test/object-store/util/test_utils.hpp @@ -194,7 +194,7 @@ std::ostream& operator<<(std::ostream&, const Exception&); class Realm; /// Open a Realm at a given path, creating its files. -bool create_dummy_realm(std::string path, std::shared_ptr* out = nullptr); +void create_dummy_realm(std::string path, std::shared_ptr* out = nullptr); std::vector make_test_encryption_key(const char start = 0); void catch2_ensure_section_run_workaround(bool did_run_a_section, std::string section_name, util::FunctionRef func);