QBasicTimer::start: Timers cannot be started from another thread issue

[jsulm]

@Cobra91151 Why do you need an event loop? Why not just use Qt in the way it was designed: asynchronously?

Do you start any threads?

[Cobra91151]

@jsulm

Yes. The work starts in the thread with connected signals and slots. I use QEventLoop to maintain all check work in the checkDBUpdate() slot.

[jsulm]

@Cobra91151 Can you show where and how you start the thread?

[Cobra91151]

@jsulm

Ok.

Code:

void AppSettings::checkDatabaseUpdate()
{
    QThread *dbUpdateThread = new QThread();
    Test *dbUpdate = new Test();
    dbUpdate->moveToThread(dbUpdateThread);
    connect(dbUpdateThread, &QThread::started, dbUpdate, &Test::checkDBUpdate);
    connect(dbUpdate, &Test::dbUpdateAvailable, this, &AppSettings::setDBUpdate);
    connect(this, &AppSettings::dbUpdateConfirmed, dbUpdate, &Test::updateDatabase);
    connect(dbUpdate, &Test::updateDBCompleted, this, &AppSettings::setDBUpdated);
    connect(dbUpdate, &Test::dbLatest, this, &AppSettings::setDBLatest);
    connect(this, &AppSettings::dbUpdateCanceled, dbUpdate, &Test::setUpdateCanceled);
    connect(dbUpdate, &Test::updateDBFinished, dbUpdateThread, &QThread::quit);
    connect(dbUpdate, &Test::updateDBFinished, dbUpdate, &Test::deleteLater);
    connect(dbUpdateThread, &QThread::finished, dbUpdateThread, &QThread::deleteLater);
    dbUpdateThread->start();
}

[jsulm]

@Cobra91151 Is netManager initialized in Test constructor? If so try to do it in Test::checkDBUpdate().
I do not see any reason to start a thread in AppSettings::checkDatabaseUpdate() which executes a blocking method. Just make Test::checkDBUpdate() non blocking. You really over complicate things.

[Cobra91151]

@jsulm

Yes, netManager (QNetworkAccessManager) is initialized in the Test constructor. I have initialized it in the constructor to access not only for the Test::checkDBUpdate(), but also for the other features which require QNetworkAccessManager. So I can't initialized it twice or in Test::checkDBUpdate().

AppSettings class is a GUI window and I connect it there to get the result of the work. It's very big and complicated application.

What do you mean by blocking method?

[jsulm]

@Cobra91151 I mean void Test::checkDBUpdate() is blocking (not returning) until it is finished (that's why you use event loop there, right?). So, you start a thread to execute a blocking method to not block main thread instead of simply making void Test::checkDBUpdate() non blocking. You introduce a complex and error prone work-around (thread) for something you could simply change. Also: why do you use new so often (for example there is no need to allocate dbFile on the heap)? Why not just allocate memory on the stack?

Back to this timer issue: you create netManager in constructor and then later move Test to another thread, I guess netManager is still in the main thread.

[jsulm]

@Cobra91151 Here a non-blocking solution without any threads (untested!):

void Test::checkDBUpdate()
{
    QFile dbFile(QString(DB_DIR) + QString(DB_FILE));
    dbFile.open(QIODevice::ReadOnly);
    QTextStream dbTextStream(dbFile);
    QString localDBFile = dbTextStream.readAll();
    QNetworkReply *checkDBReply = netManager->get(QNetworkRequest(dbFtpUrl()));
    connect(checkDBReply, &QNetworkReply::finished, [checkDBReply, localDBFile]() {
        if (checkDBReply->bytesAvailable()) {
            QString ftpDBFile = checkDBReply->readAll();
            qDebug() << ftpDBFile.length();

            if (localDBFile.length() != ftpDBFile.length()) {
                emit dbUpdateAvailable();
            } else {
                emit dbLatest();
            }
        }
        checkDBReply->deleteLater();
    });
}

void AppSettings::checkDatabaseUpdate()
{
    Test *dbUpdate = new Test();
    connect(dbUpdate, &Test::dbUpdateAvailable, this, &AppSettings::setDBUpdate);
    connect(this, &AppSettings::dbUpdateConfirmed, dbUpdate, &Test::updateDatabase);
    connect(dbUpdate, &Test::updateDBCompleted, this, &AppSettings::setDBUpdated);
    connect(dbUpdate, &Test::dbLatest, this, &AppSettings::setDBLatest);
    connect(this, &AppSettings::dbUpdateCanceled, dbUpdate, &Test::setUpdateCanceled);
    connect(dbUpdate, &Test::updateDBFinished, dbUpdateThread, &QThread::quit);
    connect(dbUpdate, &Test::updateDBFinished, dbUpdate, &Test::deleteLater);
    dbUpdate->checkDBUpdate();
}

[Cobra91151]

@jsulm

Thanks for the code but I get errors:

error: C4573: the usage of 'CheckUpdates::dbUpdateAvailable' requires the compiler to capture 'this' but the current default capture mode does not allow it

When changing lambda to connect(checkDBReply, &QNetworkReply::finished, [checkDBReply, this]() { I get error:

error: C3493: 'localDBFile' cannot be implicitly captured because no default capture mode has been specified

[jsulm]

@Cobra91151 said in QBasicTimer::start: Timers cannot be started from another thread issue:

connect(checkDBReply, &QNetworkReply::finished, checkDBReply, this {

Why did you remove localDBFile?

connect(checkDBReply, &QNetworkReply::finished, [checkDBReply, localDBFile, this]() {

[Cobra91151]

@jsulm

Yes, you right. I missed it. Now it works. Thank you.