smurfy / fahrplan

QT Application for Public transportation
GNU General Public License v2.0
58 stars 32 forks source link

Fahrplan segfault #135

Open smurfy opened 9 years ago

smurfy commented 9 years ago

Fahrplan seams to segfault if you leave the settingspage before the calendars could be loaded.

Reproduce:

The bug is old so i think it has time for 2.0.22 :)

smurfy commented 9 years ago

Interesting, it seams the problem only occures on my N9, not my N950 or at least i cant reproduce it on the N950 because it loads the calendars a lot faster than on the N9

leppa commented 9 years ago

Sounds like a race condition in loading list of calendars.

smurfy commented 9 years ago

yea, in it crashes in the thread called by the future thingy.. :) debugging is quite a pain in the ass. Only occures on the N9 with the default user not the developer user. But i'm getting there.

leppa commented 9 years ago

Aha. I have a theory. QtConcurrent runs FahrplanCalendarManager::getCalendarsList() in a separate thread. And from there it accesses object's members directly. When user exits SettingsPage, it gets deleted together with FahrplanCalendarManager. I assume that crash happens when FahrplanCalendarManager::getCalendarsList() still runs while FahrplanCalendarManager is being deleted: the members of FahrplanCalendarManager don't exist anymore while FahrplanCalendarManager::getCalendarsList() still tries to access them.

Try doing the following:

  1. Save the result of QtConcurrent::run() call (line 155) as a member of FahrplanCalendarManager.
  2. Create a destructor for FahrplanCalendarManager and do:

    m_future.cancel();
    m_future.waitForFinished();

    there.

leppa commented 9 years ago

Even better:

if (m_future->isRunning()) {
    m_future.cancel();
    m_future.waitForFinished();
}
benni0815 commented 9 years ago

From the QtConcurrentRun - Asynchronous Run doc:

Note that the QFuture returned by QtConcurrent::run() does not support canceling, pausing, or progress reporting. The QFuture returned can only be used to query for the running/finished status and the return value of the function.

Perhaps disconnecting the signals of m_watcher will work.

leppa commented 9 years ago

Perhaps disconnecting the signals of m_watcher will work.

This won't work, because getCalendarsList() will still try to access FahrplanCalendarManager's members (which don't exist anymore because manager was deleted). If it's not possible to cancel QFuture than the only way to be on a safe side is to waitForFinished() in the destructor. This might delay destruction but will avoid a crash.