fix dead lock when stop storage while balancing (#4016)

* fix dead lock when stop storage while balancing * use SemiFuture in sendSnapshot to avoid deadlock Co-authored-by: Doodle <[email protected]>
vesoft-inc · Mar 17, 2022 · 1004e29 · 1004e29
1 parent a57acd0
commit 1004e29
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 6 deletions.
diff --git a/src/kvstore/raftex/RaftexService.cpp b/src/kvstore/raftex/RaftexService.cpp
@@ -126,14 +126,16 @@ void RaftexService::stop() {
 
   // stop service
   LOG(INFO) << "Stopping the raftex service on port " << serverPort_;
+  std::unordered_map<std::pair<GraphSpaceID, PartitionID>, std::shared_ptr<RaftPart>> parts;
   {
     folly::RWSpinLock::WriteHolder wh(partsLock_);
-    for (auto& p : parts_) {
-      p.second->stop();
-    }
-    parts_.clear();
-    LOG(INFO) << "All partitions have stopped";
+    // partsLock_ should not be hold when waiting for parts stop, so swap them out first
+    parts.swap(parts_);
   }
+  for (auto& p : parts) {
+    p.second->stop();
+  }
+  LOG(INFO) << "All partitions have stopped";
   server_->stop();
 }
 

diff --git a/src/kvstore/raftex/SnapshotManager.cpp b/src/kvstore/raftex/SnapshotManager.cpp
@@ -29,7 +29,10 @@ SnapshotManager::SnapshotManager() {
 folly::Future<StatusOr<std::pair<LogID, TermID>>> SnapshotManager::sendSnapshot(
     std::shared_ptr<RaftPart> part, const HostAddr& dst) {
   folly::Promise<StatusOr<std::pair<LogID, TermID>>> p;
-  auto fut = p.getFuture();
+  // if use getFuture(), the future's executor is InlineExecutor, and if the promise setValue first,
+  // the future's callback will be called directly in thenValue in the same thread, the Host::lock_
+  // would be locked twice in one thread, this will cause deadlock
+  auto fut = p.getSemiFuture().via(executor_.get());
   executor_->add([this, p = std::move(p), part, dst]() mutable {
     auto spaceId = part->spaceId_;
     auto partId = part->partId_;