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


Can't find any other obvious problem. I still think that waiting 250ms for the 
thread to start is a worse solution than using a semaphore as i did in my own 
patch at David's suggestion, but it seems that even you say my patch is not 
good you ended up coding something that while somehow different is effectively 
the same. For me i'm happy that you commit your patch, this way if it still 
fails after this patch it won't be my fault.

- Albert


On Aug. 18, 2011, 9:45 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102238/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2011, 9:45 p.m.)
> 
> 
> Review request for kdelibs, David Faure and Thiago Macieira.
> 
> 
> Summary
> -------
> 
> The attached patch is an alternate approach to address the issue of crashes 
> that arise from terminating an active thread than the one proposed at 
> https://git.reviewboard.kde.org/r/102179/. With this patch the function 
> "QHostInfo::lookupHost(QString, int)" avoids the use of QThread::terminate 
> with the following fairly simple changes:
> 
> - Connect its finished signal to its parent deleteLater slot in the ctor so 
> that the thread is automatically deleted later.
> - Store the looked up DNS info in  the global cache to avoid unnecessary 
> queries for the same request.
> - Check for cached DNS information and avoid doing reverse look ups before 
> resorting to performing DNS queries in a separate thread.
> 
> 
> Diffs
> -----
> 
>   kio/kio/hostinfo.cpp 344b1d8 
> 
> Diff: http://git.reviewboard.kde.org/r/102238/diff
> 
> 
> Testing
> -------
> 
> Tested with the following code based on Albert's post. 
> 
> #include "hostinfo_p.h"
> #include <QtGui/QApplication>
> #include <QtCore/QElapsedTimer>
> #include <QtNetwork/QHostInfo>
> 
> int main(int a, char **b)
> {
>     QApplication app(a, b);
>     QElapsedTimer t;
>     t.start();
>     qDebug() << KIO::HostInfo::lookupHost("www.kde.org", 0).addresses();
>     qDebug() << "Time:" << t.elapsed() << "ms";
> }
> 
> 
> Thanks,
> 
> Dawit
> 
>

Reply via email to