Fix corruption of freelist during compaction (#6054)

Wrong value for the size of the top array was used because values were added after the sizes were calculated. Now the decision to compact the realm is moved to a point before the sizes are calculated so that the correct top array size is used. The reason for moving the logic to the point after the calculation of the sizes was to use an updated values for the free space size - mostly in order to get a test passing, but that was obviously a wrong decision.

CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Fixed wrong assertion on query error that could result in a crash. ([#6038](https://github.com/realm/realm-core/issues/6038), since v11.7.0)
+* Freelist may be corrupted if compaction was initiated ([#6054](https://github.com/realm/realm-core/pull/6054), since v13.0.0)
 
 ### Breaking changes
 * Updated `logger_factory` in SyncClientConfig to return a `shared_ptr` instead of a `unique_ptr` ([PR #5980](https://github.com/realm/realm-core/pull/5980))

src/realm/group_writer.cpp

@@ -766,6 +766,26 @@ ref_type GroupWriter::write_group()
     top.set(Group::s_free_version_ndx, from_ref(free_versions_ref));            // Throws
     top.set(Group::s_version_ndx, RefOrTagged::make_tagged(m_current_version)); // Throws
 
+    if (m_evacuation_limit == 0 && m_backoff == 0) {
+        size_t used_space = m_logical_size - m_free_space_size;
+        // Compacting files smaller than 1 Mb is not worth the effort. Arbitrary chosen value.
+        static constexpr size_t minimal_compaction_size = 0x100000;
+        // If we make the file too small, there is a big chance it will grow immediately afterwards
+        static constexpr size_t minimal_evac_limit = 0x10000;
+        if (m_logical_size >= minimal_compaction_size && m_free_space_size - m_locked_space_size > 2 * used_space) {
+            // Clean up potential
+            auto limit = util::round_up_to_page_size(used_space + used_space / 2 + m_locked_space_size);
+            m_evacuation_limit = std::max(minimal_evac_limit, limit);
+            // From now on, we will only allocate below this limit
+            // Save the limit in the file
+            while (top.size() <= Group::s_evacuation_point_ndx) {
+                top.add(0);
+            }
+            top.set(Group::s_evacuation_point_ndx, RefOrTagged::make_tagged(m_evacuation_limit));
+            // std::cout << "Evacuation point = " << std::hex << m_evacuation_limit << std::dec << std::endl;
+        }
+    }
+
     // Get final sizes
     size_t top_byte_size = top.get_byte_size();
     ref_type end_ref = top_ref + top_byte_size;
@@ -791,26 +811,6 @@ ref_type GroupWriter::write_group()
     m_free_lengths.set(reserve_ndx, value_9);   // Throws
     m_free_space_size += rest;
 
-    if (m_evacuation_limit == 0 && m_backoff == 0) {
-        size_t used_space = m_logical_size - m_free_space_size;
-        // Compacting files smaller than 1 Mb is not worth the effort. Arbitrary chosen value.
-        static constexpr size_t minimal_compaction_size = 0x100000;
-        // If we make the file too small, there is a big chance it will grow immediately afterwards
-        static constexpr size_t minimal_evac_limit = 0x10000;
-        if (m_logical_size >= minimal_compaction_size && m_free_space_size - m_locked_space_size > 2 * used_space) {
-            // Clean up potential
-            auto limit = util::round_up_to_page_size(used_space + used_space / 2 + m_locked_space_size);
-            m_evacuation_limit = std::max(minimal_evac_limit, limit);
-            // From now on, we will only allocate below this limit
-            // Save the limit in the file
-            while (top.size() <= Group::s_evacuation_point_ndx) {
-                top.add(0);
-            }
-            top.set(Group::s_evacuation_point_ndx, RefOrTagged::make_tagged(m_evacuation_limit));
-            // std::cout << "Evacuation point = " << std::hex << m_evacuation_limit << std::dec << std::endl;
-        }
-    }
-
 #if REALM_ALLOC_DEBUG
     std::cout << "  Final Freelist:" << std::endl;
     for (size_t j = 0; j < m_free_positions.size(); ++j) {

test/test_compaction.cpp

@@ -59,11 +59,11 @@ TEST(Compaction_WhileGrowing)
     for (int i = 0; i < 5000; ++i) {
         w[i] = '0' + (i % 64);
     }
-    int num = (REALM_MAX_BPNODE_SIZE == 1000) ? 1490 : 1400;
+    int num = (REALM_MAX_BPNODE_SIZE == 1000) ? 1400 : 1300;
     tr->promote_to_write();
     CHECK(db->get_evacuation_stage() == DB::EvacStage::idle);
     for (int j = 0; j < num; ++j) {
-        table1->create_object().set(col_bin1, BinaryData(w, 400));
+        table1->create_object().set(col_bin1, BinaryData(w, 450));
         table2->create_object().set(col_bin2, BinaryData(w, 200));
         if (j % 10 == 0) {
             tr->commit_and_continue_as_read();
@@ -113,7 +113,7 @@ TEST(Compaction_WhileGrowing)
     tr->commit_and_continue_as_read();
     // Now there should be room for compaction
 
-    auto n = 20; // Ensure that test will end
+    auto n = 50; // Ensure that test will end
     do {
         tr->promote_to_write();
         tr->commit_and_continue_as_read();