Review Request: Crash when browsing a category [@ CollectionTreeItem::row]

[Wolfgang Lorenz]

> On Feb. 25, 2011, 5:36 p.m., Maximilian Kossick wrote:
> > I'm not sure about the need to abort the QMs. The QM -> model communication is exclusively via slots. disconnecting the qm's signals should prevent any message from reaching the model.
> > 
> > Otherwise the patch makes a lot of sense and should be merged asap.

Concerning the QM abort: After I found the "-nofork"-trick, I got a backtrace of the crashes. They happened here:

#0  0x00007ffff6bbd0a0 in Collections::ServiceSqlQueryMaker::handleResult (this=0x1a27360, result=...) at /home/amarokdev/workspace/amarok/src/services/ServiceSqlQueryMaker.cpp:613
#1  0x00007ffff6bbf293 in ServiceSqlWorkerThread::run (this=0x1a1d640) at /home/amarokdev/workspace/amarok/src/services/ServiceSqlQueryMaker.cpp:58
#2  0x00007ffff24441c1 in ThreadWeaver::JobRunHelper::runTheJob (this=0x7fffbb70cd30, th=0x144a490, job=0x1a1d640) at ../../../threadweaver/Weaver/Job.cpp:106
[...]

The way I see it, the running query is not stopped, when the query maker is deleted. So, when the query finishes, the query maker is still called to handle the result, which then leads to a crash. I think deleting an object, that is being used in another thread, without finishing this thread, might not be thread save.

Of course, the aborting of the query could also be done in the destructor of the QM, if necessary.


- Wolfgang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100723/#review1649
-----------------------------------------------------------


On Feb. 25, 2011, 1:16 p.m., Wolfgang Lorenz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100723/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2011, 1:16 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Summary
> -------
> 
> patch for https://bugs.kde.org/show_bug.cgi?id=233170
> 
> When a query is started the Collections::QueryMaker* is stored in
>     QHash<CollectionTreeItem*, Collections::QueryMaker*> runningQueries.
> 
> Now, when CollectionTreeItemModelBase::populateChildren deletes items
>     CollectionTreeItemModelBase::itemAboutToBeDeleted
> is called, which in turn stops, disconnects and deletes the queryMaker associated with the item.
> 
> The problem occurs, if there is more than one queryMaker for the item, as will happen, when
>     CollectionTreeItemModelBase::handleCompilations
> is called. In this case itemAboutToBeDeleted deletes only one queryMaker. The other one finishes normally, calls
>     CollectionTreeItemModelBase::queryDone,
> which in turn calls 
>     CollectionTreeItem::row
> of a deleted item and Amarok crashes with a segfault.
> 
> The patch changes the runningQueries buffer to QMultiHash. So all queryMakers are kept and itemAboutToBeDeleted can stop them all.
> 
> 
> This addresses bug 233170.
>     https://bugs.kde.org/show_bug.cgi?id=233170
> 
> 
> Diffs
> -----
> 
>   src/browsers/CollectionTreeItemModelBase.h df260d6 
>   src/browsers/CollectionTreeItemModelBase.cpp 9e5a50d 
> 
> Diff: http://git.reviewboard.kde.org/r/100723/diff
> 
> 
> Testing
> -------
> 
> Sometimes Amarok still crashed, when entering a search. Unfortunately, the backtrace was always empty in these cases. I don't know why. This could be another bug.
> 
> 
> Thanks,
> 
> Wolfgang
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20110226/ac4f2288/attachment.htm