Page 1 of 1

QxSqlDatabase::clearAllDatabases()

PostPosted: Wed Jun 14, 2017 12:21 pm
by mdw
Wouldn't it be better to call QSqlDatabase::removeDatabase() on each database key before clearing the hash map? I mean to change clearAllDatabases() like this:

Code: Select all
void QxSqlDatabase::clearAllDatabases()
{
   qx::QxSqlDatabase::closeAllDatabases();
   qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
   if (! pSingleton) { qAssert(false); return; }
   Q_FOREACH(QString sDbKey, pSingleton->m_lstDbByThread)
   { QSqlDatabase::removeDatabase(sDbKey); }

   pSingleton->m_lstDbByThread.clear();
   pSingleton->m_sDriverName = "";
   pSingleton->m_sConnectOptions = "";
   pSingleton->m_sDatabaseName = "";
   pSingleton->m_sUserName = "";
   pSingleton->m_sPassword = "";
   pSingleton->m_sHostName = "";
   pSingleton->m_iPort = -1;
}

Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Wed Jun 14, 2017 2:34 pm
by mdw
Okay, I have reworked the patch a bit further.
  • The assertions after getSingleton() are not needed, since C++'s new operator always returns a valid instance, otherwise an exception (compare with QxSqlDatabase::getDatabase(QSqlError & dbError))
  • Fixed some missing mutexes
  • I need some way to tell if a connection pool is in use or not, for now I called the routine isEmpty() (I didn't want to call it isClosed() to not cause confusion with closeAllDatabases())

Code: Select all
diff --git a/QxSqlDatabase.h b/QxSqlDatabase.h.new
index 52824f2..4ed0b2d 100644
--- a/QxSqlDatabase.h
+++ b/QxSqlDatabase.h.new
@@ -162,6 +162,7 @@ public:
    static QSqlDatabase getDatabaseCloned();
    static void closeAllDatabases();
    static void clearAllDatabases();
+   static bool isEmpty();
 
    qx::dao::detail::IxSqlGenerator * getSqlGenerator();
 


Code: Select all
diff --git a/QxSqlDatabase.cpp b/QxSqlDatabase.cpp.new
index c3b4a7d..e217d07 100644
--- a/QxSqlDatabase.cpp
+++ b/QxSqlDatabase.cpp.new
@@ -169,7 +169,8 @@ qx::dao::detail::IxSqlGenerator * QxSqlDatabase::getSqlGenerator()
 void QxSqlDatabase::closeAllDatabases()
 {
    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
-   if (! pSingleton) { qAssert(false); return; }
+   QMutexLocker locker(& pSingleton->m_oDbMutex);
+
    Q_FOREACH(QString sDbKey, pSingleton->m_lstDbByThread)
    { QSqlDatabase::database(sDbKey).close(); }
 }
@@ -177,8 +178,13 @@ void QxSqlDatabase::closeAllDatabases()
 void QxSqlDatabase::clearAllDatabases()
 {
    qx::QxSqlDatabase::closeAllDatabases();
+
    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
-   if (! pSingleton) { qAssert(false); return; }
+   QMutexLocker locker(& pSingleton->m_oDbMutex);
+
+   Q_FOREACH(QString sDbKey, pSingleton->m_lstDbByThread)
+   { QSqlDatabase::removeDatabase(sDbKey); }
+
    pSingleton->m_lstDbByThread.clear();
    pSingleton->m_sDriverName = "";
    pSingleton->m_sConnectOptions = "";
@@ -189,4 +195,12 @@ void QxSqlDatabase::clearAllDatabases()
    pSingleton->m_iPort = -1;
 }
 
+bool QxSqlDatabase::isEmpty()
+{
+    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
+    QMutexLocker locker(& pSingleton->m_oDbMutex);
+
+    return pSingleton->m_lstDbByThread.isEmpty();
+}
+
 } // namespace qx


Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Wed Jun 21, 2017 8:11 am
by qxorm
Thx for your patch !

Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Wed Jun 21, 2017 9:47 am
by mdw
Could you please make me a pre-release available, so that I can fix the final nomenclature of the functions?

Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Wed Feb 14, 2018 4:51 pm
by mdw
Update to this issue: Unfortunately I found a problem with the patch which I have posted half a year ago. Please find attached the latest version which I encourage you to include in the upcoming QxORM releases:

  • I need a recursive mutex since clearAllDatabases() is based on closeAllDatabases() and I want to lock immediately to have the whole clear-operation atomic (once or wait). This requires the recursive mutex so that the mutex locker in closeAllDatabases() does not block on the same thread (in Java this is already the default, in Qt unfortunately not).
  • closeAllDatabases() previously did not delete the database references (no call to QSqlDatabase::removeDatabase(sDbKey);). This was wrong when re-opening the pool, since there old connections could have been reused which was not the intention (the threads could have been changed, but the hash code remained the same which caused invalid connection objects to be returned). Now the patch performs a full-cleanup and even empties m_lstDbByThread.

