> On Dec. 5, 2012, 2:16 a.m., Simeon Bird wrote: > > libnepomukcore/resource/resource.cpp, line 54 > > <http://git.reviewboard.kde.org/r/107573/diff/1/?file=97142#file97142line54> > > > > Might it be better to simply return some empty resource, instead of > > asserting? > > > > In 292996 , we just figured out that a Resource can continue being > > accessed after QApplication has been destroyed, because QRunnable cannot be > > cancelled. If that thread, instead of accessing the Resource, had > > constructed a new Resource, with this code we would crash on exit in a > > release build. > > > > Since we would likely only hit the race in a release build, we would > > get another difficult-to-debug crash on exit. > > Vishesh Handa wrote: > Maybe I should remove the assert? That empty resource doesn't really > cause any harm. What do you think? > > Simeon Bird wrote: > If it were me, I would remove the assert entirely since, as you say, the > empty resource doesn't really cause any harm, and if it is left in we could > easily get bugs reported from some user accidentally compiling in debug mode > and crashing kontact (or whatever). > > But it's up to you - I can see that the assert might be useful if it > occurred somewhere other than application quit.
Removed the assert, and added a kError() message instead :) - Vishesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107573/#review23001 ----------------------------------------------------------- On Dec. 6, 2012, 8:40 a.m., Vishesh Handa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107573/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2012, 8:40 a.m.) > > > Review request for Nepomuk, Sebastian Trueg and Simeon Bird. > > > Description > ------- > > ResourceWatcher: Do no create a new ResourceManager on quit > > The ResourceManager contains a ResourceWatcher. This ResourceWatcher is > initialized only when a Resource class requires it. On destruction it > calls ResourceWatcher::stop, which tries to disconnect it from > ResourceManager::nepomukSystemStarted signal. > > The problem arises when the application is exiting, and the > ResourceManager (being a child of QCoreApplication) is being destroyed. > It in turn destroys the ResourceWatcher, which calls > ResourceWatcher::stop, which calls ResourceManager::instance() which > results in the creation of a new ResourceManager whose parent is > QCoreApplication::instance, which is 0, cause the application is > shutting down. > > This whole extra ResourceManager being allocated is not required and can > be avoided by checking if QCoreApplication != 0, in > ResourceManager::instance. > > Also, since ResourceManager::instance() can now return 0. I've added an > assert check in the Resource class. > > > Diffs > ----- > > libnepomukcore/datamanagement/resourcewatcher.cpp a09b646 > libnepomukcore/resource/resource.cpp 4601eba > libnepomukcore/resource/resourcemanager.cpp 457c042 > > Diff: http://git.reviewboard.kde.org/r/107573/diff/ > > > Testing > ------- > > Added breakpoints in gdb, and wrote a simple application. The extra > ResourceManager is no longer being created. > > > Thanks, > > Vishesh Handa > >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
