Problem with getting QFuture and QFutureWatcher working properly

[jahsiotr]

Hey all,

I'd like to ask for a little help as I followed some quick tutorials on how to get another thread in my app working. The thing I'm doing is a kind of organiser with calendar that has ability to popup reminders for the user and I'd like to use different thread for checking if there isn't any task to remind the user of and if there is show a popup with info about it. I figured a way to do it from some websites and this is what I've come up with, but unfortunately app is not responding when I try to run it:

QFuture<Task*> future = QtConcurrent::run(this, &MainWindow::checkReminders);
 
    popupTask = future.result();
 
    QFutureWatcher<Task*> watcher;
    watcher.setFuture(future);
 
    connect(&watcher, SIGNAL(finished()), this, SLOT(popupReminder()));

QFuture<Task*> future = QtConcurrent::run(this, &MainWindow::checkReminders);

    popupTask = future.result();

    QFutureWatcher<Task*> watcher;
    watcher.setFuture(future);

    connect(&watcher, SIGNAL(finished()), this, SLOT(popupReminder()));

To copy to clipboard, switch view to plain text mode 

Above is set in MainWindow constructor.

I haven't already connected the popupTask variable to popupReminder() method, because I wanted to make it work first before providing the popup with the information to show.

Here is MainWindow::checkReminders() method:

Task* MainWindow::checkReminders() {
    do {
        for(int i = 0; i < daysList.count(); i++) {
            for(int j = 0; j < daysList[i].getListCount(); j++) {
                QDateTime current = QDateTime::currentDateTime();
                if(current == *daysList[i].getTaskAtIndex(j)->getReminderTime() &&
                        !daysList[i].getTaskAtIndex(j)->ifDone() &&
                        daysList[i].getTaskAtIndex(j)->ifNeededReminder()) {
                    daysList[i].getTaskAtIndex(j)->turnOffReminder();
                    return daysList[i].getTaskAtIndex(j);
 
                }
            }
        }
    } while (true);
 
}

Task* MainWindow::checkReminders() {
    do {
        for(int i = 0; i < daysList.count(); i++) {
            for(int j = 0; j < daysList[i].getListCount(); j++) {
                QDateTime current = QDateTime::currentDateTime();
                if(current == *daysList[i].getTaskAtIndex(j)->getReminderTime() &&
                        !daysList[i].getTaskAtIndex(j)->ifDone() &&
                        daysList[i].getTaskAtIndex(j)->ifNeededReminder()) {
                    daysList[i].getTaskAtIndex(j)->turnOffReminder();
                    return daysList[i].getTaskAtIndex(j);

                }
            }
        }
    } while (true);

}

To copy to clipboard, switch view to plain text mode 

and finally the popupReminder() method:

void MainWindow::popupReminder() {
    QMessageBox::StandardButton reply;
    reply = QMessageBox::information(this, tr("QMessageBox::information()"), MESSAGE1);
}

void MainWindow::popupReminder() {
    QMessageBox::StandardButton reply;
    reply = QMessageBox::information(this, tr("QMessageBox::information()"), MESSAGE1);
}

To copy to clipboard, switch view to plain text mode 

Earlier I used checkReminders as a void method that just made the window pop up itself, but I've read somewhere that it is better not to make multiple threads take care of GUI, so I thought I'll give it a try with QFutureWatcher. If someone could point out what I'm doing wrong I'd be grateful.

[wysota]

To be honest you are doing a couple of things wrong. One thing is definitely that you're not protecting your "daysList" data structure from concurrent access from multiple threads. Another is that your loop can potentially run forever (actively) if it fails to find a task that needs to be returned. Third of all is that your search is very inefficient. It's much more efficient to have a queue of tasks sorted by the reminder time. Then all you need to do is check once a minute the first item in the queue whether its time has come or not. Or even set the timer to alarm you when the first item in the queue becomes active. Of course then you need to reset the timer if anything in the queue changes. There is no need nor any benefit from using threads in this situation. Even without any special treatment you don't need any extra threads -- once a minute go through all the items in daysList and check whether any of them need reminding. Assuming you have 10-ish or even 100-ish items in the list, it should be fast enough to not stall your UI.

Finally your last problem (which is the only one related to the future itself) is that you start a task and immediately ask for its result. If the result is not ready yet, the calling thread will be blocked until the result becomes available. Thus your watcher is never used. If you remove line #3 from your code, the program will start working.

[jahsiotr]

That is a bunch of good advice. Thanks for that. How can I use some method every minute though and not hold up the rest of app by waiting for the timer to indicate that full minute has passed?

[wysota]

Use QTimer.

[anda_skoa]

There is an additional problem in the original code: the future watcher object is created on the stack, so it is being deleted whenever that function's context ends.
If I read this correctly this is code inside the main window's constructor, so the future watcher will be gone by the time the window shows up.

Cheers,
_