Fix a race condition in subtable accessor deletion

CHANGELOG.md

@@ -5,6 +5,9 @@
 
 ### Fixed
 * macOS binaries were built with the incorrect deployment target (10.14 rather than 10.9). ([Cocoa #6299](https://github.com/realm/realm-cocoa/issues/6299), since 5.23.4).
+* Subtable accessors could be double-deleted if the last reference was released from a different
+  thread at the wrong time. This would typically manifest as "pthread_mutex_destroy() failed", but
+  could also result in other kinds of crashes. ([Cocoa #6333](https://github.com/realm/realm-cocoa/issues/6333)).
 
 ### Breaking changes
 * None.

src/realm/column_table.cpp

@@ -258,20 +258,33 @@ void SubtableColumnBase::SubtableMap::recursive_mark() noexcept
 
 void SubtableColumnBase::SubtableMap::refresh_accessor_tree()
 {
+    typedef _impl::TableFriend tf;
+
     // iterate backwards by index because entries may be removed during iteration
     for (size_t i = m_entries.size(); i > 0; --i) {
         const auto& entry = m_entries[i - 1];
-        // Must hold a counted reference while refreshing
-        TableRef table(entry.m_table);
-        typedef _impl::TableFriend tf;
-        tf::set_ndx_in_parent(*table, entry.m_subtable_ndx);
-        if (tf::is_marked(*table)) {
-            tf::refresh_accessor_tree(*table);
+        auto& table = *entry.m_table;
+
+        // If the table's refcount is 0 then that means that unbind_ptr() is
+        // currently waiting on m_subtable_map_lock on another thread, and will
+        // delete the Table as soon as we release that lock. In this scenario,
+        // we do not want to bump Table's refcount, as decrementing it back to
+        // 0 will then result in us deleting the Table, and the other thread
+        // also deleting the Table.
+        // If the refcount is nonzero then we need to bump it to ensure that
+        // nothing we do below releases it while we're using it.
+        TableRef ref;
+        if (tf::has_references(table)) {
+            ref.reset(&table);
+        }
+        tf::set_ndx_in_parent(table, entry.m_subtable_ndx);
+        if (tf::is_marked(table)) {
+            tf::refresh_accessor_tree(table);
             bool bump_global = false;
-            tf::bump_version(*table, bump_global);
+            tf::bump_version(table, bump_global);
         }
         else {
-            tf::refresh_spec_accessor(*table);
+            tf::refresh_spec_accessor(table);
         }
     }
 }

src/realm/table.hpp

@@ -1270,6 +1270,7 @@ class Table {
 
     void bind_ptr() const noexcept;
     void unbind_ptr() const noexcept;
+    bool has_references() const noexcept;
 
     void register_view(const TableViewBase* view);
     void unregister_view(const TableViewBase* view) noexcept;
@@ -1740,6 +1741,11 @@ inline void Table::unbind_ptr() const noexcept
     }
 }
 
+inline bool Table::has_references() const noexcept
+{
+    return m_ref_count.load() > 0;
+}
+
 inline void Table::register_view(const TableViewBase* view)
 {
     util::LockGuard lock(m_accessor_mutex);
@@ -2788,6 +2794,11 @@ class _impl::TableFriend {
     {
         table.unregister_view(view);
     }
+
+    static bool has_references(const Table& table) noexcept
+    {
+        return table.has_references();
+    }
 };