Review Request: Do not terminate threads

Thiago Macieira thiago at kde.org
Thu Aug 11 12:11:29 BST 2011


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



kio/kio/hostinfo.cpp
<http://git.reviewboard.kde.org/r/102179/#comment5046>

    This class could be simplified to a simple struct.



kio/kio/hostinfo.cpp
<http://git.reviewboard.kde.org/r/102179/#comment5047>

    This class isn't necessary. We can do the same with QThread alone.
    
    To quit the thread, connect the worker's destroyed() signal to the thread's quit() slot. Then just deleteLater the worker.



kio/kio/hostinfo.cpp
<http://git.reviewboard.kde.org/r/102179/#comment5048>

    Remove the cache too. QHostInfo already caches.



kio/kio/hostinfo.cpp
<http://git.reviewboard.kde.org/r/102179/#comment5049>

    Huh? What is the acquire/release doing here?


- Thiago


On Aug. 11, 2011, 10:34 a.m., Albert Astals Cid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102179/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2011, 10:34 a.m.)
> 
> 
> Review request for kdelibs and Dawit Alemayehu.
> 
> 
> Summary
> -------
> 
> Each time the terminate code triggers my Konqueror crashes, i'm substituting the terminate for just waiting the thread to finish and we just ignoring it.
> 
> The code has a race condition in which wait() returns false, then we switch to the thread and m_autoDelete is still not set and thus noone will delete the thread. I can add a mutex if you guys think this is unacceptable.
> 
> 
> Diffs
> -----
> 
>   kio/kio/hostinfo.cpp 344b1d8 
> 
> Diff: http://git.reviewboard.kde.org/r/102179/diff
> 
> 
> Testing
> -------
> 
> When the 
> kDebug() << "Name look up for" << hostName << "failed";
> if triggers i do not get a crash anymore.
> 
> 
> Thanks,
> 
> Albert
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110811/16b0c06e/attachment.htm>


More information about the kde-core-devel mailing list