using QtConcurrent::run to run same function multiple time simultaniously

[yazwas]

Good day All

I have this issue, that I want to make sure how to solve it

I use QtConcurrent::run to run a function in the background.

This function is run multiple times (it takes a while to complete executing, it saves image to network drive and then updates the database), and the next call to this function can be issued while the first call is still executing.
I also pass a database object to this function. I want to know if this is a correct thing to do or not. the problem is that it used to run OK under Windows, but under Linux, it keeps crashing whenever the next call to this function is issued.

here is my code

QFuture<bool> future = QtConcurrent::run(&saveImages, imagesList, db);
saveImagesWatcher.setFuture(future);

QFuture<bool> future = QtConcurrent::run(&saveImages, imagesList, db);
saveImagesWatcher.setFuture(future);

To copy to clipboard, switch view to plain text mode 

This is the saveImages function, its a static function

bool MainWindow::saveImages(QList<QImage> &list, QSqlDatabase &database)
{
    QString path = /path/to/drive
 
    QDir dir;
 
    if( !dir.exists(path) )
        if( !dir.mkpath(path) )
			return false;
 
    bool ret = true;
    int i = 0;
    int id_article = 10; //set up somewhere else
 
    for(i=0; i< list.count(); i++)	//this could take a lof of time. depending on the images number 
    {
        QString fname = QString("%1-%2.jpg").arg(id_article).arg(i+1);
        if( file.exists(path + fname) )
            file.remove(path + fname);
 
        ret = list[i].save(path + fname, "jpg", 100);
        if(!ret)
        {
            qDebug() << "saveImages(): Saving image " << fname << " Unsuccessfull";
            ret = false;
            break;
        }
    }
 
    if(!ret)
    {
        deleteEntry(database, id_article);
        return false;
    }
 
    qDebug("saveImages(): Everything is OK for article %d, updating the status and images number", id_article);
    updateStatus(database, id_article, 2);
    updateImagesNumber(database, id_article, list.count());
    return true;
}

bool MainWindow::saveImages(QList<QImage> &list, QSqlDatabase &database)
{
    QString path = /path/to/drive

    QDir dir;

    if( !dir.exists(path) )
        if( !dir.mkpath(path) )
			return false;

    bool ret = true;
    int i = 0;
    int id_article = 10; //set up somewhere else

    for(i=0; i< list.count(); i++)	//this could take a lof of time. depending on the images number 
    {
        QString fname = QString("%1-%2.jpg").arg(id_article).arg(i+1);
        if( file.exists(path + fname) )
            file.remove(path + fname);

        ret = list[i].save(path + fname, "jpg", 100);
        if(!ret)
        {
            qDebug() << "saveImages(): Saving image " << fname << " Unsuccessfull";
            ret = false;
            break;
        }
    }

    if(!ret)
    {
        deleteEntry(database, id_article);
        return false;
    }

    qDebug("saveImages(): Everything is OK for article %d, updating the status and images number", id_article);
    updateStatus(database, id_article, 2);
    updateImagesNumber(database, id_article, list.count());
    return true;
}

To copy to clipboard, switch view to plain text mode 

I want to know what is the best way to run my function in a multiple threads. and should I pass the database object to it or not. if I should use QThread, how can I make multiple calls to the same function, i.e. how can I create multiple thread objects (they should be created every time the function gets called) and run them.

Best regards

[numbat]

The documentations says:

A connection can only be used from within the thread that created it. Moving connections between threads or creating queries from a different thread is not supported.

In addition, the third party libraries used by the QSqlDrivers can impose further restrictions on using the SQL Module in a multithreaded program. Consult the manual of your database client for more information

Depending on the database driver used, you should be right just creating a new connection in each thread. Otherwise, you could use signals and slots to make sure the database operations are always run in the same thread.

[yazwas]

Thanks for the reply.

I would like to know also if the running the function more than one time concurrently (even if we dont have SQL) is correct or not.
i.e. saveImages function called for the next time, while the previous call to the saveImages has not yet ended.

regards

[vcp]

Hi,

I think you should try to use QtConcurrent::mapped to do this.
This approach seems much more applicable to your case
than using QtConcurrent::run()

Take a look at the documentation QtAssistant.

QFuture<QImage> result = QtConcurrent::mapped(files, procImages);
 images->setFuture(result);

QFuture<QImage> result = QtConcurrent::mapped(files, procImages);
 images->setFuture(result);

To copy to clipboard, switch view to plain text mode 

[wysota]

I would like to know also if the running the function more than one time concurrently (even if we dont have SQL) is correct or not.

If the function is reentrant (i.e. uses no global or static data) then yes. But in this particular situation I would probably use mapped() as suggested. The only problem would be if you tried to save the same file more than once at the same time. But with run() you have the exact same situation. Also watch out for the id_article variable if you intend to modify it somewhere (it can make your function not reentrant).

