Bugfix: KTorrent crashes on exit due to access to invalid memory block
Review Request #130036 - Created March 19, 2017 and submitted
I've noticed KTorrent crashing every time it close. I rarely close such apps but during development it happens often. And I realized that my previously submitted review cause debugger to bark on invalid memory access when KTorrent exits at particular line.
It happenes when I call
ViewModel::data(). And it looks like this is because
tcpoints to object that was already deleted in
Core::onExit(). I've rolled back my changes and found out that other code in data() seems not to worry debugger and looks like that's because it acess only simple object's properties like
getDisplayName()that mey be inlined... don't know. Still original app is crashing, but somewhere in delegate classes.
I've dubugged this and think the problem is following:
ViewModel's items contain pointers
TorrentInterface*which were coped from
QueueManager's pointers list.
When I close the app I'm invoking
QueueManager::clear()and there all
TorrentInterface*are deleted but their memory is not nulled. And no one informs
ViewModelthat its items now contain invalid pointers. And in case your
ViewModellives some time longer than
coreyou're accessing dead objects throw these links.
In my case I have > 200 torrents and it takes 30 sec for KTorrent to save all chunks and stats at exit. All that time GUI is on screen. Probably this allows me to face with this problem in 100% cases.
It's not easy to communicate destruction of torrent pointers to data model without making serious changes to architecture. So I've decided to do the same trick
coredo - to subscribe
QCoreApplication::aboutToQuit()signal and set up an
exitingflag to do nothing on exit. And now everything works like a charm with my code (highlights on review) and with original code.
Well, it's not exactly what I promised - I need to add one more signal:
ModelView::onExitnow connects to it instead of
QCoreApplication::aboutToQuit(). Just because
Coreit can't be notified before
Coredelete torrents. And invoking
ModelView::torrentRemovedat this point cause crash.
So I made additional signal for
Coreand emit it at
Core::onExitbefore torents destruction. This signal might be visible via D-Bus but never could be used via D-Bus as D-Bus is already shut down at this point of time.
Revision 4 (+17)
I would remove DirectConnection, if core is not living in the same thread that "this" is living, you'd have lots of problems caused by this DirectConnection, so basically use the default that it will be a direct connection because both objects live in the same thread, it is "the same" but is cleaner (and follows the pattern of the other slots).