Search code examples
c++qtmemoryqt5

How do I use QProcess in an asynchronous manner *without* causing a memory leak?

Warning - I am a near-total noob at C++ and Qt.

I'm working on a fairly simple Qt Quick app. Part of this app needs to execute various shell commands and use their output to do stuff. The most obvious way that I could see to do this was via QProcess, by doing something a bit like this:

void MyClass::exec(QString args) {
    QProcess proc;
    QStringList argList;
    argList.append("-c");
    argList.append(args);
    proc.start("bash", argList);
    proc.waitForFinished();
    QByteArray result = proc.readAllStandardOutput();
    QString final = QString::fromUtf8(result);
    m_stdout = final;
    emit stdoutChanged();
}

This worked, but had the unfortunate side effect of causing the GUI to freeze while the shell command passed to "args" ran (since the command takes about a second or so to run).

So then I tried to get rid of the waitForFinished() call and switch to using a signal instead. Pretty quickly I figured out that I needed the QProcess object to last after exec() finishes. I tried making the QProcess a member of the class (declaring it in the .h file), which worked kinda, but had the unfortunate result of only letting me run one shell command at a time. Depending on how the user manipulates the UI, the application may end up trying to run multiple commands at once, and this setup resulted in a race condition where anything that tried to execute while something else was already executing got ignored.

So... I moved the QProcess back into the exec() function, but switched to using a QProcess * instead so that the object would survive when it went out of scope. (I'm guessing this is where the more experienced users are thinking "No. That's not how you do this.", but I don't know a better way to do this, thus why I'm here.) So now I have this mess:

void MyClass::exec(QString args) {
    QProcess *proc = new QProcess();
    QStringList argList;
    argList.append("-c");
    argList.append(args);
    proc->start("bash", argList);
    connect(proc, SIGNAL(finished(int)), this, SLOT(triggerStdout()));
}

void MyClass::triggerStdout() {
    QByteArray result = ((QProcess*)sender())->readAllStandardOutput();
    QString final = QString::fromUtf8(result);
    m_stdout = final;
    emit stdoutChanged();
}

Volia! The UI no longer blocks, and multiple commands can run at once. And I also leak an entire QProcess object every single time exec() gets called. Tar.

In an attempt to free the QProcess object, I proceeded to try using delete sender() at the end of triggerStdout() in order to free the memory. Which resulted in the application crashing when triggerStdout() gets called.

So now I'm lost. My next thought was to try and call the QProcess object's destructor at the end of triggerStdout(), but from what I researched, calling destructors on Qt objects is a Bad Idea (I'm guessing because you usually don't use pointers like this?). I think I'm probably not supposed to use a QProcess * to do this, but I don't know what else to do. All the ideas I have at this point have various drawbacks:

  • Make a QList or something to store the QProcesses in, and remove them when they're unneeded. This would be great if I could figure out which QProcess emitted the finished signal. I mean I guess I could use processID to loop through the QList and find the right process object to get rid of, but that seems clunky.
  • Spin up a new thread and then use the synchronous technique I tried at first. I'm not sure how I won't just end up with the exact same problem as I have now, only now I'll be leaking a QThread instead of a QProcess, plus multithreading could cause problems in the application itself due to how it's designed.
  • Figure out how on earth to manually dispose of the QProcess object once it's unnecessary. This seems like the cleanest route to me, but also seems like something I'm not supposed to do.

Assuming those are my only options, I think the first one is probably the best idea (if it will work at all), but I'm hoping there's some way of doing this that is clean, simple, and doesn't break best practices or established rules.

Solution

  • That's a common problem when you want to delete an object from within a method-call that was called by that very object... you call delete, which is fine, but then as soon as your method-call returns, the calling-method starts executing again, only to find that its own this-pointer is now a dangling-pointer to freed memory, and boom. :)

    One easy way to avoid that trap is to replace your call to delete sender() with a call to sender()->deleteLater(), which (as the method name suggests) won't delete the QProcess right now, but rather will add it to a queue of items to be deleted on the next go-around of the Qt event loop (read: after the current slot-method-calls from the QProcess have all returned, so it'll be safe to do)