-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102238/#review5696
-----------------------------------------------------------
When using your code with my simple test program
#include "hostinfo_p.h"
#include <QApplication>
#include <QHostInfo>
#include <QTime>
int main(int a, char **b)
{
QApplication app(a, b);
QTime t;
t.start();
qDebug() << KIO::HostInfo::lookupHost("www.google.com", 1).addresses();
qDebug() << t.elapsed();
}
I get
a.out(4386) KIO::HostInfo::lookupHost: Waiting one second for name lookup
thread to start
a.out(4386) KIO::HostInfo::lookupHost: Waiting one second for name lookup
thread to start
a.out(4386) KIO::HostInfo::lookupHost: Waiting one second for name lookup
thread to start
a.out(4386) KIO::HostInfo::lookupHost: Waiting one second for name lookup
thread to start
a.out(4386) KIO::HostInfo::lookupHost: Waiting one second for name lookup
thread to start
a.out(4386) KIO::HostInfo::lookupHost: Failed to start name lookup thread for
"www.google.com"
()
55
kDebugStream called after destruction (from virtual
KIO::NameLookUpThread::~NameLookUpThread() file kdelibs/kio/kio/hostinfo.cpp
line 125)
still running ? false
The kDebugStream line looks ugly and the waiting one second for name lookup are
wrong too since what actually has happened is that the thread already finished
so the name lookup worked
I am also concerned about you accessing the cache from the main thread and from
the helper thread without a lock (it could cause a crash i think)
- Albert
On Aug. 12, 2011, 3:45 a.m., Dawit Alemayehu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102238/
> -----------------------------------------------------------
>
> (Updated Aug. 12, 2011, 3:45 a.m.)
>
>
> Review request for kdelibs.
>
>
> 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
> -------
>
> Local unit testing code. Tested both failing and working DNS lookup scenarios.
>
>
> Thanks,
>
> Dawit
>
>
