Transaction and synchronization guard

[Bug/Feature]   KVStore returns a transaction guard
[Cause]         N/A
[Solution]      N/A
[Verification]  Build, install, run tests

Change-Id: I44d421e5817ca359bf28f70b9182c9e2e0618137

diff --git a/src/config/from-kvstore-visitor.hpp b/src/config/from-kvstore-visitor.hpp
index 0c91eb5..c7e3540 100644 (file)
--- a/src/config/from-kvstore-visitor.hpp
+++ b/src/config/from-kvstore-visitor.hpp
@@ -38,13 +38,15 @@ class FromKVStoreVisitor {
 public:
     explicit FromKVStoreVisitor(const std::string& filePath, const std::string& prefix)
         : mStorePtr(new KVStore(filePath)),
-          mKeyPrefix(prefix)
+          mKeyPrefix(prefix),
+          mTransaction(mStorePtr->getTransaction())
     {
     }
 
     FromKVStoreVisitor(const FromKVStoreVisitor& visitor, const std::string& prefix)
         : mStorePtr(visitor.mStorePtr),
-          mKeyPrefix(prefix)
+          mKeyPrefix(prefix),
+          mTransaction(visitor.mTransaction)
     {
     }
 
@@ -63,6 +65,7 @@ public:
 private:
     std::shared_ptr<KVStore> mStorePtr;
     std::string mKeyPrefix;
+    KVStore::Transaction mTransaction;
 
     template<typename T, typename std::enable_if<!isVisitable<T>::value, int>::type = 0>
     void getInternal(const std::string& name, T& value)
@@ -81,6 +84,7 @@ private:
     void getInternal(const std::string& name, std::vector<T>& values)
     {
         values.clear();
+
         size_t vectorSize = mStorePtr->get<size_t>(name);
         if (vectorSize == 0) {
             return;

diff --git a/src/config/kvstore.cpp b/src/config/kvstore.cpp
index 6c26e64..621f7f6 100644 (file)
--- a/src/config/kvstore.cpp
+++ b/src/config/kvstore.cpp
@@ -95,34 +95,43 @@ void sqliteEscapeStr(::sqlite3_context* context, int argc, ::sqlite3_value** val
 
 } // namespace
 
-
-
-struct KVStore::Transaction {
-    Transaction(KVStore* kvstorePtr)
+struct KVStore::TransactionImpl {
+    TransactionImpl(KVStore* kvstorePtr)
         : mKVStorePtr(kvstorePtr)
     {
-        if (mKVStorePtr->mTransactionCounter == 0) {
-            mKVStorePtr->mConn.exec("BEGIN EXCLUSIVE TRANSACTION");
-        }
-        mKVStorePtr->mTransactionCounter += 1;
+        mKVStorePtr->mConnMtx.lock();
+        mKVStorePtr->mConn.exec("BEGIN EXCLUSIVE TRANSACTION");
     }
 
-    ~Transaction()
+    ~TransactionImpl()
     {
-        mKVStorePtr->mTransactionCounter -= 1;
-        if (mKVStorePtr->mTransactionCounter == 0) {
-            if (std::uncaught_exception()) {
+        if (std::uncaught_exception()) {
+            try {
                 mKVStorePtr->mConn.exec("ROLLBACK TRANSACTION");
-            } else {
+            } catch (ConfigException&) {}
+        } else {
+            try {
                 mKVStorePtr->mConn.exec("COMMIT TRANSACTION");
-            }
+            } catch (ConfigException&) {}
         }
+        mKVStorePtr->mConnMtx.unlock();
     }
 
 private:
     config::KVStore* mKVStorePtr;
 };
 
+KVStore::Transaction KVStore::getTransaction()
+{
+    Lock lock(getTransactionMutex);
+
+    auto t = mTransactionImplPtr.lock();
+    if (!t) {
+        t = std::make_shared<TransactionImpl>(this);
+        mTransactionImplPtr = t;
+    }
+    return t;
+}
 
 KVStore::KVStore(const std::string& path)
     : mPath(path),
@@ -147,8 +156,7 @@ KVStore::~KVStore()
 
 void KVStore::setupDb()
 {
-    Lock lock(mConnMtx);
-    Transaction transaction(this);
+    Transaction transaction = getTransaction();
     mConn.exec("CREATE TABLE IF NOT EXISTS data (key TEXT PRIMARY KEY, value TEXT NOT NULL)");
 }
 
@@ -176,14 +184,13 @@ void KVStore::createFunctions()
 
 void KVStore::clear()
 {
-    Lock lock(mConnMtx);
-    Transaction transaction(this);
+    Transaction transaction = getTransaction();
     mConn.exec("DELETE FROM data");
 }
 
 bool KVStore::isEmpty()
 {
-    Lock lock(mConnMtx);
+    Transaction transaction = getTransaction();
     ScopedReset scopedReset(mGetIsEmptyStmt);
 
     int ret = ::sqlite3_step(mGetIsEmptyStmt->get());
@@ -198,7 +205,7 @@ bool KVStore::isEmpty()
 
 bool KVStore::exists(const std::string& key)
 {
-    Lock lock(mConnMtx);
+    Transaction transaction = getTransaction();
     ScopedReset scopedReset(mGetKeyExistsStmt);
 
     ::sqlite3_bind_text(mGetKeyExistsStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_TRANSIENT);
@@ -215,13 +222,7 @@ bool KVStore::exists(const std::string& key)
 
 void KVStore::remove(const std::string& key)
 {
-    Lock lock(mConnMtx);
-    Transaction transaction(this);
-    removeInternal(key);
-}
-
-void KVStore::removeInternal(const std::string& key)
-{
+    Transaction transaction = getTransaction();
     ScopedReset scopedReset(mRemoveValuesStmt);
 
     ::sqlite3_bind_text(mRemoveValuesStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_STATIC);
@@ -233,13 +234,12 @@ void KVStore::removeInternal(const std::string& key)
 
 void KVStore::setInternal(const std::string& key, const std::string& value)
 {
-    Lock lock(mConnMtx);
+    Transaction transaction = getTransaction();
+    ScopedReset scopedReset(mSetValueStmt);
 
     ::sqlite3_bind_text(mSetValueStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_STATIC);
     ::sqlite3_bind_text(mSetValueStmt->get(), 2, value.c_str(), AUTO_DETERM_SIZE, SQLITE_STATIC);
 
-    Transaction transaction(this);
-    ScopedReset scopedReset(mSetValueStmt);
 
     if (::sqlite3_step(mSetValueStmt->get()) != SQLITE_DONE) {
         throw ConfigException("Error during stepping: " + mConn.getErrorMessage());
@@ -257,10 +257,9 @@ void KVStore::setInternal(const std::string& key, const std::vector<std::string>
         throw ConfigException("Too many values to insert");
     }
 
-    Lock lock(mConnMtx);
-    Transaction transaction(this);
+    Transaction transaction = getTransaction();
 
-    removeInternal(key);
+    remove(key);
 
     // Save vector's capacity
     setInternal(key, values.size());
@@ -274,13 +273,11 @@ void KVStore::setInternal(const std::string& key, const std::vector<std::string>
 
 std::string KVStore::getInternal(const std::string& key, std::string*)
 {
-    Lock lock(mConnMtx);
+    Transaction transaction = getTransaction();
+    ScopedReset scopedReset(mGetValueStmt);
 
     ::sqlite3_bind_text(mGetValueStmt->get(), 1, key.c_str(), AUTO_DETERM_SIZE, SQLITE_TRANSIENT);
 
-    Transaction transaction(this);
-    ScopedReset scopedReset(mGetValueStmt);
-
     int ret = ::sqlite3_step(mGetValueStmt->get());
     if (ret == SQLITE_DONE) {
         throw ConfigException("No value corresponding to the key: " + key);
@@ -294,8 +291,7 @@ std::string KVStore::getInternal(const std::string& key, std::string*)
 
 std::vector<std::string> KVStore::getInternal(const std::string& key, std::vector<std::string>*)
 {
-    Lock lock(mConnMtx);
-    Transaction transaction(this);
+    Transaction transaction = getTransaction();
 
     unsigned int valuesSize = get<unsigned int>(key);
     std::vector<std::string> values(valuesSize);

diff --git a/src/config/kvstore.hpp b/src/config/kvstore.hpp
index 3a2455b..44c0aa1 100644 (file)
--- a/src/config/kvstore.hpp
+++ b/src/config/kvstore.hpp
@@ -35,12 +35,17 @@
 #include <sstream>
 #include <string>
 #include <vector>
+#include <atomic>
 
 namespace config {
 
 class KVStore {
 
 public:
+    /**
+     * A guard struct for thread synchronization and transaction management.
+     */
+    typedef std::shared_ptr<void> Transaction;
 
     /**
      * @param path configuration database file path
@@ -101,11 +106,15 @@ public:
         return getInternal(key, static_cast<T*>(nullptr));
     }
 
+    KVStore::Transaction getTransaction();
 
 private:
-    struct Transaction;
-    typedef std::lock_guard<std::recursive_mutex> Lock;
-    unsigned int mTransactionCounter;
+    typedef std::lock_guard<std::mutex> Lock;
+
+    struct TransactionImpl;
+    std::weak_ptr<TransactionImpl> mTransactionImplPtr;
+    std::mutex getTransactionMutex;
+    std::mutex mConnMtx;
 
     void setInternal(const std::string& key, const std::string& value);
     void setInternal(const std::string& key, const std::initializer_list<std::string>& values);
@@ -122,8 +131,6 @@ private:
     template<typename T>
     std::vector<T> getInternal(const std::string& key, std::vector<T>*);
 
-    std::recursive_mutex mConnMtx;
-
     std::string mPath;
     sqlite3::Connection mConn;
     std::unique_ptr<sqlite3::Statement> mGetValueStmt;
@@ -136,9 +143,6 @@ private:
     void setupDb();
     void prepareStatements();
     void createFunctions();
-
-    void removeInternal(const std::string& key);
-
 };
 
 namespace {

diff --git a/src/config/to-kvstore-visitor.hpp b/src/config/to-kvstore-visitor.hpp
index 477ff83..37b70a9 100644 (file)
--- a/src/config/to-kvstore-visitor.hpp
+++ b/src/config/to-kvstore-visitor.hpp
@@ -38,13 +38,15 @@ class ToKVStoreVisitor {
 public:
     ToKVStoreVisitor(const std::string& filePath, const std::string& prefix)
         : mStorePtr(new KVStore(filePath)),
-          mKeyPrefix(prefix)
+          mKeyPrefix(prefix),
+          mTransaction(mStorePtr->getTransaction())
     {
     }
 
     ToKVStoreVisitor(const ToKVStoreVisitor& visitor, const std::string& prefix)
         : mStorePtr(visitor.mStorePtr),
-          mKeyPrefix(prefix)
+          mKeyPrefix(prefix),
+          mTransaction(visitor.mTransaction)
     {
     }
 
@@ -63,7 +65,7 @@ public:
 private:
     std::shared_ptr<KVStore> mStorePtr;
     std::string mKeyPrefix;
-
+    KVStore::Transaction mTransaction;
 
     template<typename T, typename std::enable_if<!isVisitable<T>::value, int>::type = 0>
     void setInternal(const std::string& name, const T& value)