<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/102238/">http://git.reviewboard.kde.org/r/102238/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
<br />
<p>- Albert</p>
<br />
<p>On August 12th, 2011, 3:45 a.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Aug. 12, 2011, 3:45 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Local unit testing code. Tested both failing and working DNS lookup scenarios.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kio/kio/hostinfo.cpp <span style="color: grey">(344b1d8)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102238/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>