Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp

[Albert Astals Cid]

> On Aug. 8, 2011, 1:53 p.m., David Faure wrote:
> >
> 
> David Faure wrote:
>     (Sorry, flaky wifi lost the comment)
>     
>     I am very much against a nested event loop (QEventLoop::exec), it's a well-known fact nowadays that it creates unexpected re-entrancy and crashes.
>     
>     And since I just fixed the crash (missing wait() after terminate(), see commit log), I don't think we need this change. However reusing threads might be a good idea (separate issue).

What you did seems like breaking the idea of the timeout parameter altogether since you are basically waiting (and blocking the UI) until my slow DNS answers instead of only waiting for the time the timeout specifies.


- Albert


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


On Aug. 7, 2011, 4:07 a.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102238/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2011, 4:07 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This patch replaces the creation of a new thread for every DNS lookup with a local even loop based solution. This change addresses the issue of crashes that can arise from terminating the thread when the desired timeout duration is exceeded. See http://git.reviewboard.kde.org/r/102179/.
> 
> 
> Diffs
> -----
> 
>   kio/kio/hostinfo.cpp fcb7889 
> 
> Diff: http://git.reviewboard.kde.org/r/102238/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit
> 
>

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