Launching a queue of processes (was: "Working with Qthreads")

hbatalha

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.

hbatalha

@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 I isDone 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. ~~But~~ I do want 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.

JonB

@hbatalha
This looks fine. You should free the QProcess you create via new QProcess. Perhaps your lambda could add process->deleteLater() when NotRunning.

hbatalha

@JonB said in Working with Qthreads:

@hbatalha
This looks fine. You should free the QProcess you create via new QProcess. Perhaps your lambda could add process->deleteLater() when NotRunning.

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?

KroMignon

@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();
            }
        });

hbatalha

@KroMignon
The compiler is complaining about unused this.

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?

Anonymous_Banned275

@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 whatever

it seems redundant to

if process changed
if process done
cleanup later

Just saying...
Best of luck

PS
I still think you be better off using QConcurrent - definitely less messy...
Your dime...

jsulm

@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.

JonB

@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 from QObject, it has a constructor QProcess::QProcess(QObject *parent = nullptr). If you specified, say, this as parent (new QProcess(this)) that would make your Dialog be its parent. This would be an improvement, since when Dialog is destroyed it will take each of the created QProcesss 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 QProcresss to be created with the Dialog as parent if it stays there a long time. That would mean that the memory/resources used by each QProcess would persist as long as the dialog is open. Yet we actually finish with each QProcess once it has run the program and finished. Hence we should free them at that point, rather than later on. And for that we use QObject::deleteLater(), for safety.

hbatalha

@JonB
Your explanation is crystal clear. Thank you, for all the help.

hbatalha

@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?!

hbatalha

@KroMignon , @JonB , @JKSH , @jsulm please help me in this new topic I created, about saving the settings/state of an application.