Merge branch 'master' of github.com:realm/realm-core into mwb/nightly…

…-evergreen-build

CHANGELOG.md

@@ -5,7 +5,30 @@
 * None.
 
 ### Fixed
-* Trying to search a full-text indexes created as a result of an additive schema change (i.e. applying the differences between the local schema and a synchronized realm's schema) could have resulted in an IllegalOperation error with the error code `Column has no fulltext index`. ([PR #6823](https://github.com/realm/realm-core/pull/6823)).
+* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
+* None.
+
+### Breaking changes
+* None.
+
+### Compatibility
+* Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5.
+
+-----------
+
+### Internals
+* None.
+
+----------------------------------------------
+
+# 13.17.1 Release notes
+
+### Enhancements
+* None.
+
+### Fixed
+* Rare corruption of files on streaming format (often following compact, convert or copying to a new file). ([#6807](https://github.com/realm/realm-core/pull/6807), since v12.12.0)
+* Trying to search a full-text indexes created as a result of an additive schema change (i.e. applying the differences between the local schema and a synchronized realm's schema) could have resulted in an IllegalOperation error with the error code `Column has no fulltext index`. ([PR #6823](https://github.com/realm/realm-core/pull/6823), since v13.2.0).
 * Sync progress for DOWNLOAD messages from server state was updated wrongly. This may have resulted in an extra round-trip to the server. ([#6827](https://github.com/realm/realm-core/issues/6827), since v12.9.0)
 
 ### Breaking changes
@@ -19,6 +42,7 @@
 ### Internals
 * `wait_for_upload_completion`/`wait_for_download_completion` internal API was changed to use `Status`'s instead of `std::error_code`. The SDK-facing was already `Status` oriented, so this change should only result in better error messages. ([PR #6796](https://github.com/realm/realm-core/pull/6796))
 * Separate local and baas object store tests into separate evergreen tasks and allow custom test specification. ([PR #6805](https://github.com/realm/realm-core/pull/6805))
+* Consolidate object store sync util files into test/object-store/util/sync/ directory. ([PR #6789](https://github.com/realm/realm-core/pull/6789))
 
 ----------------------------------------------
 

Package.swift

@@ -3,7 +3,7 @@
 import PackageDescription
 import Foundation
 
-let versionStr = "13.17.0"
+let versionStr = "13.17.1"
 let versionPieces = versionStr.split(separator: "-")
 let versionCompontents = versionPieces[0].split(separator: ".")
 let versionExtra = versionPieces.count > 1 ? versionPieces[1] : ""

dependencies.list

@@ -1,5 +1,5 @@
 PACKAGE_NAME=realm-core
-VERSION=13.17.0
+VERSION=13.17.1
 OPENSSL_VERSION=3.0.8
 ZLIB_VERSION=1.2.13
 MDBREALM_TEST_SERVER_TAG=2023-07-07

src/realm/alloc_slab.cpp

@@ -130,7 +130,7 @@ SlabAlloc::Slab::~Slab()
         delete[] addr;
 }
 
-void SlabAlloc::detach() noexcept
+void SlabAlloc::detach(bool keep_file_open) noexcept
 {
     delete[] m_ref_translation_ptr;
     m_ref_translation_ptr.store(nullptr);
@@ -150,7 +150,8 @@ void SlabAlloc::detach() noexcept
             m_data = 0;
             m_mappings.clear();
             m_youngest_live_version = 0;
-            m_file.close();
+            if (!keep_file_open)
+                m_file.close();
             break;
         case attach_Heap:
             m_data = 0;
@@ -697,6 +698,79 @@ std::string SlabAlloc::get_file_path_for_assertions() const
     return m_file.get_path();
 }
 
+bool SlabAlloc::align_filesize_for_mmap(ref_type top_ref, Config& cfg)
+{
+    if (cfg.read_only) {
+        // If the file is opened read-only, we cannot change it. This is not a problem,
+        // because for a read-only file we assume that it will not change while we use it,
+        // hence there will be no need to grow memory mappings.
+        // This assumption obviously will not hold, if the file is shared by multiple
+        // processes or threads with different opening modes.
+        // Currently, there is no way to detect if this assumption is violated.
+        return false;
+    }
+    size_t expected_size = size_t(-1);
+    size_t size = static_cast<size_t>(m_file.get_size());
+
+    // It is not safe to change the size of a file on streaming form, since the footer
+    // must remain available and remain at the very end of the file.
+    REALM_ASSERT(!is_file_on_streaming_form());
+
+    // check if online compaction allows us to shrink the file:
+    if (top_ref) {
+        // Get the expected file size by looking up logical file size stored in top array
+        constexpr size_t max_top_size = (Group::s_file_size_ndx + 1) * 8 + sizeof(Header);
+        size_t top_page_base = top_ref & ~(page_size() - 1);
+        size_t top_offset = top_ref - top_page_base;
+        size_t map_size = std::min(max_top_size + top_offset, size - top_page_base);
+        File::Map<char> map_top(m_file, top_page_base, File::access_ReadOnly, map_size, 0, m_write_observer);
+        realm::util::encryption_read_barrier(map_top, top_offset, max_top_size);
+        auto top_header = map_top.get_addr() + top_offset;
+        auto top_data = NodeHeader::get_data_from_header(top_header);
+        auto w = NodeHeader::get_width_from_header(top_header);
+        auto logical_size = size_t(get_direct(top_data, w, Group::s_file_size_ndx)) >> 1;
+        // make sure we're page aligned, so the code below doesn't first
+        // truncate the file, then expand it again
+        expected_size = round_up_to_page_size(logical_size);
+    }
+
+    // Check if we can shrink the file
+    if (cfg.session_initiator && expected_size < size && !cfg.read_only) {
+        detach(true); // keep m_file open
+        m_file.resize(expected_size);
+        m_file.close();
+        size = expected_size;
+        return true;
+    }
+
+    // We can only safely mmap the file, if its size matches a page boundary. If not,
+    // we must change the size to match before mmaping it.
+    if (size != round_up_to_page_size(size)) {
+        // The file size did not match a page boundary.
+        // We must extend the file to a page boundary (unless already there)
+        // The file must be extended to match in size prior to being mmapped,
+        // as extending it after mmap has undefined behavior.
+        if (cfg.session_initiator || !cfg.is_shared) {
+            // We can only safely extend the file if we're the session initiator, or if
+            // the file isn't shared at all. Extending the file to a page boundary is ONLY
+            // done to ensure well defined behavior for memory mappings. It does not matter,
+            // that the free space management isn't informed
+            size = round_up_to_page_size(size);
+            detach(true); // keep m_file open
+            m_file.prealloc(size);
+            m_file.close();
+            return true;
+        }
+        else {
+            // Getting here, we have a file of a size that will not work, and without being
+            // allowed to extend it. This should not be possible. But allowing a retry is
+            // arguably better than giving up and crashing...
+            throw Retry();
+        }
+    }
+    return false;
+}
+
 ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::WriteObserver* write_observer)
 {
     m_cfg = cfg;
@@ -773,7 +847,6 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
         note_reader_end(this);
     });
 
-    size_t expected_size = size_t(-1);
     try {
         // we'll read header and (potentially) footer
         File::Map<char> map_header(m_file, File::access_ReadOnly, sizeof(Header), 0, m_write_observer);
@@ -814,21 +887,6 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
             REALM_ASSERT_EX(footer->m_magic_cookie == footer_magic_cookie, footer->m_magic_cookie,
                             get_file_path_for_assertions());
         }
-
-        if (top_ref) {
-            // Get the expected file size by looking up logical file size stored in top array
-            constexpr size_t max_top_size = (Group::s_file_size_ndx + 1) * 8 + sizeof(Header);
-            size_t top_page_base = top_ref & ~(page_size() - 1);
-            size_t top_offset = top_ref - top_page_base;
-            size_t map_size = std::min(max_top_size + top_offset, size - top_page_base);
-            File::Map<char> map_top(m_file, top_page_base, File::access_ReadOnly, map_size, 0, m_write_observer);
-            realm::util::encryption_read_barrier(map_top, top_offset, max_top_size);
-            auto top_header = map_top.get_addr() + top_offset;
-            auto top_data = NodeHeader::get_data_from_header(top_header);
-            auto w = NodeHeader::get_width_from_header(top_header);
-            auto logical_size = size_t(get_direct(top_data, w, Group::s_file_size_ndx)) >> 1;
-            expected_size = round_up_to_page_size(logical_size);
-        }
     }
     catch (const InvalidDatabase&) {
         throw;
@@ -842,6 +900,7 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
     catch (...) {
         throw InvalidDatabase("unknown error", path);
     }
+    // m_data not valid at this point!
     m_baseline = 0;
     // make sure that any call to begin_read cause any slab to be placed in free
     // lists correctly
@@ -850,45 +909,6 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
     // Ensure clean up, if we need to back out:
     DetachGuard dg(*this);
 
-    // Check if we can shrink the file
-    if (cfg.session_initiator && expected_size < size) {
-        m_file.resize(expected_size);
-        size = expected_size;
-    }
-    // We can only safely mmap the file, if its size matches a page boundary. If not,
-    // we must change the size to match before mmaping it.
-    if (size != round_up_to_page_size(size)) {
-        // The file size did not match a page boundary.
-        // We must extend the file to a page boundary (unless already there)
-        // The file must be extended to match in size prior to being mmapped,
-        // as extending it after mmap has undefined behavior.
-        if (cfg.read_only) {
-            // If the file is opened read-only, we cannot extend it. This is not a problem,
-            // because for a read-only file we assume that it will not change while we use it.
-            // This assumption obviously will not hold, if the file is shared by multiple
-            // processes or threads with different opening modes.
-            // Currently, there is no way to detect if this assumption is violated.
-            m_baseline = 0;
-        }
-        else {
-            if (cfg.session_initiator || !cfg.is_shared) {
-                // We can only safely extend the file if we're the session initiator, or if
-                // the file isn't shared at all. Extending the file to a page boundary is ONLY
-                // done to ensure well defined behavior for memory mappings. It does not matter,
-                // that the free space management isn't informed
-                size = round_up_to_page_size(size);
-                m_file.prealloc(size);
-                m_baseline = 0;
-            }
-            else {
-                // Getting here, we have a file of a size that will not work, and without being
-                // allowed to extend it. This should not be possible. But allowing a retry is
-                // arguably better than giving up and crashing...
-                throw Retry();
-            }
-        }
-    }
-
     reset_free_space_tracking();
     update_reader_view(size);
     REALM_ASSERT(m_mappings.size());
@@ -915,12 +935,12 @@ void SlabAlloc::convert_from_streaming_form(ref_type top_ref)
     {
         File::Map<Header> writable_map(m_file, File::access_ReadWrite, sizeof(Header)); // Throws
         Header& writable_header = *writable_map.get_addr();
-        realm::util::encryption_read_barrier(writable_map, 0);
+        realm::util::encryption_read_barrier_for_write(writable_map, 0);
         writable_header.m_top_ref[1] = top_ref;
         writable_header.m_file_format[1] = writable_header.m_file_format[0];
         realm::util::encryption_write_barrier(writable_map, 0);
         writable_map.sync();
-        realm::util::encryption_read_barrier(writable_map, 0);
+        realm::util::encryption_read_barrier_for_write(writable_map, 0);
         writable_header.m_flags |= flags_SelectBit;
         realm::util::encryption_write_barrier(writable_map, 0);
         writable_map.sync();

src/realm/alloc_slab.hpp

@@ -119,8 +119,7 @@ class SlabAlloc : public Allocator {
         const char* encryption_key = nullptr;
     };
 
-    struct Retry {
-    };
+    struct Retry {};
 
     /// \brief Attach this allocator to the specified file.
     ///
@@ -153,6 +152,12 @@ class SlabAlloc : public Allocator {
     /// \throw SlabAlloc::Retry
     ref_type attach_file(const std::string& file_path, Config& cfg, util::WriteObserver* write_observer = nullptr);
 
+    /// @brief Expand or contract file
+    /// @param ref_type ref to the top array
+    /// @param cfg configuration, see attach_file
+    /// \return true if the file size was changed
+    bool align_filesize_for_mmap(size_t ref_type, Config& cfg);
+
     /// If the attached file is in streaming form, convert it to normal form.
     ///
     /// This conversion must be done as part of session initialization to avoid
@@ -204,13 +209,18 @@ class SlabAlloc : public Allocator {
 
     /// Detach from a previously attached file or buffer.
     ///
+    /// if 'keep_file_attached' is set, only memory mappings will
+    /// be closed, but the file descriptor remains valid for further
+    /// operations on the file. Caller must close the file explicitly
+    /// to bring the allocator to a fully detached state.
+    ///
     /// This function does not reset free space tracking. To
     /// completely reset the allocator, you must also call
     /// reset_free_space_tracking().
     ///
     /// This function has no effect if the allocator is already in the
     /// detached state (idempotency).
-    void detach() noexcept;
+    void detach(bool keep_file_attached = false) noexcept;
 
     class DetachGuard;
 

src/realm/db.cpp

@@ -1306,7 +1306,18 @@ void DB::open(const std::string& path, bool no_create_file, const DBOptions& opt
                 }
 
                 alloc.convert_from_streaming_form(top_ref);
-
+                try {
+                    bool file_changed_size = alloc.align_filesize_for_mmap(top_ref, cfg);
+                    if (file_changed_size) {
+                        // we need to re-establish proper mappings after file size change.
+                        // we do this simply by aborting and starting all over:
+                        continue;
+                    }
+                }
+                // something went wrong. Retry.
+                catch (SlabAlloc::Retry&) {
+                    continue;
+                }
                 if (options.encryption_key) {
 #ifdef _WIN32
                     uint64_t pid = GetCurrentProcessId();

src/realm/group.cpp

@@ -1112,6 +1112,8 @@ void Group::write(std::ostream& out, int file_format_version, TableWriter& table
                 top.add(RefOrTagged::make_tagged(history_info.version));
                 top.add(RefOrTagged::make_tagged(history_info.sync_file_id));
                 top_size = s_group_max_size;
+                // ^ this is too large, since the evacuation point entry is not there:
+                // (but the code below is self correcting)
             }
         }
         top_ref = out_2.get_ref_of_next_array();

src/realm/sync/noinst/pending_bootstrap_store.cpp

@@ -176,13 +176,13 @@ void PendingBootstrapStore::add_batch(int64_t query_version, util::Optional<Sync
     }
 
     if (did_create) {
-        m_logger.trace("Created new pending bootstrap object for query version %1", query_version);
+        m_logger.debug("Created new pending bootstrap object for query version %1", query_version);
     }
     else {
-        m_logger.trace("Added batch to pending bootstrap object for query version %1", query_version);
+        m_logger.debug("Added batch to pending bootstrap object for query version %1", query_version);
     }
     if (progress) {
-        m_logger.trace("Finalized pending bootstrap object for query version %1", query_version);
+        m_logger.debug("Finalized pending bootstrap object for query version %1", query_version);
     }
     m_has_pending = true;
 }
@@ -311,12 +311,12 @@ void PendingBootstrapStore::pop_front_pending(const TransactionRef& tr, size_t c
     }
 
     if (changeset_list.is_empty()) {
-        m_logger.trace("Removing pending bootstrap obj for query version %1",
+        m_logger.debug("Removing pending bootstrap obj for query version %1",
                        bootstrap_obj.get<int64_t>(m_query_version));
         bootstrap_obj.remove();
     }
     else {
-        m_logger.trace("Removing pending bootstrap batch for query version %1. %2 changeset remaining",
+        m_logger.debug("Removing pending bootstrap batch for query version %1. %2 changeset remaining",
                        bootstrap_obj.get<int64_t>(m_query_version), changeset_list.size());
     }
 

src/realm/util/file_mapper.hpp

@@ -178,6 +178,15 @@ void encryption_read_barrier(const File::Map<T>& map, size_t index, size_t num_e
     }
 }
 
+template <typename T>
+void encryption_read_barrier_for_write(const File::Map<T>& map, size_t index, size_t num_elements = 1)
+{
+    if (auto mapping = map.get_encrypted_mapping(); REALM_UNLIKELY(mapping)) {
+        do_encryption_read_barrier(map.get_addr() + index, sizeof(T) * num_elements, nullptr, mapping,
+                                   map.is_writeable());
+    }
+}
+
 template <typename T>
 void encryption_write_barrier(const File::Map<T>& map, size_t index, size_t num_elements = 1)
 {