Solved Launching a queue of processes (was: "Working with Qthreads")
-
@hbatalha said in Working with Qthreads:
What am missing here?
When it does work, will it run only the three queued processes taken from selections or all of the selections members?I think your knowledge about how Qt works is to "light".
Perhaps you should take time to read some parts of Qt documentation:The example class I have written it very lightweight and is just an implementation example to do what you want to do (at least what I have understood):
- have a list of process to start
- launch next process when first is finished
How does it work:
the class holds aQProcess
member which will start each process and a list which defines the process to be started.
In constructor, the signalQProcess::stateChanged
is used to be informed when the current process is finished and then start next in queue.Before starting the next process, you could, for example, check the results of previous process, with:
- mProc.exitCode() to get application exit code
- mProc.readAll() to get application output
You could also add signals to
Launcher
class to be informed about all process done or to give progression, and what ever you need.Again, this is just a code skeleton, to give you a starting point.
-
@JonB said in Working with Qthreads:
What do you expect to happen, which doesn't happen?
I expect tha
program.exe
will start but it doesn't.Step through it in a debugger, or put qDebug() statements at judicious places. That's what programming is all about!
I did , nothing seems to happen when it gets to
mProc.start(nextP.procName, nextP.procParameters);
. It just finishes up the three elements in the list -
@KroMignon said in Working with Qthreads:
I think your knowledge about how Qt works is to "light".
I agree, I will do just that.
The example class I have written it very lightweight and is just an implementation example to do what you want to do (at least what I have understood):
What I want to achieve is, have a
QStringList
(selections
) with unique arguments id, each argument will be used to start a process in its due time.However, I want theses processes running concurrently but only a given number at one point in time, when one process finishes another one will be started.
For example, suppose
selections
has 5 elements{"1", "2", "3", "4", "5"}
, the element 1, 2, 3 will be the first to start the processes and these processes will run at the same time and when e.g. the process with the argument 1 finishes another process, this time with the argument 4, will be started.This code does exactly that, and as pointed out by @JonB it has repeated code and he suggested me trying to understand your example and work with it. That is what I am trying to do but as beginner to GUI programming and QT it is proving to be quite challenging.
mProc.exitCode() to get application exit code
mProc.readAll() to get application outputTried both, exitcode was 0(zero), and the readAll() shows nothing.
Can you provide an example on how you intended the code to be used?
-
@hbatalha said in Working with Qthreads:
I expect tha program.exe will start but it doesn't.
Your code does not try to execute
program.exe
, it tries to executeprograms.exe
orprgram.exe
. You don't have any code for checking for errors/output, you wouldn't be informed. Even if it did work, unless you say how you know it has not be run it is not evident you would be sure it had not run. A proper version should use the various other signals ofQProcess
; and/or put aqDebug()
statement into the lambda in yourconnect()
so that you see allQProcess:stateChanged
transitions.Glancing now at @KroMignon's code, I'm not sure how it's supposed to work as-is :) The
Launcher()
constructor does theconnect()
for the single processmProc
, whilestartNext()
re-uses thatmProc
to launch each process. I'm not sure you're supposed to re-use an existingQProcess
instance while it has a process running. But he did say "Something like that, up to you to finish it ;)".His code may work only if you run one process at a time, and wait for one to finish before starting the next. Your
start_next_process()
, which you say does work, creates anew QProcess
for each process, so may be more successful in this case.In principle to get 3 going you should be able to put (at least) 3 into the queue and then call
start_next_process()
3 times. Thereafter as one finishes a new one will be pulled and started. I agree you would have to look at how his code works/change it to allow for this. But even with your code earlier, because the body ofon_pushButton_3_clicked()
is so similar tostart_next_process()
it looks to me as thoughon_pushButton_3_clicked()
could simply callstart_next_process()
3 times rather than repeating code. -
This post is deleted! -
@hbatalha said in Working with Qthreads:
However, I want theses processes running concurrently but only a given number at one point in time, when one process finishes another one will be started.
As I wrote before, this was only a basic skeleton, and as @JonB supposed, you can not reuse a
QProcess
to start another process. That is why it runs only once.So I change the class to create a new QProcess on each process start:
class Launcher : public QObject { Q_OBJECT QList<ProcInfo> mProcToStart; QProcess *mProc; public: explicit Launcher(QObject * parent = nullptr) : QObject(parent), mProc(nullptr) { } ~Launcher() { if(mProc) mProc->deleteLater(); } void startProcs(const QList<ProcInfo> &toStart) { mProcToStart.append(toStart); startNext(); } bool startNext() { if(mProcToStart.isEmpty() || (mProc && mProc->state() != QProcess::NotRunning)) return false; auto nextP = mProcToStart.takeFirst(); mProc = new QProcess(); mProc->setProcessChannelMode(QProcess::MergedChannels); connect(mProc, &QProcess::stateChanged, [this](QProcess::ProcessState newState) { if(newState == QProcess::NotRunning) { qDebug() << "Process" << mProc->program() << "with arguments" << mProc->arguments()<< "done"; qDebug() << "Exit code is:" << mProc->exitCode(); qDebug() << "Returned data:" << qUtf8Printable(QString::fromLocal8Bit(mProc->readAll())); qDebug() << "--------------------------------------"; mProc->deleteLater(); mProc = nullptr; if(!startNext()) emit isDone(); } }); mProc->start(nextP.procName, nextP.procParameters); return true; } signals: void isDone(); };
And here is a working example of use:
int main(int argc, char *argv[]) { QCoreApplication a(argc, argv); QList<ProcInfo> procs; QStringList defArgs; defArgs << "-n" << "1" // Only once << "-w" << "5000"; // wait up to 5 seconds procs.push_back({"ping.exe", QStringList() << defArgs <<"google.com" }); procs.push_back({"ping.exe", QStringList() << defArgs <<"amazon.com" }); procs.push_back({"ping.exe", QStringList() << defArgs <<"forum.qt.io" }); Launcher l; l.startProcs(procs); QObject::connect(&l, &Launcher::isDone, &a, &QCoreApplication::quit); return a.exec(); }
But once again, this is only a "playground". Up to you to do adapt this or create a new class to fit your needs.
-
@JonB said in Working with Qthreads:
Your code does not try to execute program.exe
It does, I mistyped.
you don't have any code for checking for errors/output, you wouldn't be informed.
I created the code to check the output, which showed nothing and checked the exitcode too.
unless you say how you know it has not be run it is not evident you would be sure it had not run
The program when it is finished it will have created files in a specific directory. So I go into that directory to check if it has run.
I'm not sure you're supposed to re-use an existing
QProcess
instance while it has a process running.It has to finish first, right? Maybe that is what is stopping it to work correctly. As I said, I am a complete beginner to Qt, I am still picking up things as I go and you guys are being of great help.
it looks to me as though on_pushButton_3_clicked() could simply call start_next_process() 3 times rather than repeating code.
I did just that, works perfect, thank you.
-
@KroMignon said in Working with Qthreads:
So I change the class to create a new QProcess on each process start:
create a new QProcess was one of the many things I tried to see if I could get it to work but now I see I did that wrong.
But somehow it is not working with my code, now it is crashing the entire entire application and I can't seem to figure out why.
I tried running it as a console application but it is complaining that IisDone
is undefined.So, I won't be using your code for now until I gather more knowledge of Qt and also as I have a working code that seems to be ok.
ButIdowant to use it as a console application so I can understand it better and learn from it.As a final request before marking this thread as solved. I would like to know when am I to use this kind of approach or similar.
This is my working code now:
void Dialog::on_pushButton_3_clicked() { for(int i = 0; i < 3; ++i) start_next_process(); } void Dialog::start_next_process() { if(!selections.isEmpty()) { QStringList args; args << "some_args" << selections.takeFirst(); QProcess *process = new QProcess; connect(process, &QProcess::stateChanged, [this](QProcess::ProcessState newState) { if(newState == QProcess::NotRunning) start_next_process(); }); process->setProcessChannelMode(QProcess::MergedChannels); process->start("program.exe", args); } }
What are the differences between your approach and this? ( @JonB I would appreciate hearing this from you also) .
Edit: I got the console application working, put the class in its .h file and now it works.
-
@hbatalha
This looks fine. You should free theQProcess
you create vianew QProcess
. Perhaps your lambda could addprocess->deleteLater()
whenNotRunning
. -
@JonB said in Working with Qthreads:
@hbatalha
This looks fine. You should free theQProcess
you create vianew QProcess
. Perhaps your lambda could addprocess->deleteLater()
whenNotRunning
.I thought that every object created via
new
in Qt would be handled somehow by the Qt application itself so the user won't have to worry about memory management (this is what I got from a YouTube video, maybe I got it wrong).So basically every time, I create a a process via
new
I will have to add this code?connect(process, &QProcess::stateChanged, [this](QProcess::ProcessState newState) { if(newState == QProcess::NotRunning) { process->deleteLater(); } });
or something like that?
-
@hbatalha said in Working with Qthreads:
or something like that?
You are close, you need to capture also
process
in the lambda:connect(process, &QProcess::stateChanged, [this, process](QProcess::ProcessState newState) { if(newState == QProcess::NotRunning) { process->deleteLater(); } });
-
@KroMignon
The compiler is complaining about unusedthis
.dialog.cpp:51:48: warning: lambda capture 'this' is not used
Is it ok if I remove it when I don't have any
this
's method call inside the lambda? -
@hbatalha
In theory "warning" is just that and ignoring it "should not" affect the outcome of the code.
Your mileage will vary...I am still not comfortable with lambda , but if you are looking for whatever ( process, thread etc) to conclude why not say so?
such as
if process done
clean-up whateverit seems redundant to
if process changed
if process done
cleanup laterJust saying...
Best of luckPS
I still think you be better off using QConcurrent - definitely less messy...
Your dime... -
@hbatalha said in Launching a queue of processes (was: "Working with Qthreads"):
Is it ok if I remove it when I don't have any this's method call inside the lambda?
Yes, if you do not access any non-static members/methods.
-
@hbatalha said in Launching a queue of processes (was: "Working with Qthreads"):
I thought that every object created via new in Qt would be handled somehow by the Qt application itself so the user won't have to worry about memory management (this is what I got from a YouTube video, maybe I got it wrong).
Yes and no :)
Because
QProcess
derives fromQObject
, it has a constructorQProcess::QProcess(QObject *parent = nullptr)
. If you specified, say,this
as parent (new QProcess(this)
) that would make yourDialog
be its parent. This would be an improvement, since whenDialog
is destroyed it will take each of the createdQProcess
s with it, preventing a memory leak. (I would probably do this here anyway.) And in many cases in Qt that is enough, e.g. for widget children on a widget parent.Here, however, at least theoretically your code allows hundreds of
QProcress
s to be created with theDialog
as parent if it stays there a long time. That would mean that the memory/resources used by eachQProcess
would persist as long as the dialog is open. Yet we actually finish with eachQProcess
once it has run the program and finished. Hence we should free them at that point, rather than later on. And for that we useQObject::deleteLater()
, for safety. -
@JonB
Your explanation is crystal clear. Thank you, for all the help. -
@jsulm said in Launching a queue of processes (was: "Working with Qthreads"):
Yes, if you do not access any non-static members/methods.
thanks
@AnneRanch said in Launching a queue of processes (was: "Working with Qthreads"):
but if you are looking for whatever ( process, thread etc) to conclude why not say so?
I didn't understand what you meant.
I still think you be better off using QConcurrent - definitely less messy...
Can you elaborate on why you think so.
I read a little about
QConcurrent
and it doesn't seem to be able to do what I want to achieve.
If you disagree can you please provide some code?! -
@KroMignon , @JonB , @JKSH , @jsulm please help me in this new topic I created, about saving the settings/state of an application.