[yugabyte#24929] DocDB: Fix CleanupHiddenTables COW lock

Summary:
`CleanupHiddenTables` is calling `Upsert` on entries without holding a COW lock which causes a segmentation fault or bad data read.
Changing the order so that we always hold the write lock.

Fixes yugabyte#24929
Jira: DB-14070

Test Plan: Jenkins

Reviewers: slingam, zdrudi

Reviewed By: zdrudi

Subscribers: ybase

Differential Revision: https://phorge.dev.yugabyte.com/D40063

Author: hari90

src/yb/master/catalog_manager_ext.cc

@@ -2904,18 +2904,19 @@ void CatalogManager::CleanupHiddenTables(
   std::vector<TableInfo::WriteLock> locks;
   for (const auto& table : expired_tables) {
     auto write_lock = table->LockForWrite();
-    if (write_lock->started_deleting()) {
-      continue;
+    if (!write_lock->started_deleting()) {
+      // Because tablets for hidden tables are deleted first, there is nothing left to delete
+      // besides the table metadata itself now. So we skip the DELETING state and transition
+      // directly to DELETED.
+      write_lock.mutable_data()->set_state(
+          SysTablesEntryPB::DELETED, Format("Cleanup hidden table at $0", LocalTimeAsString()));
+      LOG_WITH_PREFIX(INFO) << Format(
+          "Cleaning up hidden table $0: $1", table->name(), AsString(table));
     }
-    // Because tablets for hidden tables are deleted first, there is nothing left to delete besides
-    // the table metadata itself now. So we skip the DELETING state and transition directly to
-    // DELETED.
-    write_lock.mutable_data()->set_state(
-        SysTablesEntryPB::DELETED, Format("Cleanup hidden table at $0", LocalTimeAsString()));
-    LOG_WITH_PREFIX(INFO) << Format(
-        "Cleaning up hidden table $0: $1", table->name(), AsString(table));
+
     locks.push_back(std::move(write_lock));
   }
+
   if (locks.empty()) {
     return;
   }

src/yb/util/cow_object.h

@@ -115,12 +115,10 @@ class CowObject {
   State* mutable_dirty() {
     DCHECK(lock_.HasWriteLock());
     is_dirty_ = true;
-    return DCHECK_NOTNULL(dirty_state_.get());
+    return CHECK_NOTNULL(dirty_state_.get());
   }
 
-  const State& dirty() const {
-    return *DCHECK_NOTNULL(dirty_state_.get());
-  }
+  const State& dirty() const { return *CHECK_NOTNULL(dirty_state_.get()); }
 
   bool is_dirty() const {
     DCHECK(lock_.HasReaders() || lock_.HasWriteLock());