Fix the my_thread_global_end() spew

It turns out that QSqlDatabase connections are totally not thread-safe.  A
connection must only be used in one thread, from the point where it's opened
and added right through until where it's closed and removed.  Our old code
depended solely on the ~MythContext dtor to close all the database connections,
so any connection not opened in the UI thread confused the Qt code, and mysql's
API waited for the wrong threads at shutdown.  This caused a 5s delay and an
ugly message like:

Error in my_thread_global_end(): 1 threads didn't exit

To fix this, the database logging thread now has its own database connection,
and threads that open connections are now explicitly closing them.

Additionally, the WriteDelayedSettings() code is forced to only run in the UI
thread.  Ideally, we need a centralized DB thread that handles all of the
connections, but that's a fairly large change, and can wait a bit longer

mythtv/libs/libmythbase/dbutil.cpp

@@ -837,34 +837,6 @@ int DBUtil::CountClients(void)
     return count;
 }
 
-/**
- *  \brief Checks whether table exists
- *
- *  \param  table  The name of the table to check (without schema name)
- *  \return true if table exists in schema or false if not
- */
-bool DBUtil::TableExists(const QString &table)
-{
-    bool result = false;
-    MSqlQuery query(MSqlQuery::InitCon());
-    if (query.isConnected())
-    {
-        QString sql = "SELECT INFORMATION_SCHEMA.TABLES.TABLE_NAME "
-                      "  FROM INFORMATION_SCHEMA.TABLES "
-                      " WHERE INFORMATION_SCHEMA.TABLES.TABLE_SCHEMA = "
-                      "       DATABASE() "
-                      "   AND INFORMATION_SCHEMA.TABLES.TABLE_NAME = "
-                      "       :TABLENAME ;";
-        if (query.prepare(sql))
-        {
-            query.bindValue(":TABLENAME", table);
-            if (query.exec() && query.next())
-                result = true;
-        }
-    }
-    return result;
-}
-
 /**
  * \brief Try to get a lock on the table schemalock.
  *

mythtv/libs/libmythbase/dbutil.h

@@ -44,7 +44,6 @@ class MBASE_PUBLIC DBUtil
     static bool IsNewDatabase(void);
     static bool IsBackupInProgress(void);
     static int  CountClients(void);
-    static bool TableExists(const QString &table);
 
     static bool lockSchema(MSqlQuery &);
     static void unlockSchema(MSqlQuery &);

mythtv/libs/libmythbase/mythdb.cpp

@@ -7,12 +7,14 @@ using namespace std;
 #include <QFile>
 #include <QHash>
 #include <QDir>
+#include <QThread>
 
 #include "mythdb.h"
 #include "mythdbcon.h"
 #include "mythverbose.h"
 #include "oldsettings.h"
 #include "mythdirs.h"
+#include "mythcorecontext.h"
 
 static MythDB *mythdb = NULL;
 static QMutex dbLock;
@@ -1026,6 +1028,9 @@ void MythDB::WriteDelayedSettings(void)
     if (!HaveValidDatabase())
         return;
 
+    if (!gCoreContext->IsUIThread())
+	return;
+
     while (!d->delayedSettings.isEmpty())
     {
         SingleSetting setting = d->delayedSettings.takeFirst();

mythtv/libs/libmythbase/mythdbcon.cpp

@@ -61,7 +61,9 @@ bool TestDatabase(QString dbHostName,
 MSqlDatabase::MSqlDatabase(const QString &name)
 {
     m_name = name;
+    m_name.detach();
     m_db = QSqlDatabase::addDatabase("QMYSQL", m_name);
+    VERBOSE(VB_IMPORTANT, "Database connection created: " + m_name);
 
     if (!m_db.isValid())
     {
@@ -81,6 +83,7 @@ MSqlDatabase::~MSqlDatabase()
                                 // removeDatabase() so that connections
                                 // and queries are cleaned up correctly
         QSqlDatabase::removeDatabase(m_name);
+        VERBOSE(VB_IMPORTANT, "Database connection deleted: " + m_name);
     }
 }
 
@@ -276,6 +279,7 @@ MDBManager::MDBManager()
 
     m_schedCon = NULL;
     m_DDCon = NULL;
+    m_LogCon = NULL;
 }
 
 MDBManager::~MDBManager()
@@ -284,8 +288,9 @@ MDBManager::~MDBManager()
         delete m_pool.takeFirst();
     delete m_sem;
 
-    delete m_schedCon;
-    delete m_DDCon;
+    closeStaticCon(&m_schedCon);
+    closeStaticCon(&m_DDCon);
+    closeStaticCon(&m_LogCon);
 }
 
 MSqlDatabase *MDBManager::popConnection()
@@ -392,30 +397,59 @@ void MDBManager::PurgeIdleConnections(void)
     }
 }
 
-MSqlDatabase *MDBManager::getSchedCon()
+MSqlDatabase *MDBManager::getStaticCon(MSqlDatabase **dbcon, QString name)
 {
-    if (!m_schedCon)
+    if (!dbcon)
+        return NULL;
+
+    if (!*dbcon)
     {
-        m_schedCon = new MSqlDatabase("SchedCon");
-        VERBOSE(VB_IMPORTANT, "New DB scheduler connection");
+        *dbcon = new MSqlDatabase(name);
+        VERBOSE(VB_IMPORTANT, "New static DB connection" + name);
     }
 
-    m_schedCon->OpenDatabase();
+    (*dbcon)->OpenDatabase();
+
+    return *dbcon;
+}
 
-    return m_schedCon;
+MSqlDatabase *MDBManager::getSchedCon()
+{
+    return getStaticCon(&m_schedCon, "SchedCon");
 }
 
 MSqlDatabase *MDBManager::getDDCon()
 {
-    if (!m_DDCon)
+    return getStaticCon(&m_DDCon, "DataDirectCon");
+}
+
+MSqlDatabase *MDBManager::getLogCon()
+{
+    return getStaticCon(&m_LogCon, "LogCon");
+}
+
+void MDBManager::closeStaticCon(MSqlDatabase **dbcon)
+{
+    if (dbcon && *dbcon)
     {
-        m_DDCon = new MSqlDatabase("DataDirectCon");
-        VERBOSE(VB_IMPORTANT, "New DB DataDirect connection");
+        delete *dbcon;
+        *dbcon = NULL;
     }
+}
 
-    m_DDCon->OpenDatabase();
+void MDBManager::closeSchedCon()
+{
+    closeStaticCon(&m_schedCon);
+}
 
-    return m_DDCon;
+void MDBManager::closeDDCon()
+{
+    closeStaticCon(&m_DDCon);
+}
+
+void MDBManager::closeLogCon()
+{
+    closeStaticCon(&m_LogCon);
 }
 
 // Dangerous. Should only be used when the database connection has errored?
@@ -547,6 +581,41 @@ MSqlQueryInfo MSqlQuery::DDCon()
     return qi;
 }
 
+MSqlQueryInfo MSqlQuery::LogCon()
+{
+    MSqlDatabase *db = GetMythDB()->GetDBManager()->getLogCon();
+    MSqlQueryInfo qi;
+
+    InitMSqlQueryInfo(qi);
+    qi.returnConnection = false;
+
+    if (db)
+    {
+        qi.db = db;
+        qi.qsqldb = db->db();
+
+        db->KickDatabase();
+    }
+
+    return qi;
+}
+
+void MSqlQuery::CloseSchedCon()
+{
+    GetMythDB()->GetDBManager()->closeSchedCon();
+}
+
+void MSqlQuery::CloseDDCon()
+{
+    GetMythDB()->GetDBManager()->closeDDCon();
+}
+
+void MSqlQuery::CloseLogCon()
+{
+    GetMythDB()->GetDBManager()->closeLogCon();
+}
+
+
 bool MSqlQuery::exec()
 {
     // Database connection down.  Try to restart it, give up if it's still

mythtv/libs/libmythbase/mythdbcon.h

@@ -63,8 +63,15 @@ class MBASE_PUBLIC MDBManager
 
     MSqlDatabase *getSchedCon(void);
     MSqlDatabase *getDDCon(void);
+    MSqlDatabase *getLogCon(void);
+    void closeSchedCon(void);
+    void closeDDCon(void);
+    void closeLogCon(void);
 
   private:
+    MSqlDatabase *getStaticCon(MSqlDatabase **dbcon, QString name);
+    void closeStaticCon(MSqlDatabase **dbcon);
+
     QList<MSqlDatabase*> m_pool;
     QMutex m_lock;
     QSemaphore *m_sem;
@@ -73,6 +80,7 @@ class MBASE_PUBLIC MDBManager
 
     MSqlDatabase *m_schedCon;
     MSqlDatabase *m_DDCon;
+    MSqlDatabase *m_LogCon;
 };
 
 /// \brief MSqlDatabase Info, used by MSqlQuery. Do not use directly.
@@ -164,6 +172,13 @@ class MBASE_PUBLIC MSqlQuery : public QSqlQuery
     /// \brief Returns dedicated connection. (Required for using temporary SQL tables.)
     static MSqlQueryInfo DDCon();
 
+    /// \brief Returns dedicated connection.
+    static MSqlQueryInfo LogCon();
+
+    static void CloseSchedCon();
+    static void CloseDDCon();
+    static void CloseLogCon();
+
   private:
     MSqlDatabase *m_db;
     bool m_isConnected;

mythtv/libs/libmythbase/mythlogging.cpp

@@ -52,6 +52,7 @@ QMutex                   logThreadTidMutex;
 QHash<uint64_t, int64_t> logThreadTidHash;
 
 LoggerThread            logThread;
+bool                    logThreadFinished = false;
 bool                    debugRegistration = false;
 
 #define TIMESTAMP_MAX 30
@@ -314,7 +315,7 @@ bool DatabaseLogger::logmsg(LoggingItem_t *item)
 bool DatabaseLogger::logqmsg(LoggingItem_t *item)
 {
     char        timestamp[TIMESTAMP_MAX];
-    char       *threadName = getThreadName(item);;
+    char       *threadName = getThreadName(item);
 
     if( !isDatabaseReady() )
         return false;
@@ -326,7 +327,7 @@ bool DatabaseLogger::logqmsg(LoggingItem_t *item)
         m_host = strdup((char *)gCoreContext->GetHostName()
                         .toLocal8Bit().constData());
 
-    MSqlQuery   query(MSqlQuery::InitCon());
+    MSqlQuery   query(MSqlQuery::LogCon());
     query.prepare( m_query );
     query.bindValue(":HOST",        m_host);
     query.bindValue(":APPLICATION", m_application);
@@ -384,6 +385,8 @@ void DBLoggerThread::run(void)
 
         qLock.relock();
     }
+    MSqlQuery::CloseLogCon();
+    threadDeregister();
 }
 
 bool DatabaseLogger::isDatabaseReady()
@@ -392,14 +395,44 @@ bool DatabaseLogger::isDatabaseReady()
     MythDB *db;
 
     if ( !m_loggingTableExists )
-        m_loggingTableExists = DBUtil::TableExists(m_handle.string);
+        m_loggingTableExists = tableExists(m_handle.string);
 
     if ( m_loggingTableExists && (db = GetMythDB()) && db->HaveValidDatabase() )
         ready = true;
 
     return ready;
 }
 
+/**
+ *  \brief Checks whether table exists
+ *
+ *  \param  table  The name of the table to check (without schema name)
+ *  \return true if table exists in schema or false if not
+ */
+bool DatabaseLogger::tableExists(const QString &table)
+{
+    bool result = false;
+    MSqlQuery query(MSqlQuery::LogCon());
+    if (query.isConnected())
+    {
+        QString sql = "SELECT INFORMATION_SCHEMA.TABLES.TABLE_NAME "
+                      "  FROM INFORMATION_SCHEMA.TABLES "
+                      " WHERE INFORMATION_SCHEMA.TABLES.TABLE_SCHEMA = "
+                      "       DATABASE() "
+                      "   AND INFORMATION_SCHEMA.TABLES.TABLE_NAME = "
+                      "       :TABLENAME ;";
+        if (query.prepare(sql))
+        {
+            query.bindValue(":TABLENAME", table);
+            if (query.exec() && query.next())
+                result = true;
+        }
+    }
+    return result;
+}
+
+
+
 char *getThreadName( LoggingItem_t *item )
 {
     static const char  *unknown = "thread_unknown";
@@ -488,6 +521,7 @@ void LoggerThread::run(void)
     threadRegister("Logger");
     LoggingItem_t *item;
 
+    logThreadFinished = false;
     aborted = false;
 
     QMutexLocker qLock(&logQueueMutex);
@@ -580,6 +614,8 @@ void LoggerThread::run(void)
 
         qLock.relock();
     }
+
+    logThreadFinished = true;
 }
 
 void deleteItem( LoggingItem_t *item )
@@ -756,6 +792,9 @@ void threadRegister(QString name)
     if (!item)
         return;
 
+    if (logThreadFinished)
+        return;
+
     memset( item, 0, sizeof(LoggingItem_t) );
     LogTimeStamp( &epoch, &usec );
 
@@ -786,6 +825,9 @@ void threadDeregister(void)
     if (!item)
         return;
 
+    if (logThreadFinished)
+        return;
+
     memset( item, 0, sizeof(LoggingItem_t) );
     LogTimeStamp( &epoch, &usec );
 

mythtv/libs/libmythbase/mythlogging.h

@@ -165,6 +165,7 @@ class DatabaseLogger : public LoggerBase {
         bool logqmsg(LoggingItem_t *item);
     private:
         bool isDatabaseReady();
+        bool tableExists(const QString &table);
 
         DBLoggerThread *m_thread;
         char *m_host;