0

Everytime I call QDir::removeRecursively() my application crashes AFTER having removed the folder containing the files correctly. Having done some testing I found out that it depends on how I call the function. This is my code:

Recorder::Recorder(ParentClass *parent): QObject(parent){

     connect(this,SIGNAL(finishedRec(QString,QString)),this,SLOT(finishedRecording(QString,QString)));
}

void Recorder::start(){
    if (!recording){
        recording=true;
        recThread.reset(new std::thread(&Recorder::recordThread, this));
    }
}

void Recorder::stop(){
    recording = false;
    recThread->join(); recThread.reset();
}
void Recorder::recordThread(){
    QString picDir;
    QString filename;
    //....
    while(recording){
        //writing frames to folder
    }    
    emit finishedRec(picDir,filename);    
}

void Recorder::finishedRecording(QString picDir, QString filename){
    QProcess* proc = new QProcess();
    vecProcess.push_back(proc);
    vecString.push_back(picDir);
    proc->start("C:\\ffmpeg.exe", QStringList() <<"-i"<< picDir << "-r"<< "30" << "-vcodec"<< "ffv1" << filename);
    connect(proc,SIGNAL(finished(int)),this,SLOT(finishedProcess()));
}

void Recorder::finishedProcess(){
    for (int i=0; i<vecProcess.size();i++){
        if(vecProcess.at(i)->state()==QProcess::NotRunning){
            delete vecProcess.at(i);
            vecProcess.erase(vecProcess.begin() + i);

            QString folderToRemove=vecString.at(i);
            folderToRemove.chop(12);
            qDebug() << folderToRemove;
            QDir dir(folderToRemove);
            dir.removeRecursively();
            vecString.erase(vecString.begin() + i);

        }
    }
}

Only if I leave dir.removeRecursively() in, my application will always crash. Without it everything works as intended. Even deleting all the files with

QDir dir(path);
dir.setNameFilters(QStringList() << "*.*");
dir.setFilter(QDir::Files);
foreach(QString dirFile, dir.entryList()){
    dir.remove(dirFile);
}

will cause a crash AFTRER all the files were deleted.

I'm running my recordThead as a std::unique_ptr<std::thread>. I did try to run the thread as a QThread but that gave me the exact same result. If dir.removeRecursively() was called the program will crash after finishing the event in finishedProcess()

Calling removeRecursively() in a different event loop works. Why doesn't it work when using it in a SLOT like shown in my example?

testus
  • 183
  • 2
  • 21

1 Answers1

1

vector erase effectively reduces the container size by the number of elements removed, which are destroyed.

// one suspect
vecProcess.erase(vecProcess.begin() + i);
// another suspect
vecString.erase(vecString.begin() + i);

And you call that in a loop where 'i' gets incremented? Should eventually attempt to erase something beyond the vector size. I would just release entire container if possible after the loop finished or used list. And maybe you don't need to store pointers in those containers but values (?). Storing pointers to objects allocated by you makes you to release the one by one and sometimes, yes, justified but with C++ 11 and move semantics it is not always the case.

Alexander V
  • 8,351
  • 4
  • 38
  • 47
  • Hmm..I don't know if thats the problem. Like I said, If I only remove the dir.removeRecursively(); line my program works as intended without any crashes. I ended up creating my own program that deletes a folder and then starting it as a QProcess and passing the folder location. This works. I have no idea why I can't use the same code in my slot. – testus Apr 22 '15 at 15:50
  • At least the logic is definitely wrong with these lines. First you get Undefined Behavior and then crash. The 'if' operator is also affecting whether or not the code actually crash. – Alexander V Apr 22 '15 at 19:47
  • I don't get whats wrong with the logic. So I want to loop through the vecProcess, find the QProcess which is not running, delete the pointer and remove the element from the vector. vecProcess.erase(vecProcess.begin() + i); only removes one element and I'm not seeing how resizing the vector will cause a problem. Can you explain? – testus Apr 22 '15 at 20:19
  • I will probably add a break statment after erasing the element from the vector just to be safe (since finishedProcess() will only remove one element) but so far I haven't seen any unexpected crashes with my implementation of dir.removeRecursively(); as its own QProcess – testus Apr 22 '15 at 20:21
  • For instance: when there is just 1 item vecProcess.erase(vecProcess.begin() + i) will attempt to erase what? Also these 2 don't seem to me to go in sync: delete vecProcess.at(i); vecProcess.erase(vecProcess.begin() + i); if you meant to delete the object and then delete its item in the vector. This whole code is maybe interesting but I would rewrite this part somehow easier and more understandable. – Alexander V Apr 23 '15 at 01:08
  • Wouldn't that delete the element at position 0? http://stackoverflow.com/a/875117/4023471 – testus Apr 23 '15 at 01:22
  • See, testus, at position 0 you get that right. And then? The vector is getting shorter, right? And at the same time you increment 'i'? I believe that somewhere in the middle of original vector size there is an interesting effect there for skipping certain vector items with 'if' condition, as well. – Alexander V Apr 23 '15 at 07:34