RCORE-2242: Include pathname in exception thrown in File::rw_lock (#8000

)

* Include pathname in exception thrown in File::rw_lock
* Strengthen check on synced file name size

CHANGELOG.md

@@ -6,9 +6,11 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](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.

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;

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,15 +1183,15 @@ 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);
             err = errno;
         }
 
         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

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));
         }
     }

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<Realm>* out)
+void create_dummy_realm(std::string path, std::shared_ptr<Realm>* 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);
     }
 }
 

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<Realm>* out = nullptr);
+void create_dummy_realm(std::string path, std::shared_ptr<Realm>* out = nullptr);
 std::vector<char> 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<void()> func);