diff --git a/velox/common/memory/MemoryArbitrator.cpp b/velox/common/memory/MemoryArbitrator.cpp index 769630bf8bd3..a98ecd395788 100644 --- a/velox/common/memory/MemoryArbitrator.cpp +++ b/velox/common/memory/MemoryArbitrator.cpp @@ -241,15 +241,24 @@ uint64_t MemoryReclaimer::reclaim( std::shared_ptr pool; int64_t reclaimableBytes; }; + + // NOTE: We hold candidate reference for non-reclaimable pools as well. This + // is to make sure child shared pointer is stored to keep child alive, + // avoiding destruction of child pool within below parents' 'poolMutex_' lock. + // Otherwise a double acquisition of 'poolMutex_' can happen in destructor, + // which creates deadlock. + std::vector nonReclaimableCandidates; std::vector candidates; { std::shared_lock guard{pool->poolMutex_}; candidates.reserve(pool->children_.size()); + nonReclaimableCandidates.reserve(pool->children_.size()); for (auto& entry : pool->children_) { auto child = entry.second.lock(); if (child != nullptr) { const auto reclaimableBytesOpt = child->reclaimableBytes(); if (!reclaimableBytesOpt.has_value()) { + nonReclaimableCandidates.push_back(Candidate{std::move(child), 0}); continue; } candidates.push_back(Candidate{