From 4930bb9f5f99c80c358924b11170924cc1580bdf Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Tue, 20 Mar 2018 14:06:07 +0300
Subject: [PATCH] Don't hold cs_storage in CKeyHolderStorage while calling
 functions which might lock cs_wallet (#2000)

Co-authored-by: Alexander Block <ablock84@gmail.com>
---
 src/privatesend-util.cpp | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/privatesend-util.cpp b/src/privatesend-util.cpp
index 22bb9af59b..4fc59eaaab 100644
--- a/src/privatesend-util.cpp
+++ b/src/privatesend-util.cpp
@@ -27,32 +27,45 @@ CScript CKeyHolder::GetScriptForDestination() const
 
 CScript CKeyHolderStorage::AddKey(CWallet* pwallet)
 {
+    auto keyHolder = std::unique_ptr<CKeyHolder>(new CKeyHolder(pwallet));
+    auto script = keyHolder->GetScriptForDestination();
+
     LOCK(cs_storage);
-    storage.emplace_back(std::unique_ptr<CKeyHolder>(new CKeyHolder(pwallet)));
+    storage.emplace_back(std::move(keyHolder));
     LogPrintf("CKeyHolderStorage::%s -- storage size %lld\n", __func__, storage.size());
-    return storage.back()->GetScriptForDestination();
+    return script;
 }
 
 void CKeyHolderStorage::KeepAll()
 {
-    LOCK(cs_storage);
-    if (storage.size() > 0) {
-        for (auto &key : storage) {
+    std::vector<std::unique_ptr<CKeyHolder>> tmp;
+    {
+        // don't hold cs_storage while calling KeepKey(), which might lock cs_wallet
+        LOCK(cs_storage);
+        std::swap(storage, tmp);
+    }
+
+    if (tmp.size() > 0) {
+        for (auto &key : tmp) {
             key->KeepKey();
         }
-        LogPrintf("CKeyHolderStorage::%s -- %lld keys kept\n", __func__, storage.size());
-        storage.clear();
+        LogPrintf("CKeyHolderStorage::%s -- %lld keys kept\n", __func__, tmp.size());
     }
 }
 
 void CKeyHolderStorage::ReturnAll()
 {
-    LOCK(cs_storage);
-    if (storage.size() > 0) {
-        for (auto &key : storage) {
+    std::vector<std::unique_ptr<CKeyHolder>> tmp;
+    {
+        // don't hold cs_storage while calling ReturnKey(), which might lock cs_wallet
+        LOCK(cs_storage);
+        std::swap(storage, tmp);
+    }
+
+    if (tmp.size() > 0) {
+        for (auto &key : tmp) {
             key->ReturnKey();
         }
-        LogPrintf("CKeyHolderStorage::%s -- %lld keys returned\n", __func__, storage.size());
-        storage.clear();
+        LogPrintf("CKeyHolderStorage::%s -- %lld keys returned\n", __func__, tmp.size());
     }
 }