Ref T314, thread CContinuousWorker::finish

- use application thread if there is no owner
- use QPointer for owner
- more detailled error message
This commit is contained in:
Klaus Basan
2018-08-30 01:17:14 +02:00
parent f6dee9e7ff
commit 5096df034b
5 changed files with 70 additions and 26 deletions

View File

@@ -118,6 +118,7 @@ namespace BlackCore
QCoreApplication::setApplicationName(m_applicationName); QCoreApplication::setApplicationName(m_applicationName);
QCoreApplication::setApplicationVersion(CBuildConfig::getVersionString()); QCoreApplication::setApplicationVersion(CBuildConfig::getVersionString());
this->setObjectName(m_applicationName); this->setObjectName(m_applicationName);
this->thread()->setObjectName(m_applicationName); // normally no effect as thread already runs, but does not harm either
// init skipped when called from CGuiApplication // init skipped when called from CGuiApplication
if (init) if (init)

View File

@@ -31,6 +31,12 @@ namespace BlackMisc
return (QCoreApplication::instance()->thread() == toBeTested->thread()); return (QCoreApplication::instance()->thread() == toBeTested->thread());
} }
bool CThreadUtils::isApplicationThread(const QThread *toBeTested)
{
if (!toBeTested || !QCoreApplication::instance() || !QCoreApplication::instance()->thread()) { return false; }
return (QCoreApplication::instance()->thread() == toBeTested);
}
bool CThreadUtils::isCurrentThreadApplicationThread() bool CThreadUtils::isCurrentThreadApplicationThread()
{ {
if (!QCoreApplication::instance()) { return false; } if (!QCoreApplication::instance()) { return false; }
@@ -68,11 +74,15 @@ namespace BlackMisc
return s.arg(reinterpret_cast<long long>(t), 0, 16); return s.arg(reinterpret_cast<long long>(t), 0, 16);
} }
const QString CThreadUtils::currentThreadInfo() const QString CThreadUtils::threadInfo(QThread *thread)
{ {
static const QString info("thread: %1 name: '%2' priority: '%3'"); static const QString info("thread: %1 name: '%2' priority: '%3'");
const QThread *t = QThread::currentThread(); if (!thread) { return QString("no thread"); }
if (!t) { return QString("no thread"); } return info.arg(threadToString(thread), thread->objectName(), priorityToString(thread->priority()));
return info.arg(threadToString(t), t->objectName(), priorityToString(t->priority())); }
const QString CThreadUtils::currentThreadInfo()
{
return threadInfo(QThread::currentThread());
} }
} // ns } // ns

View File

@@ -34,6 +34,10 @@ namespace BlackMisc
//! \remarks can be used as ASSERT check for threaded objects //! \remarks can be used as ASSERT check for threaded objects
static bool isApplicationThreadObjectThread(const QObject *toBeTested); static bool isApplicationThreadObjectThread(const QObject *toBeTested);
//! Is the application thread the QObject's thread?
//! \remarks can be used as ASSERT check for threaded objects
static bool isApplicationThread(const QThread *toBeTested);
//! Is the current thread the Application thread? //! Is the current thread the Application thread?
//! \remarks can be used as ASSERT check for threaded objects //! \remarks can be used as ASSERT check for threaded objects
static bool isCurrentThreadApplicationThread(); static bool isCurrentThreadApplicationThread();
@@ -44,6 +48,9 @@ namespace BlackMisc
//! Thread to int string info //! Thread to int string info
static const QString threadToString(const void *t); static const QString threadToString(const void *t);
//! Info about current thread
static const QString threadInfo(QThread *thread);
//! Info about current thread //! Info about current thread
static const QString currentThreadInfo(); static const QString currentThreadInfo();
}; };

View File

