QxSqlDatabase::clearAllDatabases()

[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;
}


[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



[qxorm]

Thx for your patch !

[mdw]

Could you please make me a pre-release available, so that I can fix the final nomenclature of the functions?

[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



[qxorm]

Hello,
Here is a version which includes your patch : https://www.qxorm.com/version/QxOrm_1.4.5_BETA_04.zip

[mdw]

Yes fine, this should work for us . I am looking forward for the final 1.4.5 release.