[CP] adjust tablet pointer reset order to avoid memory access violation

src/storage/meta_mem/ob_tablet_pointer.cpp

@@ -77,7 +77,6 @@ ObTabletPointer::~ObTabletPointer()
 
 void ObTabletPointer::reset()
 {
-  ls_handle_.reset();
   {
     ObByteLockGuard guard(ddl_kv_mgr_lock_);
     ddl_kv_mgr_handle_.reset();
@@ -85,11 +84,11 @@ void ObTabletPointer::reset()
   mds_table_handler_.reset();
   protected_memtable_mgr_handle_.reset();
   ddl_info_.reset();
-  initial_state_ = true;
   ATOMIC_STORE(&initial_state_, true);
   old_version_chain_ = nullptr;
   reset_obj();
   phy_addr_.reset();
+  ls_handle_.reset();
 }
 
 void ObTabletPointer::reset_obj()

src/storage/ob_i_memtable_mgr.cpp

@@ -27,6 +27,18 @@ namespace oceanbase
 namespace storage
 {
 
+ObIMemtableMgr::~ObIMemtableMgr()
+{
+  int ret = OB_SUCCESS;
+  const int64_t ref_cnt = get_ref();
+  if (OB_UNLIKELY(0 != ref_cnt)) {
+    STORAGE_LOG(ERROR, "ref cnt is NOT 0", K(ret), K(ref_cnt), K_(tablet_id), KPC(this));
+  }
+
+  reset_tables();
+  ATOMIC_STORE(&ref_cnt_, 0);
+}
+
 int ObIMemtableMgr::get_active_memtable(ObTableHandleV2 &handle) const
 {
   int ret = OB_SUCCESS;

src/storage/ob_i_memtable_mgr.h

@@ -48,6 +48,7 @@ enum LockType
 class MemtableMgrLock
 {
 public:
+  MemtableMgrLock() : lock_type_(OB_LOCK_UNKNOWN), lock_(nullptr) {}
   MemtableMgrLock(const LockType lock_type, void *lock) : lock_type_(lock_type), lock_(lock) {}
   ~MemtableMgrLock() { reset(); }
   void reset()
@@ -137,7 +138,7 @@ public:
     }
   }
 
-private:
+public:
   LockType lock_type_;
   void *lock_;
 };
@@ -193,7 +194,7 @@ private:
 class ObIMemtableMgr
 {
 public:
-  ObIMemtableMgr(const LockType lock_type, void *lock)
+  ObIMemtableMgr()
     : is_inited_(false),
       ref_cnt_(0),
       tablet_id_(),
@@ -202,11 +203,11 @@ public:
       memtable_head_(0),
       memtable_tail_(0),
       t3m_(nullptr),
-      lock_(lock_type, lock)
+      lock_()
   {
     memset(tables_, 0, sizeof(tables_));
   }
-  virtual ~ObIMemtableMgr() { reset_tables(); }
+  virtual ~ObIMemtableMgr();
 
   int init(
       const share::ObLSID &ls_id,
@@ -270,11 +271,7 @@ public:
   OB_INLINE int64_t dec_ref() { return ATOMIC_SAF(&ref_cnt_, 1 /* just sub 1 */); }
   OB_INLINE int64_t get_ref() const { return ATOMIC_LOAD(&ref_cnt_); }
   OB_INLINE void inc_ref() { ATOMIC_INC(&ref_cnt_); }
-  OB_INLINE void reset()
+
-  {
-    destroy();
-    ATOMIC_STORE(&ref_cnt_, 0);
-  }
   virtual int init_storage_recorder(
       const ObTabletID &tablet_id,
       const share::ObLSID &ls_id,

src/storage/tablelock/ob_lock_memtable_mgr.cpp

@@ -28,13 +28,36 @@ namespace transaction
 namespace tablelock
 {
 
-ObLockMemtableMgr::ObLockMemtableMgr() : ObIMemtableMgr(LockType::OB_QSYNC_LOCK, &lock_def_)
+ObLockMemtableMgr::ObLockMemtableMgr()
+  : ls_id_(),
+    lock_def_()
 {
   int ret = OB_SUCCESS;
   if (OB_FAIL(lock_def_.init(lib::ObMemAttr(MTL_ID(), "LockMemtableMgr")))) {
     LOG_WARN("lock memtable mgr lock init error", K(ret), "tenant_id", MTL_ID());
   }
-  UNUSED(ret);
+  lock_.lock_type_ = LockType::OB_QSYNC_LOCK;
+  lock_.lock_ = &lock_def_;
+}
+
+ObLockMemtableMgr::~ObLockMemtableMgr()
+{
+  destroy();
+}
+
+void ObLockMemtableMgr::destroy()
+{
+  int ret = OB_SUCCESS;
+  const int64_t ref_cnt = get_ref();
+  if (OB_UNLIKELY(0 != ref_cnt)) {
+    LOG_ERROR("ref cnt is NOT 0", K(ret), K(ref_cnt), K_(ls_id), KPC(this));
+  }
+
+  MemMgrWLockGuard lock_guard(lock_);
+  reset_tables();
+  ls_id_.reset();
+  freezer_ = NULL;
+  is_inited_ = false;
 }
 
 int ObLockMemtableMgr::init(
@@ -67,19 +90,6 @@ int ObLockMemtableMgr::init(
   return ret;
 }
 
-void ObLockMemtableMgr::destroy()
-{
-  reset();
-}
-
-void ObLockMemtableMgr::reset()
-{
-  MemMgrWLockGuard lock_guard(lock_);
-  reset_tables();
-  freezer_ = NULL;
-  is_inited_ = false;
-}
-
 int ObLockMemtableMgr::create_memtable(const SCN clog_checkpoint_scn,
                                        const int64_t schema_version,
                                        const SCN newest_clog_checkpoint_scn,

src/storage/tablelock/ob_lock_memtable_mgr.h

@@ -48,8 +48,7 @@ class ObLockMemtableMgr : public storage::ObIMemtableMgr
 {
 public:
   ObLockMemtableMgr();
-  virtual ~ObLockMemtableMgr() { reset(); }
+  virtual ~ObLockMemtableMgr();
-  void reset();
 
   // ================== Unified Class Method ==================
   //

src/storage/tablet/ob_tablet.h

@@ -115,9 +115,9 @@ public:
   ~ObTableStoreCache() { reset(); }
   void reset();
   int init(
-    const ObSSTableArray &major_tables,
+      const ObSSTableArray &major_tables,
-    const ObSSTableArray &minor_tables,
+      const ObSSTableArray &minor_tables,
-    const bool is_row_store);
+      const bool is_row_store);
   void assign(const ObTableStoreCache &other);
   TO_STRING_KV(K_(last_major_snapshot_version), K_(major_table_cnt),
       K_(minor_table_cnt), K_(recycle_version), K_(last_major_column_count), K_(is_row_store),

src/storage/tablet/ob_tablet_memtable_mgr.cpp

@@ -34,13 +34,15 @@ namespace storage
 using namespace mds;
 
 ObTabletMemtableMgr::ObTabletMemtableMgr()
-  : ObIMemtableMgr(LockType::OB_SPIN_RWLOCK, &lock_def_),
+  : ls_(nullptr),
-    ls_(nullptr),
     lock_def_(common::ObLatchIds::TABLET_MEMTABLE_LOCK),
     retry_times_(0),
     schema_recorder_(),
     medium_info_recorder_()
 {
+  lock_.lock_type_ = LockType::OB_SPIN_RWLOCK;
+  lock_.lock_ = &lock_def_;
+
 #if defined(__x86_64__)
   static_assert(sizeof(ObTabletMemtableMgr) <= 480, "The size of ObTabletMemtableMgr will affect the meta memory manager, and the necessity of adding new fields needs to be considered.");
 #endif
@@ -81,6 +83,11 @@ void ObTabletMemtableMgr::destroy()
   is_inited_ = false;
 }
 
+void ObTabletMemtableMgr::reset()
+{
+  destroy();
+}
+
 int ObTabletMemtableMgr::init(const common::ObTabletID &tablet_id,
                               const ObLSID &ls_id,
                               ObFreezer *freezer,

src/storage/tablet/ob_tablet_memtable_mgr.h

@@ -56,6 +56,7 @@ public:
   virtual int get_active_memtable(ObTableHandleV2 &handle) const override;
   virtual int get_all_memtables(ObTableHdlArray &handle) override;
   virtual void destroy() override;
+  void reset();
   uint32_t get_ls_freeze_clock();
 
   bool has_active_memtable();

src/storage/tx_table/ob_tx_ctx_memtable_mgr.cpp

@@ -25,6 +25,34 @@ using namespace palf;
 namespace storage
 {
 
+ObTxCtxMemtableMgr::ObTxCtxMemtableMgr()
+  : ls_id_(),
+    lock_def_()
+{
+  lock_.lock_type_ = LockType::OB_SPIN_RWLOCK;
+  lock_.lock_ = &lock_def_;
+}
+
+ObTxCtxMemtableMgr::~ObTxCtxMemtableMgr()
+{
+  destroy();
+}
+
+void ObTxCtxMemtableMgr::destroy()
+{
+  int ret = OB_SUCCESS;
+  const int64_t ref_cnt = get_ref();
+  if (OB_UNLIKELY(0 != ref_cnt)) {
+    LOG_ERROR("ref cnt is NOT 0", K(ret), K(ref_cnt), K_(ls_id), KPC(this));
+  }
+
+  MemMgrWLockGuard lock_guard(lock_);
+  reset_tables();
+  ls_id_.reset();
+  freezer_ = NULL;
+  is_inited_ = false;
+}
+
 int ObTxCtxMemtableMgr::init(const common::ObTabletID &tablet_id,
                              const ObLSID &ls_id,
                              ObFreezer *freezer,
@@ -45,19 +73,6 @@ int ObTxCtxMemtableMgr::init(const common::ObTabletID &tablet_id,
   return ret;
 }
 
-void ObTxCtxMemtableMgr::destroy()
-{
-  reset();
-}
-
-void ObTxCtxMemtableMgr::reset()
-{
-  MemMgrWLockGuard lock_guard(lock_);
-  reset_tables();
-  freezer_ = NULL;
-  is_inited_ = false;
-}
-
 int ObTxCtxMemtableMgr::create_memtable(const SCN last_replay_scn,
                                         const int64_t schema_version,
                                         const SCN newest_clog_checkpoint_scn,
@@ -90,7 +105,7 @@ int ObTxCtxMemtableMgr::create_memtable(const SCN last_replay_scn,
     LOG_WARN("failed to create memtable", K(ret));
   } else if (OB_ISNULL(table = handle.get_table())) {
     ret = OB_ERR_UNEXPECTED;
-    STORAGE_LOG(ERROR, "table is nullptr", K(ret));
+    LOG_ERROR("table is nullptr", K(ret));
   } else {
     tx_ctx_memtable = dynamic_cast<ObTxCtxMemtable *>(table);
     if (NULL == tx_ctx_memtable) {

src/storage/tx_table/ob_tx_ctx_memtable_mgr.h

@@ -42,9 +42,8 @@ namespace storage
 class ObTxCtxMemtableMgr : public ObIMemtableMgr
 {
 public:
-  ObTxCtxMemtableMgr() : ObIMemtableMgr(LockType::OB_SPIN_RWLOCK, &lock_def_) {}
+  ObTxCtxMemtableMgr();
-  ~ObTxCtxMemtableMgr() {}
+  virtual ~ObTxCtxMemtableMgr();
-  void reset();
 
   // ================== Unified Class Method ==================
   //

src/storage/tx_table/ob_tx_data_memtable_mgr.cpp

@@ -26,6 +26,42 @@ namespace checkpoint
   class ObCheckpointExecutor;
 }
 
+ObTxDataMemtableMgr::ObTxDataMemtableMgr()
+  : is_freezing_(false),
+    ls_id_(),
+    mini_merge_recycle_commit_versions_ts_(0),
+    tx_data_table_(nullptr),
+    ls_tablet_svr_(nullptr),
+    slice_allocator_(nullptr)
+{
+  lock_.lock_type_ = LockType::OB_SPIN_RWLOCK;
+  lock_.lock_ = &lock_def_;
+}
+
+ObTxDataMemtableMgr::~ObTxDataMemtableMgr()
+{
+  destroy();
+}
+
+void ObTxDataMemtableMgr::destroy()
+{
+  int ret = OB_SUCCESS;
+  const int64_t ref_cnt = get_ref();
+  if (OB_UNLIKELY(0 != ref_cnt)) {
+    STORAGE_LOG(ERROR, "ref cnt is NOT 0", K(ret), K(ref_cnt), K_(ls_id), KPC(this));
+  }
+
+  MemMgrWLockGuard guard(lock_);
+  reset_tables();
+  ls_id_.reset();
+  tablet_id_.reset();
+  tx_data_table_ = nullptr;
+  ls_tablet_svr_ = nullptr;
+  freezer_ = nullptr;
+  mini_merge_recycle_commit_versions_ts_ = 0;
+  is_inited_ = false;
+}
+
 int ObTxDataMemtableMgr::init(const common::ObTabletID &tablet_id,
                               const ObLSID &ls_id,
                               ObFreezer *freezer,
@@ -76,19 +112,6 @@ int ObTxDataMemtableMgr::init(const common::ObTabletID &tablet_id,
   return ret;
 }
 
-void ObTxDataMemtableMgr::destroy()
-{
-  MemMgrWLockGuard guard(lock_);
-  reset_tables();
-  ls_id_ = 0;
-  tablet_id_ = 0;
-  tx_data_table_ = nullptr;
-  ls_tablet_svr_ = nullptr;
-  freezer_ = nullptr;
-  mini_merge_recycle_commit_versions_ts_ = 0;
-  is_inited_ = false;
-}
-
 int ObTxDataMemtableMgr::offline()
 {
   int ret = OB_SUCCESS;

src/storage/tx_table/ob_tx_data_memtable_mgr.h

@@ -82,15 +82,8 @@ private:
   static const int64_t TX_DATA_MEMTABLE_MAX_FREEZE_WAIT_TIME = 1000; // 1ms
 
 public:  // ObTxDataMemtableMgr
-  ObTxDataMemtableMgr()
+  ObTxDataMemtableMgr();
-    : ObIMemtableMgr(LockType::OB_SPIN_RWLOCK, &lock_def_),
+  virtual ~ObTxDataMemtableMgr();
-      is_freezing_(false),
-      ls_id_(0),
-      mini_merge_recycle_commit_versions_ts_(0),
-      tx_data_table_(nullptr),
-      ls_tablet_svr_(nullptr),
-      slice_allocator_(nullptr) {}
-  virtual ~ObTxDataMemtableMgr() = default;
   int init(const common::ObTabletID &tablet_id,
            const share::ObLSID &ls_id,
            ObFreezer *freezer,

unittest/storage/tx_table/test_tx_ctx_table.cpp

@@ -143,7 +143,7 @@ protected:
   }
   virtual void TearDown() override
   {
-    ctx_mt_mgr_->reset();
+    ctx_mt_mgr_->destroy();
     ls_tx_ctx_mgr_.reset();
     delete mt_mgr_;
     mt_mgr_ = NULL;