QxSqlDatabase.h
Code: Select all
diff --git a/include/QxDao/QxSqlDatabase.h.old b/include/QxDao/QxSqlDatabase.h
index 52824f2..c94cd04 100644
--- a/include/QxDao/QxSqlDatabase.h.old
+++ b/include/QxDao/QxSqlDatabase.h
@@ -57,6 +57,7 @@
 #include <QxDao/QxSqlGenerator/IxSqlGenerator.h>
 
 #define QX_CONSTRUCT_QX_SQL_DATABASE() \
+m_oDbMutex(QMutex::Recursive), \
 m_iPort(-1), m_bTraceSqlQuery(true), m_bTraceSqlRecord(false), \
 m_bTraceSqlBoundValues(false), m_bTraceSqlBoundValuesOnError(true), \
 m_ePlaceHolderStyle(ph_style_2_point_name), m_bSessionThrowable(false), \
@@ -162,6 +163,7 @@ public:
    static QSqlDatabase getDatabaseCloned();
    static void closeAllDatabases();
    static void clearAllDatabases();
+   static bool isEmpty();
 
    qx::dao::detail::IxSqlGenerator * getSqlGenerator();
 


QxSqlDatabase.cpp
Code: Select all
diff --git a/src/QxDao/QxSqlDatabase.cpp.old b/src/QxDao/QxSqlDatabase.cpp
index c3b4a7d..8d444dc 100644
--- a/src/QxDao/QxSqlDatabase.cpp.old
+++ b/src/QxDao/QxSqlDatabase.cpp
@@ -169,17 +169,23 @@ qx::dao::detail::IxSqlGenerator * QxSqlDatabase::getSqlGenerator()
 void QxSqlDatabase::closeAllDatabases()
 {
    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
-   if (! pSingleton) { qAssert(false); return; }
+   QMutexLocker locker(& pSingleton->m_oDbMutex);
+
    Q_FOREACH(QString sDbKey, pSingleton->m_lstDbByThread)
-   { QSqlDatabase::database(sDbKey).close(); }
+   {
+       QSqlDatabase::database(sDbKey).close();
+       QSqlDatabase::removeDatabase(sDbKey);
+   }
+   pSingleton->m_lstDbByThread.clear();
 }
 
 void QxSqlDatabase::clearAllDatabases()
 {
-   qx::QxSqlDatabase::closeAllDatabases();
    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
-   if (! pSingleton) { qAssert(false); return; }
-   pSingleton->m_lstDbByThread.clear();
+   QMutexLocker locker(& pSingleton->m_oDbMutex);
+
+   pSingleton->closeAllDatabases();
+
    pSingleton->m_sDriverName = "";
    pSingleton->m_sConnectOptions = "";
    pSingleton->m_sDatabaseName = "";
@@ -189,4 +195,12 @@ void QxSqlDatabase::clearAllDatabases()
    pSingleton->m_iPort = -1;
 }
 
+bool QxSqlDatabase::isEmpty()
+{
+    qx::QxSqlDatabase * pSingleton = qx::QxSqlDatabase::getSingleton();
+    QMutexLocker locker(& pSingleton->m_oDbMutex);
+
+    return pSingleton->m_lstDbByThread.isEmpty();
+}
+
 } // namespace qx


Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Mon Mar 05, 2018 11:18 am
by qxorm
Hello,
Here is a version which includes your patch : https://www.qxorm.com/version/QxOrm_1.4.5_BETA_04.zip

Re: QxSqlDatabase::clearAllDatabases()

PostPosted: Thu Mar 22, 2018 8:22 am
by mdw
Yes fine, this should work for us :) . I am looking forward for the final 1.4.5 release.