[Bug/Feature] Invalid mutexes lock sequence in kv store transaction.
Missing db transaction in FromKVJsonVisitor.
[Cause] N/A
[Solution] N/A
[Verification] Build, run tests
Change-Id: Ie44dbfe5ad86c9cbf0e19a162a267676b05f1aed
: mStorePtr(new KVStore(db))
, mKeyPrefix(prefix)
, mIsUnion(false)
+ , mTransaction(mStorePtr->getTransaction())
{
mObject = json_tokener_parse(json.c_str());
if (mObject == nullptr) {
mObject(v.mObject ? json_object_get(v.mObject) : nullptr),
mStorePtr(v.mStorePtr),
mKeyPrefix(v.mKeyPrefix),
- mIsUnion(v.mIsUnion)
+ mIsUnion(v.mIsUnion),
+ mTransaction(v.mTransaction)
{
}
FromKVJsonVisitor& operator=(const FromKVJsonVisitor&) = delete;
std::string mKeyPrefix;
json_object* mObject;
bool mIsUnion;
+ KVStore::Transaction mTransaction;
// create visitor for field object (visitable object)
FromKVJsonVisitor(const FromKVJsonVisitor& v, const std::string& name, const bool isUnion) :
- mStorePtr(v.mStorePtr), mIsUnion(isUnion || v.mIsUnion)
+ mStorePtr(v.mStorePtr),
+ mKeyPrefix(key(v.mKeyPrefix, name)),
+ mIsUnion(isUnion || v.mIsUnion),
+ mTransaction(v.mTransaction)
{
- mKeyPrefix = key(v.mKeyPrefix, name);
json_object* object = nullptr;
if (v.mObject && !json_object_object_get_ex(v.mObject, name.c_str(), &object)) {
if (!mIsUnion)
// create visitor for vector i-th element (visitable object)
FromKVJsonVisitor(const FromKVJsonVisitor& v, int i, const bool isUnion) :
- mStorePtr(v.mStorePtr), mIsUnion(isUnion || v.mIsUnion)
+ mStorePtr(v.mStorePtr),
+ mKeyPrefix(key(v.mKeyPrefix, std::to_string(i))),
+ mIsUnion(isUnion || v.mIsUnion),
+ mTransaction(v.mTransaction)
{
- mKeyPrefix = key(v.mKeyPrefix, std::to_string(i));
json_object* object = nullptr;
if (v.mObject) {
object = json_object_array_get_idx(v.mObject, i);
#include <limits>
#include <memory>
#include <set>
+#include <cassert>
namespace config {
} // namespace
struct KVStore::TransactionImpl {
- TransactionImpl(KVStore* kvstorePtr)
- : mKVStorePtr(kvstorePtr)
+ TransactionImpl(KVStore& kvStore)
+ : mLock(kvStore.mMutex)
+ , mKVStore(kvStore)
{
- mKVStorePtr->mConnMtx.lock();
- mKVStorePtr->mConn.exec("BEGIN EXCLUSIVE TRANSACTION");
+ mKVStore.mConn.exec("BEGIN EXCLUSIVE TRANSACTION");
}
~TransactionImpl()
{
+ // be aware of a bug in gcc with uncaught_exception and rethrow_exception
if (std::uncaught_exception()) {
try {
- mKVStorePtr->mConn.exec("ROLLBACK TRANSACTION");
+ mKVStore.mConn.exec("ROLLBACK TRANSACTION");
} catch (ConfigException&) {}
} else {
try {
- mKVStorePtr->mConn.exec("COMMIT TRANSACTION");
+ mKVStore.mConn.exec("COMMIT TRANSACTION");
} catch (ConfigException&) {}
}
- mKVStorePtr->mConnMtx.unlock();
}
private:
- config::KVStore* mKVStorePtr;
+ config::KVStore::Lock mLock;
+ config::KVStore& mKVStore;
};
KVStore::Transaction KVStore::getTransaction()
{
- Lock lock(getTransactionMutex);
+ Lock lock(mMutex);
auto t = mTransactionImplPtr.lock();
if (!t) {
- t = std::make_shared<TransactionImpl>(this);
+ t = std::make_shared<TransactionImpl>(*this);
mTransactionImplPtr = t;
}
return t;
prepareStatements();
}
-KVStore::KVStore(const KVStore& store)
- : mPath(store.mPath),
- mConn(mPath)
-{
- createFunctions();
- prepareStatements();
-}
-
KVStore::~KVStore()
{
+ assert(!mTransactionImplPtr.lock());
}
void KVStore::setupDb()
{
- Transaction transaction = getTransaction();
+ // called only from ctor, transaction is not needed
mConn.exec("CREATE TABLE IF NOT EXISTS data (key TEXT PRIMARY KEY, value TEXT NOT NULL)");
}
/**
* @param path configuration database file path
*/
- KVStore(const std::string& path);
- KVStore(const KVStore& store);
+ explicit KVStore(const std::string& path);
~KVStore();
+ KVStore(const KVStore&) = delete;
+ KVStore& operator=(const KVStore&) = delete;
+
/**
* Clears all the stored data
*/
KVStore::Transaction getTransaction();
private:
- typedef std::lock_guard<std::mutex> Lock;
+ typedef std::lock_guard<std::recursive_mutex> Lock;
struct TransactionImpl;
std::weak_ptr<TransactionImpl> mTransactionImplPtr;
- std::mutex getTransactionMutex;
- std::mutex mConnMtx;
+ std::recursive_mutex mMutex;
void setInternal(const std::string& key, const std::string& value);
void setInternal(const std::string& key, const std::initializer_list<std::string>& values);