From 7e746ad90c88f8708f7ea9177d2728d38f93068f Mon Sep 17 00:00:00 2001 From: Roman Nozdrin Date: Tue, 22 Aug 2023 08:36:37 +0000 Subject: [PATCH] fix(extent-map): MCOL-5559 This patch extends the scope of the crit section to cover remap operation to avoid access races running EMIndex operations --- versioning/BRM/extentmap.cpp | 52 +++++++++++++++++++++++++++++------- versioning/BRM/extentmap.h | 2 ++ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/versioning/BRM/extentmap.cpp b/versioning/BRM/extentmap.cpp index e59af8603..df4bbfc52 100644 --- a/versioning/BRM/extentmap.cpp +++ b/versioning/BRM/extentmap.cpp @@ -2080,20 +2080,54 @@ void ExtentMap::grabEMIndex(OPS op) } else { - // Sending down current Managed Shmem size. If EMIndexImpl instance size doesn't match - // fEMIndexShminfo->allocdSize makeExtentMapIndexImpl will remap managed shmem segment. - fPExtMapIndexImpl_ = - ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), fEMIndexShminfo->allocdSize); + if (op == READ) + { + fMST.getTable_upgrade(MasterSegmentTable::EMIndex); + emIndexLocked = true; - if (r_only) - fPExtMapIndexImpl_->makeReadOnly(); + // Sending down current Managed Shmem size. If EMIndexImpl instance size doesn't match + // fEMIndexShminfo->allocdSize makeExtentMapIndexImpl will remap managed shmem segment. + fPExtMapIndexImpl_ = ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), + fEMIndexShminfo->allocdSize); + + if (r_only) + fPExtMapIndexImpl_->makeReadOnly(); + + emIndexLocked = false; + fMST.getTable_downgrade(MasterSegmentTable::EMIndex); + } + else + { + fPExtMapIndexImpl_ = ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), + fEMIndexShminfo->allocdSize); + + if (r_only) + fPExtMapIndexImpl_->makeReadOnly(); + } } } else if (fPExtMapIndexImpl_->getShmemImplSize() != (unsigned)fEMIndexShminfo->allocdSize) { - fPExtMapIndexImpl_->refreshShm(); - fPExtMapIndexImpl_ = - ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), fEMIndexShminfo->allocdSize); + if (op == READ) + { + fMST.getTable_upgrade(MasterSegmentTable::EMIndex); + emIndexLocked = true; + + fPExtMapIndexImpl_->refreshShm(); + + fPExtMapIndexImpl_ = + ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), fEMIndexShminfo->allocdSize); + + emIndexLocked = false; + fMST.getTable_downgrade(MasterSegmentTable::EMIndex); + } + else + { + fPExtMapIndexImpl_->refreshShm(); + + fPExtMapIndexImpl_ = + ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), fEMIndexShminfo->allocdSize); + } } } diff --git a/versioning/BRM/extentmap.h b/versioning/BRM/extentmap.h index 0f16d3018..5cba29ab3 100644 --- a/versioning/BRM/extentmap.h +++ b/versioning/BRM/extentmap.h @@ -372,6 +372,8 @@ class ExtentMapIndexImpl { if (fInstance_) { + // effectively umpas a mapped managed shmem segment changing the segment VA address + // when mapped next time. delete fInstance_; fInstance_ = nullptr; }