Signals and not propagated outside thread

[sierdzio]

@Mark81 said in Signals and not propagated outside thread:

threadCopier->start();
Engine engine(&worker, &copier); // this is my main class

You start the thread before the engine has a chance to run the connect calls. This way some of the initial emits of finished() might be missed by the engine.

[Mark81]

@sierdzio Actually the copy begins when the user press a PushButton, there are no fires of the finished() signal in the constructor. I forgot to mention in the question - I apologize.

[sierdzio]

@Mark81 said in Signals and not propagated outside thread:

if (info.isFile())
{
_copier->copy(info);

You are not executing this method in a thread at all! This is a direct call to copy() and it will be executed in your main thread.

You have to either trigger the copy by a signal-slot connection, or use QObject::invokeMethod() to fire your method in the thread.

And that explains why the signals are seemingly fired too late - the event loop does not have enough time to process the signals because everything runs on main thread.

I may be wrong here, of course, I just had a quick look at the code.

[Mark81]

Oh, my fault! I thought that because _copier is running in another thread (from main.cpp) any call to its method will run in that thread!

So, I'm changing the code. I'm not sure about the syntax for registering metatypes to be used with QMetaObject::invokeMethod(). Hence I tried the signal-slot connection.

I moved the copy function in the public slot section of Copier.h. Then I declared a signal copier_copy(QFileInfo source) in Engine.h and added this code in the cpp:

qRegisterMetaType<QFileInfo>("QFileInfo");
connect(this, &Engine::copier_copy, _copier, &Copier::copy);
// ...

emit copier_copy(info);

but unfortunately the behavior is exactly the same!

EDIT:

here the invokeMethod() version:

qRegisterMetaType<QFileInfo>("QFileInfo");
// ..

QGenericArgument argument = Q_ARG(QFileInfo, info);
QMetaObject::invokeMethod(_copier, "copy", argument);

but nothing has changed.

[Mark81]

"Solved" in this way - even I don't fully understand why:

in copier.cpp

Copier::Copier(QObject *parent) : QObject(parent)
{
    //connect(this, &Copier::finished, this, &Copier::copyFile);
    connect(this, &Copier::finished, this, &Copier::copier_finished);
}

void Copier::copier_finished()
{
    QTimer::singleShot(0, this, &Copier::copyFile);
}

[dheerendra]

I saw your issue. In the first case 'finished' did not reach engine, becz you never allowed the event loop to start in main thread. I felt your code did not move beyond following line.
"Engine engine(&worker, &copier);"
You can check this by placing debug statement.

I felt that you wanted to make the threaded copy of files.

1. There two threads(main-thread and copy-thread)
2. Main-thread as engine
3. Copy-thread has copier
4. Your idea was, engine should ask the copier to copy the file
5. Copier should copy the contents of the file. This copy operation should happen in copy-thread.
6. Copy-thread should emit the signal saying that copy is 'finished'
7. This finished signal should be caught by main-thread(engine).
8. Engine should update progress bar. Now Engine should tell the copier again that start copying new file.
9. Same task repeat till it copies all the files.

Hope my understanding is correct.

I felt maintaining separate Queue inside the Copier is unnecessary. Signal-Slots across the thread work using Queue.

If this is the case, here is sample code for you.

#include "Copier.h"

Copier::Copier(QObject *parent) : QObject(parent){}
void Copier::run(){}
void Copier::copyFile(int fi)
{
   qDebug() << Q_FUNC_INFO <<  " File Name =" << fi << " Thread ID=" <<   QThread::currentThreadId() << endl;
   QThread::sleep(5);// Simulation code. Here copy file code comes. 
   return copyComplete();
}

Engine::Engine(QObject *parent) : QObject(parent){}

void Engine::copyComplete()
{
   qDebug() << Q_FUNC_INFO << " Copy Complete" <<endl;
   emit startNextFile(x++);
}

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    qDebug() << Q_FUNC_INFO << " Thread ID=" << QThread::currentThreadId() << endl;

    QThread thr;
    Engine eng;
    Copier cp;
    cp.moveToThread(&thr);
    QObject::connect(&thr,SIGNAL(started()),&cp,SLOT(run()));
    QObject::connect(&cp,SIGNAL(copyComplete()),&eng,SLOT(copyComplete()));
    QObject::connect(&eng,SIGNAL(startNextFile(int)),&cp,SLOT(copyFile(int)));
    thr.start();
    eng.startNextFile(0);

    return a.exec();
}

Hope this helps.

[Mark81]

It's all correct but point 8. Actually, in my idea the finished() signal is caught by both copier and engine. Copier will start automatically the next queued file (if any), and engine will update the progress bar only.

In this way I can append new files to be copied everywhere in the program without worrying about the current state. For this reason I kept the queue in the copier class. Anyway your suggestion should work in a simplified case.

[sierdzio]

@dheerendra great post, thanks for chiming in!

@Mark81 said in Signals and not propagated outside thread:

QGenericArgument argument = Q_ARG(QFileInfo, info);

Never use QGenericArgument, it is an internal symbol of Qt. You should just use Q_ARG directly in invokeMethod, however ugly it might look ;-)

[JKSH]

@sierdzio said in Signals and not propagated outside thread:

You should just use Q_ARG directly in invokeMethod, however ugly it might look ;-)

Alternatively, you can use lambdas with QMetaObject::invokeMethod() since Qt 5.10: http://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-4 (No more Q_ARGs!)

[dheerendra]

Additional information. You can keep sending the file names to copier. They will be automatically queued and slot will be executed. It is not like filenames need to be sent once the copy is complete. Engine can keep sending them.