> 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 > >
