[Kde-pim] Review Request: Fix deadlock situation when open mail (problem in nepomuksearch.cpp)

[David Faure]

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



server/src/nepomuksearch.cpp
<http://git.reviewboard.kde.org/r/105599/#comment12607>

    If you use new and delete in the same block of code, then you don't need new and delete; create the object on the stack instead.
    
    QueryServiceClient idSearchService( this );
    connect( &idSearchService, ... );



server/src/nepomuksearch.cpp
<http://git.reviewboard.kde.org/r/105599/#comment12606>

    Isn't a mResultsWithoutIdProperty.clear() missing here?


- David Faure


On July 16, 2012, 8:36 p.m., Andreas Cord-Landwehr wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105599/
> -----------------------------------------------------------
> 
> (Updated July 16, 2012, 8:36 p.m.)
> 
> 
> Review request for Akonadi.
> 
> 
> Description
> -------
> 
> (I posted my problems already at the kdepim list: http://lists.kde.org/?l=kde-pim&m=134236644810213&w=2, the following is copy&paste from that mail, concluding some infos about the patch)
> 
> Introduction:
> In KMail I have a couple of mails that when I click on them (i.e. to display) I get the "Retrieving Folder Contents" bluescreen. With GDB I followed the problem into the Akonadi sources: When trying to display the mail at some point an Akonadi Connection is created as well as AkonadiConnection::run() is exucuted, which yields (after some more calls) to search.cpp:55 where a 
> NepomukSearch object is created and (one line later) a search is executed. The following I give in detail to explain how there a threading issues emerges from the calls (especially here in case of a specific mail I try to display):
> 
> Step 1. Object creation of "NepomukSearch* service" in search.cpp:55
> This causes the NepomukSearch ctor to create:
>   mSearchService = new Nepomuk::Query::QueryServiceClient( this );
>   mIdSearchService = new Nepomuk::Query::QueryServiceClient( this );
> Futher in the ctor, for both there newEntries-signals are connected to methods 
> of NepomukSearch, ie.
>   connect( mSearchService, SIGNAL(newEntries(QList<Nepomuk::Query::Result>)),
>            this, SLOT(hitsAdded(QList<Nepomuk::Query::Result>)) );
>   mIdSearchService = new Nepomuk::Query::QueryServiceClient( this );
>   connect( mIdSearchService, 
> SIGNAL(newEntries(QList<Nepomuk::Query::Result>)),
>            this, SLOT(idHitsAdded(QList<Nepomuk::Query::Result>)) );
> 
> Step 2. search:cpp:56 service->search(...) is called. There we get:
> * mSearchService->blockingQuery(...) issues a query
> * this query finds 6 results (in my example)
> * by former connection of mSearchService, for each result 
> NepomukSearch::hitsAdded is invoked
> * FOR-EVERY-OF-THESE-RESULTS it happens in NepomukSearch::hitsAdded(...)
>   * the result has akonadi ID => mIdSearchService->blockingQuery(...) is 
> invoked
> * THE-LAST-INVOKED-QUERY finishs and invokes by the, in the ctor created, 
> connection NepomukSearch::idHitsAdded(...)
>   * uses addHit(...) to add result to result-list
>   * then idHitsAdded(...) returns
>   * the blockingQuery THE-LAST-INVOKED-QUERY returns from the call from 
> QueryServiceClient::blockingQuery
> <<< AKONADI STALLS >>>
> 
> Conclusion:
> I verfied that virtuoso finished all queries as well as that none of the other blockingQuery calls returned. (Akonadi is Git master, KDEPIM/KDEPIMlibs is KDE 4.9 RC2.)
> I suspect that by return from the blockingQuery the still blocking thread is invoked. Hence, the next blockingQuery issued by mSearchService can never return.
> 
> Solution:
> In the attached patch I move the corresponding code that was formerly used by mIdSearchService to request missing IDs in parallel to the exuecuted mSearchService blocking query is moved to a point where the initial blocking query is finished an the described deadlock cannot happen.
> 
> 
> Diffs
> -----
> 
>   server/src/nepomuksearch.h c502bd6 
>   server/src/nepomuksearch.cpp da7b19a 
> 
> Diff: http://git.reviewboard.kde.org/r/105599/diff/
> 
> 
> Testing
> -------
> 
> I applied the patch to master and tested for several hours. Without the patch I got deadlockes deterministly when trying to open specific mails. With the patch I did not get any new deadlock.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/