<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 14th, 2011, 11:02 p.m., <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> kDebugStream called after destruction (from virtual KIO::NameLookUpThread::~NameLookUpThread() file kdelibs/kio/kio/hostinfo.cpp line 125)
> still running ? false
That is there only for debugging session and was not meant to be left around.
> 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.
Actually the reason you encountered the error aboveis because I accidentally left the "m_started = false" at the end of the NameLookUpThread::run class. As such the fix is very easy. Remove that line. It was left in by mistake when I was experimenting with things. The same goes for the debug statements. They are there for the testing purposes only and can be easily commented out.
> 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)
huh ? The helper thread is waited upon to complete its job before any code in the main thread accesses the case ; so I fail to see how this can possibly result in what you are stating here.
</pre>
<br />
<p>- Dawit</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>