From 5b4fe43c2577f3f39fd174c84a8c23c99646903d Mon Sep 17 00:00:00 2001
From: Alexander Block <ablock84@gmail.com>
Date: Fri, 17 Jan 2020 16:01:22 +0100
Subject: [PATCH] Replace generic CScopedDBTransaction with specialized
 CEvoDBScopedCommitter (#3292)

This has the wanted side effect of proper locking of "cs" inside
CommitCurTransaction and RollbackCurTransaction, which was not easily
possible to implement in the generic version. This fixes some rare crashes.
---
 src/dbwrapper.h   | 45 ---------------------------------------------
 src/evo/evodb.cpp | 37 +++++++++++++++++++++++++++++++++++++
 src/evo/evodb.h   | 28 ++++++++++++++++++++++++----
 3 files changed, 61 insertions(+), 49 deletions(-)

diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 29beb473d1..82c142c01d 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -724,49 +724,4 @@ public:
     }
 };
 
-template<typename Parent, typename CommitTarget>
-class CScopedDBTransaction {
-public:
-    typedef CDBTransaction<Parent, CommitTarget> Transaction;
-
-private:
-    Transaction &dbTransaction;
-    std::function<void ()> commitHandler;
-    std::function<void ()> rollbackHandler;
-    bool didCommitOrRollback{};
-
-public:
-    explicit CScopedDBTransaction(Transaction &dbTx) : dbTransaction(dbTx) {}
-    ~CScopedDBTransaction() {
-        if (!didCommitOrRollback)
-            Rollback();
-    }
-    void Commit() {
-        assert(!didCommitOrRollback);
-        didCommitOrRollback = true;
-        dbTransaction.Commit();
-        if (commitHandler)
-            commitHandler();
-    }
-    void Rollback() {
-        assert(!didCommitOrRollback);
-        didCommitOrRollback = true;
-        dbTransaction.Clear();
-        if (rollbackHandler)
-            rollbackHandler();
-    }
-
-    static std::unique_ptr<CScopedDBTransaction<Parent, CommitTarget>> Begin(Transaction &dbTx) {
-        assert(dbTx.IsClean());
-        return std::make_unique<CScopedDBTransaction<Parent, CommitTarget>>(dbTx);
-    }
-
-    void SetCommitHandler(const std::function<void ()> &h) {
-        commitHandler = h;
-    }
-    void SetRollbackHandler(const std::function<void ()> &h) {
-        rollbackHandler = h;
-    }
-};
-
 #endif // BITCOIN_DBWRAPPER_H
diff --git a/src/evo/evodb.cpp b/src/evo/evodb.cpp
index b46d6efcb1..b1bf157c2a 100644
--- a/src/evo/evodb.cpp
+++ b/src/evo/evodb.cpp
@@ -6,6 +6,31 @@
 
 CEvoDB* evoDb;
 
+CEvoDBScopedCommitter::CEvoDBScopedCommitter(CEvoDB &_evoDB) :
+    evoDB(_evoDB)
+{
+}
+
+CEvoDBScopedCommitter::~CEvoDBScopedCommitter()
+{
+    if (!didCommitOrRollback)
+        Rollback();
+}
+
+void CEvoDBScopedCommitter::Commit()
+{
+    assert(!didCommitOrRollback);
+    didCommitOrRollback = true;
+    evoDB.CommitCurTransaction();
+}
+
+void CEvoDBScopedCommitter::Rollback()
+{
+    assert(!didCommitOrRollback);
+    didCommitOrRollback = true;
+    evoDB.RollbackCurTransaction();
+}
+
 CEvoDB::CEvoDB(size_t nCacheSize, bool fMemory, bool fWipe) :
     db(fMemory ? "" : (GetDataDir() / "evodb"), nCacheSize, fMemory, fWipe),
     rootBatch(db),
@@ -14,6 +39,18 @@ CEvoDB::CEvoDB(size_t nCacheSize, bool fMemory, bool fWipe) :
 {
 }
 
+void CEvoDB::CommitCurTransaction()
+{
+    LOCK(cs);
+    curDBTransaction.Commit();
+}
+
+void CEvoDB::RollbackCurTransaction()
+{
+    LOCK(cs);
+    curDBTransaction.Clear();
+}
+
 bool CEvoDB::CommitRootTransaction()
 {
     assert(curDBTransaction.IsClean());
diff --git a/src/evo/evodb.h b/src/evo/evodb.h
index 66eedfec72..c620edfb54 100644
--- a/src/evo/evodb.h
+++ b/src/evo/evodb.h
@@ -13,6 +13,22 @@
 // "b_b2" was used after compact diffs were introduced
 static const std::string EVODB_BEST_BLOCK = "b_b2";
 
+class CEvoDB;
+
+class CEvoDBScopedCommitter
+{
+private:
+    CEvoDB& evoDB;
+    bool didCommitOrRollback{false};
+
+public:
+    explicit CEvoDBScopedCommitter(CEvoDB& _evoDB);
+    ~CEvoDBScopedCommitter();
+
+    void Commit();
+    void Rollback();
+};
+
 class CEvoDB
 {
 private:
@@ -21,7 +37,6 @@ private:
 
     typedef CDBTransaction<CDBWrapper, CDBBatch> RootTransaction;
     typedef CDBTransaction<RootTransaction, RootTransaction> CurTransaction;
-    typedef CScopedDBTransaction<RootTransaction, RootTransaction> ScopedTransaction;
 
     CDBBatch rootBatch;
     RootTransaction rootDBTransaction;
@@ -30,11 +45,10 @@ private:
 public:
     explicit CEvoDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false);
 
-    std::unique_ptr<ScopedTransaction> BeginTransaction()
+    std::unique_ptr<CEvoDBScopedCommitter> BeginTransaction()
     {
         LOCK(cs);
-        auto t = ScopedTransaction::Begin(curDBTransaction);
-        return t;
+        return std::make_unique<CEvoDBScopedCommitter>(*this);
     }
 
     CurTransaction& GetCurTransaction()
@@ -84,6 +98,12 @@ public:
 
     bool VerifyBestBlock(const uint256& hash);
     void WriteBestBlock(const uint256& hash);
+
+private:
+    // only CEvoDBScopedCommitter is allowed to invoke these
+    friend class CEvoDBScopedCommitter;
+    void CommitCurTransaction();
+    void RollbackCurTransaction();
 };
 
 extern CEvoDB* evoDb;