| Summary: | ktp-contactlist crashes when setting status to "Offline" | ||
|---|---|---|---|
| Product: | [Unmaintained] telepathy | Reporter: | Elias Probst <mail> |
| Component: | contactlist | Assignee: | Telepathy Bugs <kde-telepathy-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | kde, lacsilva, mklapetek |
| Priority: | NOR | ||
| Version: | git-latest | ||
| Target Milestone: | 0.4.0 | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Bug Depends on: | |||
| Bug Blocks: | 285413 | ||
|
Description
Elias Probst
2012-01-10 19:29:21 UTC
This one is a bit tricky, it's a race condition. Basically what happens is that when you click "Offline" and the combobox closes, your mouse stays right above some contact and a tooltip is spawned. But before it can be fully drawn etc., the backend model data are already gone and therefore it hits null pointer and crashes. I have basic safety checks in place, but it will have to be more sophisticated as that does not prevent the crash. We could perhaps listen to some signal "going offline" (from global presence) and don't create tooltips when it receive that signal. *** Bug 291399 has been marked as a duplicate of this bug. *** We need to solve this before 0.3 release as it is very annoying and very easy to trigger. But this crash is inside Qt itself, which implies - either it's a Qt bug - our models are in a corrupt state for a small period of time. QSortFilterProxyModelPrivate::proxy_to_source checks the index is valid. It must thinks it is.. otherwise it wouldn't continue and get to the line that crashes presumably "m_sourceRows->size()" in QSortFilterProxyModelPrivate::proxy_to_source Our models should never be in a corrupt state, especially as all our stuff runs in one thread. I made a version once that linked against Qt model test lib, we might want to try linking against that again and then triggering this issue. It might tell us where it's screwed. Failing that someone should re-read through accounts-model and look for potential discrepancies where an index could look valid even when it's not. Eliasp (and other people who have seen this crash) were you in groups view or accounts view? (In reply to comment #5) > Eliasp (and other people who have seen this crash) were you in groups view or > accounts view? You mean whether I was using the compact or the full list view? I'm using 'compact'. It used to happen in account view. Curiously enough, after the latest qt upgrade (yesterday) I don't seem to be able to trigger this crash anymore. I am using Kubuntu packages for qt but compiling telepathy-qt from source (master). Few notes: - I tested it only with groups - Listening to the model's rowsRemoved() shows that the model emits the signal way before the data is actually removed (which might actually be correct), but there's a notable delay, therefore the model might still have "valid" indexes (but that's just a wild guess after severe sleep deprivation and sounds really wrong as the indexes imho should be invalidated by the remove-rows call) - I tested with recent Qt git snapshot, 100% reproducible - Checking for indexes in the tooltip part does not help, it returns true for isValid() everytime Checking for the indexes That wouldn't cause a crash though, you'd get it if you emitted rowsRemoved and there was a delay before removing from parent->mPriv->children.. and there isn't. Right. We both made this supercomplicated when it turned out to be really really easy when we look in the right place. QModelIndex.isValid() simply checks model exists and that rows + columns are non negative, it's not a continual check that works forever. It possibly checks < rowCount on creation, but it's not checked each time you called isValid(); We had code in the tooltip-manager that: 1) saved a qmodelindex 2) waited a bit 3) used that modelindex You should /never ever ever/ store a QModelIndex. Patch - QModelIndex item; + QPersistentModelIndex item; isValid now updates correctly so we don't crash. Seems to fix it. If you test it, tomorrow I'll ship it if you're happy enough. Alternate fix is to re-request the model index in prepareToolTip. While testing this I'm reliably unable to reproduce it anymore (I was able to do that 100%). 'Ship it' from me! Git commit b007f34980c2767d2a22e961ef6bb5dda237498f by David Edmundson. Committed on 19/01/2012 at 13:39. Pushed by davidedmundson into branch 'master'. Tooltip manager should store QPersistentModelIndexes not QModelIndexes fixes a crash M +1 -1 tooltips/tooltipmanager.cpp http://commits.kde.org/telepathy-contact-list/b007f34980c2767d2a22e961ef6bb5dda237498f |