From 8141fd0984bddb440d7afba0086f170900dbe0be Mon Sep 17 00:00:00 2001 From: Kyungwook Tak Date: Tue, 20 Dec 2016 17:56:42 +0900 Subject: [PATCH] Refactor: c pointer to std::unique_ptr on db db connection info map in DTapps2SqliteDB have resource leak (isn't deleted when db is closed). To manage it in c++ style, use std::unique_ptr instead of c pointer at least here... Change-Id: I6e645fcf654fa95dbafe99426f461cb42d273fe4 Signed-off-by: Kyungwook Tak --- tappsd/src/db/DTapps2SqliteDB.cpp | 215 ++++++++++++++++---------------------- 1 file changed, 89 insertions(+), 126 deletions(-) diff --git a/tappsd/src/db/DTapps2SqliteDB.cpp b/tappsd/src/db/DTapps2SqliteDB.cpp index e06b4ac..49f26f8 100644 --- a/tappsd/src/db/DTapps2SqliteDB.cpp +++ b/tappsd/src/db/DTapps2SqliteDB.cpp @@ -21,6 +21,8 @@ #include "DTapps2SqliteDB.h" #include +#include +#include /* Define EXPORT_API temporary */ #ifndef EXPORT_API @@ -40,7 +42,7 @@ typedef long int dtappsThreadID; }; using namespace std; - static std::map g_dtapps_sqlite_connection_table; + static std::map> g_dtapps_sqlite_connection_table; DtappsDBConnectionInfo::DtappsDBConnectionInfo() { @@ -87,72 +89,55 @@ BOOL DTappsDBOpen(void *&pDb, const char* CallingFun) TAPPSDbApiLock Dblock; dtappsThreadID id_curr_thread = drmgettid(); - bool IsMemAllocated = false; - int result = SQLITE_ERROR; - unsigned int pairCount = 0; - sqlite3* h_db = NULL; - DtappsDBConnectionInfo *pDBConnectionInfo = NULL; - DRM_TAPPS_SECURE_LOG("DB-OPEN-CLOSE [%s]Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",__func__,getppid(),getpid(),drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - DRM_TAPPS_FRQ_LOG("pairCount=[%u]", pairCount); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it != g_dtapps_sqlite_connection_table.end()) { + DRM_TAPPS_FRQ_LOG("Connection already exists.."); + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_FRQ_LOG("pDBConnectionInfo=[0x%x]", pDBConnectionInfo.get()); - if (0 != pairCount) - { - DRM_TAPPS_FRQ_LOG("Connection already exists.. pairCount=[%ld]", pairCount); - pDBConnectionInfo = g_dtapps_sqlite_connection_table[id_curr_thread]; - DRM_TAPPS_FRQ_LOG("pDBConnectionInfo=[0x%x]", pDBConnectionInfo); + if (pDBConnectionInfo == NULL || pDBConnectionInfo->pDBConnection == NULL) + return FALSE; - MTRY_BL(NULL != pDBConnectionInfo); - MTRY_BL(NULL != pDBConnectionInfo->pDBConnection); DRM_TAPPS_FRQ_LOG("pDBConnectionInfo->countOpenConnection=[%d], pDBConnectionInfo->pDBConnection=[0x%x]", pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); ++(pDBConnectionInfo->countOpenConnection); pDb = pDBConnectionInfo->pDBConnection; - } - else - { - DRM_TAPPS_LOG("no connection exists.. pairCount=[%ld]", pairCount); - pDBConnectionInfo = new(std::nothrow) DtappsDBConnectionInfo; - MTRY_BL(NULL != pDBConnectionInfo); - IsMemAllocated = true; // prevent fix + } else { + DRM_TAPPS_LOG("no connection exists.."); + std::unique_ptr pDBConnectionInfo( + new(std::nothrow) DtappsDBConnectionInfo); + if (pDBConnectionInfo == NULL) + return FALSE; DRM_TAPPS_FRQ_LOG("Opening DB connection."); - result = db_util_open(DTAPPS_DB_NAME, &h_db, 0); - if (result != SQLITE_OK) - { + sqlite3 *h_db = NULL; + int result = db_util_open(DTAPPS_DB_NAME, &h_db, 0); + if (result != SQLITE_OK) { DRM_TAPPS_EXCEPTION("sqlite msg :[%d]%s",result, sqlite3_errmsg(h_db)); DRM_TAPPS_SECURE_LOG("db name :%s", DTAPPS_DB_NAME); - MTHROW_BL + return FALSE; } DRM_TAPPS_FRQ_LOG("sqlite3_open() is success. h_db:%x", h_db); pDBConnectionInfo->countOpenConnection = 1; - pDBConnectionInfo->pDBConnection = h_db; h_db = NULL; + pDBConnectionInfo->pDBConnection = h_db; pDb = pDBConnectionInfo->pDBConnection; // Insert the node DRM_TAPPS_FRQ_LOG("pDBConnectionInfo->countOpenConnection=[%d], pDBConnectionInfo->pDBConnection=[0x%x]", pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - g_dtapps_sqlite_connection_table.insert(pair(id_curr_thread, pDBConnectionInfo));pDBConnectionInfo = NULL; + g_dtapps_sqlite_connection_table.emplace( + std::make_pair(id_curr_thread, std::move(pDBConnectionInfo))); } DRM_TAPPS_LOG("This fn finishes successfully."); return TRUE; - -MCATCH_B - - if (IsMemAllocated && pDBConnectionInfo) - delete pDBConnectionInfo; - - DRM_TAPPS_EXCEPTION("This fn fails"); - - return FALSE; } BOOL DTappsDBGet(void *& pDBConnection) @@ -160,29 +145,24 @@ BOOL DTappsDBGet(void *& pDBConnection) DRM_TAPPS_LOG("Inside %s", __func__); TAPPSDbApiLock Dblock; - unsigned int pairCount; dtappsThreadID id_curr_thread = drmgettid(); pDBConnection = NULL; DRM_TAPPS_LOG("Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",getppid(),getpid(),drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - MTRY_BL(0 != pairCount); - - DRM_TAPPS_FRQ_LOG("g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection=[%d], g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection=[0x%x]", g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection, g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it == g_dtapps_sqlite_connection_table.end() || + it->second == NULL || + it->second->pDBConnection == NULL) + return FALSE; - pDBConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection; + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_FRQ_LOG("countOpenConnection=[%d], pDBConnection=[0x%x]", + pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - DRM_TAPPS_LOG("pairCount=[%u], g_dtapps_sqlite_connection_table[connectionIterator]=[0x%x]", pairCount, g_dtapps_sqlite_connection_table[id_curr_thread]); - DRM_TAPPS_LOG(" This fn finishes successfully..pDBConnection=[0x%x]", pDBConnection); - DRM_TAPPS_FRQ_LOG("pDBConnection=[0x%x]",pDBConnection); + pDBConnection = pDBConnectionInfo->pDBConnection; return TRUE; - -MCATCH_B - DRM_TAPPS_EXCEPTION("This fn fails.. pDBConnection=[0x%x]", pDBConnection); - - return FALSE; } BOOL DTappsDBClose(const char* CallingFun) @@ -190,91 +170,81 @@ BOOL DTappsDBClose(const char* CallingFun) DRM_TAPPS_LOG("Inside %s Calling function = %s", __func__, CallingFun); TAPPSDbApiLock Dblock; - unsigned int pairCount; - sqlite3_stmt* pstmt = NULL; dtappsThreadID id_curr_thread = drmgettid(); - int countConnection; - sqlite3 *pDBConnection = NULL; - int result = SQLITE_ERROR; - DRM_TAPPS_SECURE_LOG("DB-OPEN-CLOSE [%s]Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",__func__,getppid(),getpid(),drmgettid(), id_curr_thread); + DRM_TAPPS_SECURE_LOG( + "DB-OPEN-CLOSE [%s]Parent Process ID=[%ld]:Process-ID=[%ld]" + ":Thread-ID=[%ld], id_curr_thread=[%ld]", + __func__, getppid(), getpid(), drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - MTRY_BL(0 != pairCount); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it == g_dtapps_sqlite_connection_table.end()) { + return FALSE; + } else if (it->second == NULL || it->second->pDBConnection == NULL) { + g_dtapps_sqlite_connection_table.erase(it); + return FALSE; + } - DRM_TAPPS_FRQ_LOG("g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection=[%d], g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection=[0x%x] \n", g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection, g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection); + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_FRQ_LOG("countOpenConnection=[%d], pDBConnection=[0x%x]", + pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - --(g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection); - countConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection; + int countConnection = --(pDBConnectionInfo->countOpenConnection); DRM_TAPPS_LOG(" countConnection=[%d] ", countConnection); + if (countConnection != 0) + return TRUE; - if (0 == countConnection) - { - DRM_TAPPS_LOG("closing DB connection info "); - - pDBConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection; - - DRM_TAPPS_LOG("pairCount=[%u], g_sqlite_connection_table[connectionIterator]=[0x%x]", pairCount, g_dtapps_sqlite_connection_table[id_curr_thread]); - DRM_TAPPS_LOG("erasing map entry..pDBConnection=[0x%x]", pDBConnection); + DRM_TAPPS_LOG("closing DB connection info "); - g_dtapps_sqlite_connection_table.erase(id_curr_thread); + sqlite3 *pDBConnection = it->second->pDBConnection; - DRM_TAPPS_LOG("finalizing all statements..pDBConnection=[0x%x]", pDBConnection); - - while ((pstmt = sqlite3_next_stmt(pDBConnection, pstmt)) != NULL) // prevent fix - { - DRM_TAPPS_LOG("finalizing DB statement..pstmt=[0x%x]", pstmt); - sqlite3_finalize(pstmt); - } - - DRM_TAPPS_LOG(" Closing DB connection..pDBConnection=[0x%x]", pDBConnection); - - result = db_util_close(pDBConnection); - if(result != SQLITE_OK) - { - DRM_TAPPS_EXCEPTION("SQLite Close fails. errmsg:%s", sqlite3_errmsg(pDBConnection)); - MTHROW_BL - } + DRM_TAPPS_LOG("finalizing all statements..pDBConnection=[0x%x]", pDBConnection); + sqlite3_stmt *pstmt = NULL; + while ((pstmt = sqlite3_next_stmt(pDBConnection, pstmt)) != NULL) { + DRM_TAPPS_LOG("finalizing DB statement..pstmt=[0x%x]", pstmt); + sqlite3_finalize(pstmt); } - DRM_TAPPS_LOG("This fn finishes successfully..pDBConnection=[0x%x]", pDBConnection); - - return TRUE; + BOOL ret = TRUE; -MCATCH_B + DRM_TAPPS_LOG(" Closing DB connection..pDBConnection=[0x%x]", pDBConnection); + if (db_util_close(pDBConnection) != SQLITE_OK) { + DRM_TAPPS_EXCEPTION("db_util_close failed. errmsg:%s", + sqlite3_errmsg(pDBConnection)); + ret = FALSE; + } - DRM_TAPPS_EXCEPTION(" This fn fails.. pDBConnection=[0x%x]", pDBConnection); + // erase map element regardless sqlite handle resource releasement + // because it's not usable anymore. + DRM_TAPPS_LOG("erasing map entry..pDBConnection=[0x%x]", pDBConnection); + g_dtapps_sqlite_connection_table.erase(it); - return FALSE; + return ret; } BOOL DTappsDBBeginImmedTrans (const char* CallingFun) { DRM_TAPPS_LOG("Inside %s, Calling function = %s", __func__, CallingFun); - unsigned int pairCount; dtappsThreadID id_curr_thread = drmgettid(); - sqlite3* pDBConnection = NULL; int count_try_db=0,rc = -1; DRM_TAPPS_SECURE_LOG("DB-OPEN-CLOSE-BEG-COM-RB [%s]Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",__func__,getppid(),getpid(),drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - MTRY_BL(0 != pairCount); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it == g_dtapps_sqlite_connection_table.end()) + return FALSE; - DRM_TAPPS_FRQ_LOG("g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection=[%d], g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection=[0x%x]", __func__, g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection, g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection); + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_FRQ_LOG("countOpenConnection=[%d], DBConnection=[0x%x]", __func__, pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - pDBConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection; + auto pDBConnection = pDBConnectionInfo->pDBConnection; - DRM_TAPPS_LOG("pairCount=[%u], g_dtapps_sqlite_connection_table[connectionIterator]=[0x%x]", pairCount, g_dtapps_sqlite_connection_table[id_curr_thread]); DRM_TAPPS_LOG("Beginning DB operations..pDBConnection=[0x%x]", pDBConnection); - DRM_TAPPS_FRQ_LOG("pDBConnection=[0x%x]",pDBConnection); - - do - { + do { DRM_TAPPS_LOG("START BEGIN"); rc = sqlite3_exec (pDBConnection, DTAPPS_SQLITE3_SQL_BEGIN_IMMEDIATE, NULL, NULL, NULL); @@ -319,27 +289,24 @@ MCATCH_B BOOL DTappsDBCommit(const char* CallingFun) { DRM_TAPPS_LOG("Inside %s, Calling function = %s", __func__,CallingFun); - unsigned int pairCount; dtappsThreadID id_curr_thread = drmgettid(); - sqlite3* pDBConnection = NULL; int count_try_db_commit=0,rc = -1; DRM_TAPPS_SECURE_LOG("DB-OPEN-CLOSE-BEG-COM-RB [%s]Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",__func__,getppid(),getpid(),drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - MTRY_BL(1 == pairCount); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it == g_dtapps_sqlite_connection_table.end()) + return FALSE; - DRM_TAPPS_FRQ_LOG("g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection=[%d], g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection=[0x%x] \n", g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection, g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection); + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_FRQ_LOG("countOpenConnection=[%d], pDBConnection=[0x%x]", pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - pDBConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection; + auto pDBConnection = pDBConnectionInfo->pDBConnection; - DRM_TAPPS_LOG("pairCount=[%u], g_dtapps_sqlite_connection_table[connectionIterator]=[0x%x]", pairCount, g_dtapps_sqlite_connection_table[id_curr_thread]); DRM_TAPPS_LOG("Commiting DB operations..pDBConnection=[0x%x]", pDBConnection); - DRM_TAPPS_FRQ_LOG("pDBConnection=[0x%x]",pDBConnection); - do - { + do { DRM_TAPPS_FRQ_LOG("START Commit"); rc = sqlite3_exec (pDBConnection, DTAPPS_SQLITE3_SQL_COMMIT, NULL, NULL, NULL); DRM_TAPPS_FRQ_LOG("START Commit rc=%d", rc); @@ -386,28 +353,24 @@ BOOL DTappsDBRollback (const char* CallingFun) { DRM_TAPPS_LOG("Inside %s, Calling function = %s", __func__,CallingFun); - unsigned int pairCount; dtappsThreadID id_curr_thread = drmgettid(); - sqlite3* pDBConnection = NULL; int count_try_db_rollback = 0, rc = -1; DRM_TAPPS_SECURE_LOG("DB-OPEN-CLOSE-BEG-COM-RB [%s]Parent Process ID=[%ld]:Process-ID=[%ld]:Thread-ID=[%ld], id_curr_thread=[%ld]",__func__,getppid(),getpid(),drmgettid(), id_curr_thread); - pairCount = g_dtapps_sqlite_connection_table.count(id_curr_thread); - MTRY_BL(1 == pairCount); + auto it = g_dtapps_sqlite_connection_table.find(id_curr_thread); + if (it == g_dtapps_sqlite_connection_table.end()) + return FALSE; - DRM_TAPPS_SECURE_LOG("[%s] g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection=[%d], g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection=[0x%x]", __func__, g_dtapps_sqlite_connection_table[id_curr_thread]->countOpenConnection, g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection); + auto &pDBConnectionInfo = it->second; + DRM_TAPPS_SECURE_LOG("[%s] countOpenConnection=[%d], pDBConnection=[0x%x]", __func__, pDBConnectionInfo->countOpenConnection, pDBConnectionInfo->pDBConnection); - pDBConnection = g_dtapps_sqlite_connection_table[id_curr_thread]->pDBConnection; + auto pDBConnection = pDBConnectionInfo->pDBConnection; - DRM_TAPPS_LOG("pairCount=[%u], g_sqlite_connection_table[connectionIterator]=[0x%x]", __func__, pairCount, g_dtapps_sqlite_connection_table[id_curr_thread]); DRM_TAPPS_LOG("Rollback DB operations..pDBConnection=[0x%x]", pDBConnection); - DRM_TAPPS_FRQ_LOG("[%s] pDBConnection=[0x%x]",pDBConnection); - - do - { + do { DRM_TAPPS_FRQ_LOG("START Rollback"); rc = sqlite3_exec (pDBConnection, DTAPPS_SQLITE3_SQL_ROLLBACK, NULL, NULL, NULL); DRM_TAPPS_FRQ_LOG("START Rollback rc=%d",rc); -- 2.7.4