[Nepomuk] Review Request: Delete the IndexSchduler before its thread quits

[David Faure]

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



services/fileindexer/fileindexer.cpp
<http://git.reviewboard.kde.org/r/105806/#comment13832>

    Vishesh, if you're not sure why something gets deleted, use gdb to set a breakpoint in the dtor :-)
    
    My take on this is: indeed, deleteLater+quit, in any order, is not reliable. This is because quit sets a bool, rather than posting an event, which m
    eans the thread's eventloop could exit before processing the event.
    
    The solution is to post an event to the thread. In KDSoapServer, I call myThread->quitThread(), which is an added public method which calls QMetaObj
    ect::invokeMethod(someObject, "quitThreadImpl"). someObject has to be an object "associated with this thread", i.e. created by this thread, in run(). And quitThreadImpl() can then call thread->quit(), from the thread itself rather than from the outside.
    Don't call invokeMethod(myThread, "...") because myThread is associated with the main thread! The common QThread trap.
    You want an object whose event processing happens within the thread itself.
    
    This sounds a bit convoluted, but actually it's good practice to never have slots in a QThread subclass (to avoid falling in the trap above), so I a
    lways recommend to split the QThread subclass and the actual object that you're working with. If you already do that, then that object is associated with the thread, and you can post events to it. Maybe you already have such an object (this construct is useful for any slots, not just for calling quit from the outside), I admit that I didn't read the code that all this is about :-)
    
    Maybe Qt could make some of this easier, for instance with a QThread::postedQuit() or something like that.


- David Faure


On Aug. 18, 2012, 7:34 p.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105806/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2012, 7:34 p.m.)
> 
> 
> Review request for Nepomuk, David Faure, Thiago Macieira, Sebastian Trueg, and Marc Mutz.
> 
> 
> Description
> -------
> 
>     The IndexScheduler lives in the m_schedulingThread, which should ideally
>     be deleted before the thread finishes execution.
> 
> 
> This addresses bug 301698.
>     http://bugs.kde.org/show_bug.cgi?id=301698
> 
> 
> Diffs
> -----
> 
>   services/fileindexer/fileindexer.cpp af8be4d 
> 
> Diff: http://git.reviewboard.kde.org/r/105806/diff/
> 
> 
> Testing
> -------
> 
> Nope. Cannot test this. In fact I'm not even sure if this was actually the reason for the crash.
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20120818/17f2a3df/attachment.html>