@@ -14,9 +14,10 @@
#include <future> #include <future>
#include <QTimer> #include <QTimer>
#include <QPointer> #include <QPointer>
#include <QCoreApplication>
#ifdef Q_OS_WIN32 #ifdef Q_OS_WIN32
#include <windows.h> #include <Windows.h>
#endif #endif
namespace BlackMisc namespace BlackMisc
@@ -149,7 +150,7 @@ namespace BlackMisc
if (m_owner) if (m_owner)
{ {
const QString ownerName = m_owner->objectName().isEmpty() ? m_owner->metaObject()->className() : m_owner->objectName(); const QString ownerName = m_owner->objectName().isEmpty() ? m_owner->metaObject()->className() : m_owner->objectName();
thread->setObjectName(ownerName + ":" + m_name); thread->setObjectName(ownerName + ": " + m_name);
} }
moveToThread(thread); moveToThread(thread);
@@ -175,12 +176,12 @@ namespace BlackMisc
{ {
this->quit(); this->quit();
// already in different thread? otherwise return // already in application (main) thread? => return
if (CThreadUtils::isApplicationThreadObjectThread(this)) { return; } if (CThreadUtils::isApplicationThreadObjectThread(this)) { return; }
// Called by own thread, will deadlock, return // called by own thread, will deadlock, return
if (CThreadUtils::isCurrentThreadObjectThread(this)) { return; } if (CThreadUtils::isCurrentThreadObjectThread(this)) { return; }
auto *ownThread = thread(); QThread *ownThread = thread();
const qint64 beforeWait = QDateTime::currentMSecsSinceEpoch(); const qint64 beforeWait = QDateTime::currentMSecsSinceEpoch();
ownThread->wait(30 * 1000); //! \todo KB 2017-10 temp workaround: in T145 this will be fixed, sometimes (very rarely) hanging here during shutdown ownThread->wait(30 * 1000); //! \todo KB 2017-10 temp workaround: in T145 this will be fixed, sometimes (very rarely) hanging here during shutdown
@@ -225,24 +226,49 @@ namespace BlackMisc
{ {
this->setFinished(); this->setFinished();
QThread *ownThread = thread(); QPointer<QThread> pOwnThreadBeforeMove(this->thread()); // thread before moved back
if (!ownThread) { return; } // normally never happening
moveToThread(ownThread->thread()); // move worker back to the thread which constructed it, so there is no race on deletion
// very sporadic crash here, changing to QSingleShot version for testing
// QMetaObject::invokeMethod(ownThread, "deleteLater");
// QMetaObject::invokeMethod(this, "deleteLater");
QPointer<QThread> pOwnThread(ownThread);
QPointer<CContinuousWorker> myself(this); QPointer<CContinuousWorker> myself(this);
QTimer::singleShot(0, ownThread, [ = ]
if (!m_owner || (pOwnThreadBeforeMove.data() != m_owner->thread()))
{ {
if (!pOwnThread) { return; } if (m_owner)
pOwnThread->deleteLater(); {
}); // we have a owner, and the owner thread is not the same as the worker thread
QTimer::singleShot(0, this, [ = ] // move worker back to the thread which constructed it, so there is no race on deletion
this->moveToThread(m_owner->thread());
Q_ASSERT_X(this->thread() == m_owner->thread(), Q_FUNC_INFO, "Thread mismatch owner");
}
else if (qApp && qApp->thread())
{
// no owner, we move to main thread
this->moveToThread(qApp->thread());
Q_ASSERT_X(this->thread() == qApp->thread(), Q_FUNC_INFO, "Thread mismatch qApp");
}
// if thread still exists and is not the main thread, delete it
if (pOwnThreadBeforeMove && !CThreadUtils::isApplicationThread(pOwnThreadBeforeMove.data()))
{
// delete the original "background" thread
QTimer::singleShot(0, pOwnThreadBeforeMove.data(), [ = ]
{
if (pOwnThreadBeforeMove) { pOwnThreadBeforeMove->deleteLater(); }
});
}
}
else
{ {
if (!myself) { return; } // getting here would mean we have a owner and the owner thread is the same as the work thread
myself->deleteLater(); const QString msg = QString("Owner thread: '%1' own thread: '%2'").arg(CThreadUtils::threadInfo(m_owner->thread()), CThreadUtils::threadInfo(pOwnThreadBeforeMove.data()));
}); BLACK_VERIFY_X(false, Q_FUNC_INFO, msg.toLatin1().constData());
}
if (myself)
{
// if worker still exists delete it
QTimer::singleShot(0, myself.data(), [ = ]
{
if (myself) { myself->deleteLater(); }
});
}
} }
} // ns } // ns

View File

@@ -334,7 +334,7 @@ namespace BlackMisc
using CWorkerBase::setStarted; using CWorkerBase::setStarted;
using CWorkerBase::setFinished; using CWorkerBase::setFinished;
QObject *m_owner = nullptr; QPointer<QObject> m_owner; //!< owner, QPointer will detect if the owner is deleted
QString m_name; //!< worker's name QString m_name; //!< worker's name
std::atomic<bool> m_enabled { true }; //!< marker it is enabled std::atomic<bool> m_enabled { true }; //!< marker it is enabled
}; };