[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: [PATCH] DNS cache for Konqueror/KIO
From: "Roland Harnau" <truthandprogress () googlemail ! com>
Date: 2008-07-07 14:30:27
Message-ID: 476f836a0807070730r7c6a84edod65701602ca7bdc6 () mail ! gmail ! com
[Download RAW message or body]
2008/7/2, Thiago Macieira <thiago@kde.org>:
> Ok, I have finally got my act together and reviewed the patch.
Thanks for the quite in-depth review. I have tried to address the
points you've raised in the attached patch.
[..]
> Why do we need a static function to return the instance to call
> lookupHost()? Can't lookupHost() be static instead?
There is a rather stylistic reason: if state is involved this should
be exposed to the user and not hidden by static methods (as in
KIO::Scheduler). But as the cache and the list of processed queries
(the state maintained by the HostInfoAgentPrivate instance) is
transparent to the user, this argument is not really conclusive and I
have therefore reduced the HostInfoAgent class to a mere function.
[..]
> The same in:
> + if (openQueries.contains(hostName)) {
> + Query* query = openQueries.value(hostName);
>
> Please use an iterator and compare to openQueries.end() to see if the
> object is there.
openQueries has type QHash<QString, Query*> and so I've replaced the
double lookup (which is not really hurting because of O(1) complexity
in the average case) by a test for 0. It is somewhat surprising that
your approach is apparently only available for the STL iterator
interface but not for Java-style iterators.
> In all, your patch is very good and very clever. The only drawback (=
> possible future improvement) is the fact that the IP addresses will be
> tried always in the same order. Right now, the DNS server will change the
> order in a round-robin fashion at every request.
According to [1] the addreses returned by getaddrinfo() (glibc
version) are ordered:
"The important thing to remember here is that the addresses are
returned in an order where the first returned address has the highest
probability to succeed."
[1] http://people.redhat.com/drepper/userapi-ipv6.html
Roland
["kdelibs-DNS-cache.patch" (text/x-patch)]
diff --git a/kio/CMakeLists.txt b/kio/CMakeLists.txt
index 0e6f149..d0b4ee0 100644
--- a/kio/CMakeLists.txt
+++ b/kio/CMakeLists.txt
@@ -116,6 +116,7 @@ set(kiocore_STAT_SRCS
kio/slaveinterface.cpp
kio/tcpslavebase.cpp
kio/udsentry.cpp
+ kio/hostinfo.cpp
)
qt4_add_dbus_adaptor(kiocore_STAT_SRCS kio/org.kde.kio.FileUndoManager.xml \
fileundomanager_p.h KIO::FileUndoManagerPrivate fileundomanager_adaptor \
KIOFileUndoManagerAdaptor)
diff --git a/kio/kio/global.h b/kio/kio/global.h
index b1fcbc5..4e5d10b 100644
--- a/kio/kio/global.h
+++ b/kio/kio/global.h
@@ -177,7 +177,8 @@ namespace KIO
CMD_READ = 'Z', // 90
CMD_WRITE = 91,
CMD_SEEK = 92,
- CMD_CLOSE = 93
+ CMD_CLOSE = 93,
+ CMD_HOST_INFO = 94
// Add new ones here once a release is done, to avoid breaking binary \
compatibility.
// Note that protocol-specific commands shouldn't be added here, but should use \
special. };
diff --git a/kio/kio/hostinfo.cpp b/kio/kio/hostinfo.cpp
new file mode 100644
index 0000000..528a8cc
--- /dev/null
+++ b/kio/kio/hostinfo.cpp
@@ -0,0 +1,144 @@
+/*
+Copyright 2008 Roland Harnau <tau@gmx.eu>
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License as published by the Free Software Foundation; either
+version 2.1 of the License, or (at your option) version 3, or any
+later version accepted by the membership of KDE e.V. (or its
+successor approved by the membership of KDE e.V.), which shall
+act as a proxy defined in Section 6 of version 3 of the license.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "hostinfo_p.h"
+
+#include <kglobal.h>
+#include <QtCore/QString>
+#include <QtCore/QHash>
+#include <QtCore/QCache>
+#include <QtCore/QTime>
+#include <QtCore/QList>
+#include <QtCore/QPair>
+#include <QtCore/QFutureWatcher>
+#include <QtCore/QtConcurrentRun>
+#include <QtNetwork/QHostInfo>
+
+#define TTL 300
+
+namespace KIO
+{
+ class HostInfoAgentPrivate : public QObject
+ {
+ Q_OBJECT
+ public:
+ HostInfoAgentPrivate(int cacheSize = 100);
+ virtual ~HostInfoAgentPrivate() {};
+ void lookupHost(const QString& hostName, QObject* receiver, const char* \
member); + private slots:
+ void queryFinished(const QHostInfo&);
+ private:
+ class Result;
+ class Query;
+
+ QHash<QString, Query*> openQueries;
+ QCache<QString, QPair<QHostInfo, QTime> > dnsCache;
+ };
+
+ class HostInfoAgentPrivate::Result : public QObject
+ {
+ Q_OBJECT
+ signals:
+ void result(QHostInfo);
+ private:
+ friend class HostInfoAgentPrivate;
+ };
+
+ class HostInfoAgentPrivate::Query : public QObject
+ {
+ Q_OBJECT
+ public:
+ Query(): m_watcher(), m_hostName()
+ {
+ connect(&m_watcher, SIGNAL(finished()), this, SLOT(relayFinished()));
+ }
+ void start(const QString& hostName)
+ {
+ m_hostName = hostName;
+ QFuture<QHostInfo> future = QtConcurrent::run(&QHostInfo::fromName, \
hostName); + m_watcher.setFuture(future);
+ }
+ QString hostName() const
+ {
+ return m_hostName;
+ }
+ signals:
+ void result(QHostInfo);
+ private slots:
+ void relayFinished()
+ {
+ emit result(m_watcher.result());
+ }
+ private:
+ QFutureWatcher<QHostInfo> m_watcher;
+ QString m_hostName;
+ };
+}
+
+using namespace KIO;
+
+K_GLOBAL_STATIC(HostInfoAgentPrivate, hostInfoAgentPrivate);
+
+void HostInfo::lookupHost(const QString& hostName, QObject* receiver,
+ const char* member)
+{
+ hostInfoAgentPrivate->lookupHost(hostName, receiver, member);
+}
+
+HostInfoAgentPrivate::HostInfoAgentPrivate(int cacheSize) : openQueries(),
+ dnsCache(cacheSize) {}
+
+void HostInfoAgentPrivate::lookupHost(const QString& hostName,
+ QObject* receiver, const char* member)
+{
+ if (QPair<QHostInfo, QTime>* info = dnsCache.object(hostName)) {
+ if (QTime::currentTime() <= info->second.addSecs(TTL)) {
+ Result result;
+ QObject::connect(&result, SIGNAL(result(QHostInfo)),receiver, member);
+ emit result.result(info->first);
+ return;
+ }
+ dnsCache.remove(hostName);
+ }
+
+ if (Query* query = openQueries.value(hostName)) {
+ connect(query, SIGNAL(result(QHostInfo)), receiver, member);
+ return;
+ }
+
+ Query* query = new Query();
+ openQueries.insert(hostName, query);
+ connect(query, SIGNAL(result(QHostInfo)), this, SLOT(queryFinished(QHostInfo)));
+ connect(query, SIGNAL(result(QHostInfo)), receiver, member);
+ query->start(hostName);
+}
+
+void HostInfoAgentPrivate::queryFinished(const QHostInfo& info)
+{
+ Query* query = static_cast<Query* >(sender());
+ openQueries.remove(query->hostName());
+ if (info.error() == QHostInfo::NoError) {
+ dnsCache.insert(query->hostName(),
+ new QPair<QHostInfo, QTime>(info, QTime::currentTime()));
+ }
+ query->deleteLater();
+}
+
+#include "hostinfo.moc"
diff --git a/kio/kio/hostinfo_p.h b/kio/kio/hostinfo_p.h
new file mode 100644
index 0000000..a073074
--- /dev/null
+++ b/kio/kio/hostinfo_p.h
@@ -0,0 +1,35 @@
+/*
+Copyright 2008 Roland Harnau <tau@gmx.eu>
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License as published by the Free Software Foundation; either
+version 2.1 of the License, or (at your option) version 3, or any
+later version accepted by the membership of KDE e.V. (or its
+successor approved by the membership of KDE e.V.), which shall
+act as a proxy defined in Section 6 of version 3 of the license.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef HOSTINFO_H_
+#define HOSTINFO_H_
+
+#include <QtCore/QString>
+#include <QtCore/QObject>
+
+namespace KIO
+{
+ namespace HostInfo
+ {
+ void lookupHost(const QString& hostName, QObject* receiver, const char* \
member); + }
+}
+
+#endif
diff --git a/kio/kio/slavebase.cpp b/kio/kio/slavebase.cpp
index 59e4dfb..4dc150c 100644
--- a/kio/kio/slavebase.cpp
+++ b/kio/kio/slavebase.cpp
@@ -1335,3 +1335,36 @@ void SlaveBase::send(int cmd, const QByteArray& arr )
void SlaveBase::virtual_hook( int, void* )
{ /*BASE::virtual_hook( id, data );*/ }
+
+void SlaveBase::lookupHost(const QString& host)
+{
+ KIO_DATA << host;
+ send(MSG_HOST_INFO_REQ, data);
+}
+
+int SlaveBase::waitForHostInfo(QHostInfo& info)
+{
+ QByteArray data;
+ int result = waitForAnswer(CMD_HOST_INFO, 0, data);
+
+ if (result == -1) {
+ info.setError(QHostInfo::UnknownError);
+ info.setErrorString("Unknown Error.");
+ return result;
+ }
+
+ QDataStream stream(data);
+ QString hostName;
+ QList<QHostAddress> addresses;
+ int error;
+ QString errorString;
+
+ stream >> hostName >> addresses >> error >> errorString;
+
+ info.setHostName(hostName);
+ info.setAddresses(addresses);
+ info.setError(QHostInfo::HostInfoError(error));
+ info.setErrorString(errorString);
+
+ return result;
+}
diff --git a/kio/kio/slavebase.h b/kio/kio/slavebase.h
index 25cc148..d638652 100644
--- a/kio/kio/slavebase.h
+++ b/kio/kio/slavebase.h
@@ -27,6 +27,7 @@
#include <klocale.h>
#include <QtCore/QByteArray>
+#include <QtNetwork/QHostInfo>
class KConfigGroup;
class KRemoteEncoding;
@@ -790,6 +791,16 @@ public:
*/
void setKillFlag();
+ /** Internally used
+ * @internal
+ */
+ void lookupHost(const QString& host);
+
+ /** Internally used
+ * @internal
+ */
+ int waitForHostInfo(QHostInfo& info);
+
protected:
/**
* Name of the protocol supported by this slave
diff --git a/kio/kio/slaveinterface.cpp b/kio/kio/slaveinterface.cpp
index 8e41ea7..da18635 100644
--- a/kio/kio/slaveinterface.cpp
+++ b/kio/kio/slaveinterface.cpp
@@ -21,6 +21,7 @@
#include "slavebase.h"
#include "connection.h"
+#include "hostinfo_p.h"
#include <errno.h>
#include <assert.h>
#include <kdebug.h>
@@ -336,7 +337,13 @@ bool SlaveInterface::dispatch( int _cmd, const QByteArray \
&rawdata ) emit needSubUrlData();
break;
}
- default:
+ case MSG_HOST_INFO_REQ: {
+ QString hostName;
+ stream >> hostName;
+ HostInfo::lookupHost(hostName, this, SLOT(slotHostInfo(QHostInfo)));
+ break;
+ }
+ default:
kWarning(7007) << "Slave sends unknown command (" << _cmd << "), dropping \
slave"; return false;
}
@@ -496,4 +503,12 @@ int SlaveInterfacePrivate::messageBox(int type, const QString \
&text, return result;
}
+void SlaveInterfacePrivate::slotHostInfo(const QHostInfo& info)
+{
+ QByteArray data;
+ QDataStream stream(&data, QIODevice::WriteOnly);
+ stream << info.hostName() << info.addresses() << info.error() << \
info.errorString(); + connection->send(CMD_HOST_INFO, data);
+}
+
#include "slaveinterface.moc"
diff --git a/kio/kio/slaveinterface.h b/kio/kio/slaveinterface.h
index d17c5c2..3cdc2ae 100644
--- a/kio/kio/slaveinterface.h
+++ b/kio/kio/slaveinterface.h
@@ -24,6 +24,7 @@
#include <sys/types.h>
#include <QtCore/QObject>
+#include <QtNetwork/QHostInfo>
#include <kio/global.h>
#include <kio/udsentry.h>
@@ -82,7 +83,8 @@ class SlaveInterfacePrivate;
MSG_AUTH_KEY, // 115 // deprecated.
MSG_DEL_AUTH_KEY, // deprecated.
MSG_OPENED,
- MSG_WRITTEN
+ MSG_WRITTEN,
+ MSG_HOST_INFO_REQ
// add new ones here once a release is done, to avoid breaking binary \
compatibility };
@@ -169,10 +171,11 @@ protected:
protected Q_SLOTS:
void calcSpeed();
-
protected:
SlaveInterfacePrivate* const d_ptr;
Q_DECLARE_PRIVATE(SlaveInterface)
+private:
+ Q_PRIVATE_SLOT(d_func(), void slotHostInfo(QHostInfo))
};
}
diff --git a/kio/kio/slaveinterface_p.h b/kio/kio/slaveinterface_p.h
index 08abbc7..19ad7ba 100644
--- a/kio/kio/slaveinterface_p.h
+++ b/kio/kio/slaveinterface_p.h
@@ -21,7 +21,8 @@
#include "global.h"
#include "connection.h"
-#include <QTimer>
+#include <QtCore/QTimer>
+#include <QtNetwork/QHostInfo>
static const unsigned int max_nums = 8;
@@ -58,6 +59,7 @@ public:
int messageBox(int type, const QString &text, const QString &caption,
const QString &buttonYes, const QString &buttonNo, const QString \
&dontAskAgainName);
+ void slotHostInfo(const QHostInfo& info);
};
#endif
diff --git a/kio/kio/tcpslavebase.cpp b/kio/kio/tcpslavebase.cpp
index a9f1abf..2e10920 100644
--- a/kio/kio/tcpslavebase.cpp
+++ b/kio/kio/tcpslavebase.cpp
@@ -2,7 +2,8 @@
* Copyright (C) 2000 Alex Zepeda <zipzippy@sonic.net
* Copyright (C) 2001-2003 George Staikos <staikos@kde.org>
* Copyright (C) 2001 Dawit Alemayehu <adawit@kde.org>
- * Copyright (C) 2007,2008 Andreas Hartmetz <ahartmetz@gmail.com>
+ * Copyright (C) 2007,2008 Andreas Hartmetz <ahartmetz@gmail.com>
+ * Copyright (C) 2008 Roland Harnau <tau@gmx.eu>
*
* This file is part of the KDE project
*
@@ -46,6 +47,7 @@
#include <klocale.h>
#include <QtCore/QDataStream>
+#include <QtCore/QTime>
#include <QtNetwork/QTcpSocket>
#include <QtNetwork/QHostInfo>
#include <QtDBus/QtDBus>
@@ -271,7 +273,6 @@ bool TCPSlaveBase::connectToHost(const QString &/*protocol*/,
}
}
-
KTcpSocket::SslVersion trySslVersion = KTcpSocket::TlsV1;
while (true) {
disconnectFromHost(); //Reset some state, even if we are already \
disconnected @@ -279,26 +280,37 @@ bool TCPSlaveBase::connectToHost(const QString \
&/*protocol*/,
//FIXME! KTcpSocket doesn't know or care about protocol ports! Fix it there, \
then use it here.
- kDebug(7029) << "before connectToHost: Socket error is"
- << d->socket.error() << ", Socket state is" << \
d->socket.state();
- d->socket.connectToHost(host, port);
- kDebug(7029) << "after connectToHost: Socket error is"
- << d->socket.error() << ", Socket state is" << \
d->socket.state();
-
- bool connectOk = d->socket.waitForConnected(d->timeout > -1 ? d->timeout * \
1000 : -1);
- kDebug(7029) << "after waitForConnected: Socket error is"
- << d->socket.error() << ", Socket state is" << d->socket.state()
- << ", waitForConnected returned " << connectOk;
-
- if (d->socket.state() != KTcpSocket::ConnectedState) {
- if (d->socket.error() == KTcpSocket::HostNotFoundError) {
- error(ERR_UNKNOWN_HOST,
- host + QLatin1String(": ") + d->socket.errorString());
- } else {
+ QList<QHostAddress> addresses;
+
+ QHostAddress address;
+ if (address.setAddress(host)) {
+ addresses.append(address);
+ } else {
+ QHostInfo info;
+ lookupHost(host);
+ waitForHostInfo(info);
+ if (info.error() != QHostInfo::NoError) {
+ error(ERR_UNKNOWN_HOST, info.errorString());
+ return false;
+ }
+ addresses = info.addresses();
+ }
+
+ QListIterator<QHostAddress> it(addresses);
+ int timeout = d->timeout * 1000;
+ QTime time;
+ time.start();
+ forever {
+ d->socket.connectToHost(it.next(), port);
+ if (d->socket.waitForConnected(timeout)) {
+ break;
+ }
+ timeout =- time.elapsed();
+ if (!it.hasNext() || (timeout < 0)) {
error(ERR_COULD_NOT_CONNECT,
- host + QLatin1String(": ") + d->socket.errorString());
+ host + QLatin1String(": ") + d->socket.errorString());
+ return false;
}
- return false;
}
//### check for proxyAuthenticationRequiredError
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic