> On Aug. 14, 2011, 11:02 p.m., Albert Astals Cid wrote:
> > 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)
> 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.
- Dawit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102238/#review5696
-----------------------------------------------------------
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
>
>
