Problem with qtConcurrent function implementation

[Kira]

@mranger90 , @Asperamanca @J-Hilk:
Hello eveyone sharing the logic code of implementation please have a look:

Sharing the Logic of Saving Image using QtConcurrent:

MainWindow.h

struct SaveImage{
// char fileName[100];
String filename;
Mat image; // Please note mat is a class of opencv

SaveImage(){}

SaveImage(String file , Mat img){
    file = filename;
    image = img;
}

};

class MainWindow : public QMainWindow
{

---

Declarations

QQueue<SaveImage> saveQueue;
QFuture<void> future;
QFutureWatcher<void> watcher;

---

---

};

MainWindow.cpp

MainWindow constructor:
watcher.setFuture(future);
connect(&watcher, &QFutureWatcher<void>::finished, this,&MainWindow::processQueue);

Mainwindow readData() // It is slot which gets called data arrives at serialport

if serial data contains given value call
newSaveRequestFromSerialPort() // It is main function which is descibed below

MainWindow::newSaveRequestFromSerialPort()

//The function perform the following
Request the frame from the camera and store it

if(future.isFinished() && saveQueue.isEmpty()) {
future = QtConcurrent::run(this,&MainWindow::saveConcurrentImage,saveImage,capturedFrame);
}else{
elapsedQueue.start();
saveQueue.append(SaveImage(saveImage,capturedFrame));
qDebug()<<"Size of the queue"<< saveQueue.size();
int millisecqueue = elapsedQueue.elapsed();
qDebug()<<"Time taken to save to queue"<<millisecqueue;
}

MainWindow::processQueue //This function is called above when qwatcher is finished:

if(!saveQueue.isEmpty()){
//Save queue is not Empty
if(future.isFinished()){
//Double check to be sure
SaveImage str = saveQueue.takeFirst(); //Removes first element;
future = QtConcurrent::run(this,&MainWindow::saveConcurrentImage,str.filename,str.image);
}

MainWindow::saveConcurrentImage(String fileName, Mat capturedFrame)
It is function called in qconcurrent
It takes received image and store it drive.

My problem is if i create a delay in the saveConcurrentImage such as QThread::sleep(2000);
Future never gets completed and image starts getting saved in the queue.
But if i remove the wait in the above function everything works fine.

[Asperamanca]

Are you sure you want to sleep for 2000 seconds? That's what your code says...

EDIT: Based on this experience, I posted the following suggestion: https://bugreports.qt.io/browse/QTBUG-67367

[J.Hilk]

@Kira
hi,

I don't quite get why you want to sleep the thread in the first place?

By calling QThread::spleep, all event managment is halted and Signal/Slot stuff is actually, I believe, dropped/ignored. This might be the reason why QFuture does not return correctly.

That said, the idea behind Qt::Concurrent is a fire and forget type of 2nd thread.
You passing QtConcurrent data and tell it to write it, and want to know when its done.
Now you tell it, take this data, write it and wait 2 seconds before you tell me you're done.

Techincally, you don't need QFuture in the first place, if you're certain that you won't start more than a handful of Save-functions simultaniously.
In that case,just drop it. You can have multiple QtConcurrents running concurrently. I don't think there's an actual limit, but if you exceed your OS-optimal thread count, the overall process will slow down.

[J.Hilk]

@Asperamanca said in Problem with qtConcurrent function implementation:

Are you sure you want to sleep for 2000 seconds? That's what your code says...

EDIT: Based on this experience, I posted the following suggestion: https://bugreports.qt.io/browse/QTBUG-67367

good catch,

but thats not a bug, Qthread has 3 Sleep functions:

void	msleep(unsigned long msecs) //Miliseconds
void	sleep(unsigned long secs)        //Seconds
void	usleep(unsigned long usecs)   //Microseconds

[Asperamanca]

@J.Hilk said in Problem with qtConcurrent function implementation:

@Asperamanca said in Problem with qtConcurrent function implementation:

Are you sure you want to sleep for 2000 seconds? That's what your code says...

EDIT: Based on this experience, I posted the following suggestion: https://bugreports.qt.io/browse/QTBUG-67367

good catch,

but thats not a bug, Qthread has 3 Sleep functions:

void	msleep(unsigned long msecs) //Miliseconds
void	sleep(unsigned long secs)        //Seconds
void	usleep(unsigned long usecs)   //Microseconds

I know. It's still easy to make a mistake. Type-rich programming can really help here.

[Kira]

@J.Hilk @Asperamanca : Thanks both of you for your reply.
Actually i am not using sleep in my program it was to test a scenario.

The qconcurrent works properly initially but sometimes the save function takes more time to save and QtConcurrent is busy executing the task. By the time the request arises at the serial port so QFuture returns false and image is saved to the queue.

So the problem arises here. After this event occurs QFuture iscompleted never returns true and image keeps on getting saved to the queue.

On debugging line by line i found that when entering the QTConcurrent loops to analyse the flow i introduce a time delay by the time value gets generated at serial port setting the future completed to false.
But after that qfuture never returns true.

The purpose of entering sleep was to figure out whether a wait time in the qtconcurrent function caused the below problem.

[Kira]

@J-Hilk : Would it a better idea if i append the image in queue and run qtconcurrent to save the image or should i go with the QThread approach

[Christian Ehrlicher]

I don't understand why you need to use the same QFuture every time. Why not just create a new future every time and use a QThreadPool to avoid thread creation overhead as explained e.g. here: http://doc.qt.io/qt-5/qtconcurrentrun.html

[Kira]

@Christian-Ehrlicher : I want to keep the order of saving the image sequential.
Also if suppose i create new QFuture everytime and the thread pool executes a qtconcurrent function which never returns true as happening in my situation it would lead to arise a situation like deadlock.

[J.Hilk]

@Kira QThread is an option,

but from what you describe, the case that a 2nd Save request arrives while the old one is still ongoing is rather rare, I would simply go without QFuture and QFuturWatcher and simply call QtConcurrent. The liklyhood that the 2nd save function finished before the first is very very very slim.

[Kira]

@J.Hilk : As per the program logic it can be possible because there is possibility that the qfuture is working and other save request arises because data at serial port is responsible for generating the request. Serial port read function is independent of the qfuture event.
Please correct me if i am wrong!
So if i remove qfuture would QtConcurrent would generate two threads ? And if its so possible then would the second thread can be completed before the first thread.

[Christian Ehrlicher]

QtConcurrent::run() will never block ... please take a look at the documentation: http://doc.qt.io/qt-5/qtconcurrent.html#run

[kshegunov]

@Kira said in Problem with qtConcurrent function implementation:

I want to keep the order of saving the image sequential.

Use a QThread with a worker object to save the images (sequentially), and don't block the event loop ever - read the serial port as fast as possible and then delegate the IO operation to the io thread through a signal. Something like this (needs to go in the appropriate class and the thread and object should be members, code untested):

QThread ioThread;
ioThread.start();

QObject ioWorker;
ioWorker.moveToThread(&ioThread);

QObject::connect(this, &MainWindow::saveConcurrentImage, &ioWorker, [] (String fileName, Mat capturedFrame) -> void  {
     // ... Saving to disk goes here
});

You will also need to have Q_DECLARE_METATYPE for your String and Mat, and qRegisterMetatype at the beginning of your main to leverage argument packing for queued connections.

PS.
If you use stack objects, you need this piece in the class' destructor to ensure proper cleanup:

MainWindow::~MainWindow()
{
    ioThread.quit();
    ioThread.wait();
}