[yazwas]

Thank you all for the reply.

I have another question, if I want to still use the same function, but I would like to use Mutex, like the following case

//GLOBAL definition
QMutex mutex;

//GLOBAL definition
QMutex mutex;

To copy to clipboard, switch view to plain text mode 

bool MainWindow::saveImages(QList<QImage> &list, int id_article)
{
   mutex.lock();
   QString path = /path/to/drive
 
    QDir dir;
 
    if( !dir.exists(path) )
        if( !dir.mkpath(path) )
        {
                mutex.unlock();
        	return false;
        }
 
    bool ret = true;
    int i = 0;
 
    for(i=0; i< list.count(); i++)	//this could take a lof of time. depending on the images number 
    {
        QString fname = QString("%1-%2.jpg").arg(id_article).arg(i+1);
        if( file.exists(path + fname) )
            file.remove(path + fname);
 
        ret = list[i].save(path + fname, "jpg", 100);
        if(!ret)
        {
            qDebug() << "saveImages(): Saving image " << fname << " Unsuccessfull";
            ret = false;
            break;
        }
    }
 
    mutex.unlock();
    return ret;
}

bool MainWindow::saveImages(QList<QImage> &list, int id_article)
{
   mutex.lock();
   QString path = /path/to/drive

    QDir dir;

    if( !dir.exists(path) )
        if( !dir.mkpath(path) )
        {
                mutex.unlock();
        	return false;
        }

    bool ret = true;
    int i = 0;

    for(i=0; i< list.count(); i++)	//this could take a lof of time. depending on the images number 
    {
        QString fname = QString("%1-%2.jpg").arg(id_article).arg(i+1);
        if( file.exists(path + fname) )
            file.remove(path + fname);

        ret = list[i].save(path + fname, "jpg", 100);
        if(!ret)
        {
            qDebug() << "saveImages(): Saving image " << fname << " Unsuccessfull";
            ret = false;
            break;
        }
    }

    mutex.unlock();
    return ret;
}

To copy to clipboard, switch view to plain text mode 

here I removed all the static code, but I added the mutex variable (global one).

if I run this code

QFuture<bool> future_1 = QtConcurrent::run(&saveImages, list_1, id_1);  //list_1 is 100's images, and it can take a few minutes
saveImagesWatcher.setFuture(future_1);
 
.... do some work to get list_2, and id_2
 
QFuture<bool> future_2 = QtConcurrent::run(&saveImages, list_2, id_2);//list_2 is a couple of images and it will take a few seconds
saveImagesWatcher.setFuture(future_2);

QFuture<bool> future_1 = QtConcurrent::run(&saveImages, list_1, id_1);  //list_1 is 100's images, and it can take a few minutes
saveImagesWatcher.setFuture(future_1);

.... do some work to get list_2, and id_2

QFuture<bool> future_2 = QtConcurrent::run(&saveImages, list_2, id_2);//list_2 is a couple of images and it will take a few seconds
saveImagesWatcher.setFuture(future_2);

To copy to clipboard, switch view to plain text mode 

This make the program save the images in list 1 before saving the images in list 2, right?
Is this a good way to enforce locking of the function? and ensure serialization access to the saveImages function?

Thanks a lot

[wysota]

If you use a mutex in such way then you have no concurrency at all. I don't see the point of running the method several times this way.

[yazwas]

If you use a mutex in such way then you have no concurrency at all. I don't see the point of running the method several times this way.

The reason for this is that the user do a generate operation that creates a list of images and then I save those images, I used to do the saving operation them in foreground, and that worked OK, but it took too much time and the program just hangs!

The solution was to move the save operation to background. I use QtConcurrent::run() to do that

Now when it runs in background, the user can do generate operations a few times while the images of the first operation has not finished saving

What I really want to do, is make the function save the images of the 2nd generate operation AFTER the completion of saving for the images of the first operation.
saving of images from the 3rd generation after saving of images from 2nd operation and so on.

I hope this makes sense why I'm asking about Mutex and using it as in the code above, and if that can do this for me.

Best regards

[rickbsgu]

Seems like you could do this with just a worker thread running in the background, saving the images in a loop. In which case, you don't need mutexes at all and you only need one extra thread. If you want to allow the user to do successive saves, but you want the previous one to complete, push them onto a queue and run them serially (might be easier with QThread objects that you can push onto the queue.)

Still don't need mutexes.

[wysota]

I suggest you read this article: [wiki=Keeping_the_GUI_Responsive]Keeping the GUI Responsive[/wiki], there are different approaches you can take described there.

[numbat]

It might be worth noting that Qt's signals and slots provide a nice thread safe queueing mechanism. If you use your own queue, you have to be careful about serialising access to the queue object (ie. you don't want to be pushing and popping at exactly the same instance).