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

[Dawit Alemayehu]

> 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).
> 
> Albert Astals Cid wrote:
>     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.

I have the same concerns. See http://lists.kde.org/?l=kde-commits&m=131281315828663&w=2. Under most circumstances, I too am against using local event loops because they are simply evil for reasons David mentioned above. However, I rather make the exception and go that route in this case to avoid constant UI blocking for those with slow DNS.


- Dawit


-----------------------------------------------------------
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/6e4bb24e/attachment.htm>