diff --git a/src/blackcore/application.cpp b/src/blackcore/application.cpp index a4c78b4b4..818626147 100644 --- a/src/blackcore/application.cpp +++ b/src/blackcore/application.cpp @@ -118,6 +118,7 @@ namespace BlackCore QCoreApplication::setApplicationName(m_applicationName); QCoreApplication::setApplicationVersion(CBuildConfig::getVersionString()); 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 if (init) diff --git a/src/blackmisc/threadutils.cpp b/src/blackmisc/threadutils.cpp index e5fc31c2a..1e902154f 100644 --- a/src/blackmisc/threadutils.cpp +++ b/src/blackmisc/threadutils.cpp @@ -31,6 +31,12 @@ namespace BlackMisc 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() { if (!QCoreApplication::instance()) { return false; } @@ -68,11 +74,15 @@ namespace BlackMisc return s.arg(reinterpret_cast(t), 0, 16); } - const QString CThreadUtils::currentThreadInfo() + const QString CThreadUtils::threadInfo(QThread *thread) { static const QString info("thread: %1 name: '%2' priority: '%3'"); - const QThread *t = QThread::currentThread(); - if (!t) { return QString("no thread"); } - return info.arg(threadToString(t), t->objectName(), priorityToString(t->priority())); + if (!thread) { return QString("no thread"); } + return info.arg(threadToString(thread), thread->objectName(), priorityToString(thread->priority())); + } + + const QString CThreadUtils::currentThreadInfo() + { + return threadInfo(QThread::currentThread()); } } // ns diff --git a/src/blackmisc/threadutils.h b/src/blackmisc/threadutils.h index 9857ba141..dcb04441a 100644 --- a/src/blackmisc/threadutils.h +++ b/src/blackmisc/threadutils.h @@ -34,6 +34,10 @@ namespace BlackMisc //! \remarks can be used as ASSERT check for threaded objects 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? //! \remarks can be used as ASSERT check for threaded objects static bool isCurrentThreadApplicationThread(); @@ -44,6 +48,9 @@ namespace BlackMisc //! Thread to int string info static const QString threadToString(const void *t); + //! Info about current thread + static const QString threadInfo(QThread *thread); + //! Info about current thread static const QString currentThreadInfo(); }; diff --git a/src/blackmisc/worker.cpp b/src/blackmisc/worker.cpp index c820e0195..c35a1ea62 100644 --- a/src/blackmisc/worker.cpp +++ b/src/blackmisc/worker.cpp @@ -14,9 +14,10 @@ #include #include #include +#include #ifdef Q_OS_WIN32 -#include +#include #endif namespace BlackMisc @@ -149,7 +150,7 @@ namespace BlackMisc if (m_owner) { 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); @@ -175,12 +176,12 @@ namespace BlackMisc { this->quit(); - // already in different thread? otherwise return + // already in application (main) thread? => 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; } - auto *ownThread = thread(); + QThread *ownThread = thread(); 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 @@ -225,24 +226,49 @@ namespace BlackMisc { this->setFinished(); - QThread *ownThread = thread(); - 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 pOwnThread(ownThread); + QPointer pOwnThreadBeforeMove(this->thread()); // thread before moved back QPointer myself(this); - QTimer::singleShot(0, ownThread, [ = ] + + if (!m_owner || (pOwnThreadBeforeMove.data() != m_owner->thread())) { - if (!pOwnThread) { return; } - pOwnThread->deleteLater(); - }); - QTimer::singleShot(0, this, [ = ] + if (m_owner) + { + // we have a owner, and the owner thread is not the same as the worker thread + // 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; } - myself->deleteLater(); - }); + // getting here would mean we have a owner and the owner thread is the same as the work thread + 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 diff --git a/src/blackmisc/worker.h b/src/blackmisc/worker.h index 0b2ad7a10..845f7dfec 100644 --- a/src/blackmisc/worker.h +++ b/src/blackmisc/worker.h @@ -334,7 +334,7 @@ namespace BlackMisc using CWorkerBase::setStarted; using CWorkerBase::setFinished; - QObject *m_owner = nullptr; + QPointer m_owner; //!< owner, QPointer will detect if the owner is deleted QString m_name; //!< worker's name std::atomic m_enabled { true }; //!< marker it is enabled };