Merge pull request #11950 from Chocobo1/rdns

Improve ReverseResolution class

src/base/net/reverseresolution.cpp

@@ -28,19 +28,19 @@
 
 #include "reverseresolution.h"
 
-#include <QDebug>
+#include <QHostAddress>
 #include <QHostInfo>
 #include <QString>
 
-const int CACHE_SIZE = 500;
+const int CACHE_SIZE = 2048;
 
 using namespace Net;
 
 namespace
 {
-    bool isUsefulHostName(const QString &hostname, const QString &ip)
+    bool isUsefulHostName(const QString &hostname, const QHostAddress &ip)
     {
-        return (!hostname.isEmpty() && (hostname != ip));
+        return (!hostname.isEmpty() && (hostname != ip.toString()));
     }
 }
 
@@ -52,37 +52,36 @@ ReverseResolution::ReverseResolution(QObject *parent)
 
 ReverseResolution::~ReverseResolution()
 {
-    qDebug("Deleting host name resolver...");
+    // abort on-going lookups instead of waiting them
+    for (auto iter = m_lookups.cbegin(); iter != m_lookups.cend(); ++iter)
+        QHostInfo::abortHostLookup(iter.key());
 }
 
-void ReverseResolution::resolve(const QString &ip)
+void ReverseResolution::resolve(const QHostAddress &ip)
 {
-    if (m_cache.contains(ip)) {
+    const QString *hostname = m_cache.object(ip);
-        const QString &hostname = *m_cache.object(ip);
+    if (hostname) {
-        qDebug() << "Resolved host name using cache: " << ip << " -> " << hostname;
+        emit ipResolved(ip, *hostname);
-        if (isUsefulHostName(hostname, ip))
+        return;
-            emit ipResolved(ip, hostname);
-    }
-    else {
-        // Actually resolve the ip
-        m_lookups.insert(QHostInfo::lookupHost(ip, this, &ReverseResolution::hostResolved), ip);
     }
+
+    // do reverse lookup: IP -> hostname
+    const int lookupId = QHostInfo::lookupHost(ip.toString(), this, &ReverseResolution::hostResolved);
+    m_lookups.insert(lookupId, ip);
 }
 
 void ReverseResolution::hostResolved(const QHostInfo &host)
 {
-    const QString ip = m_lookups.take(host.lookupId());
+    const QHostAddress ip = m_lookups.take(host.lookupId());
-    Q_ASSERT(!ip.isNull());
 
     if (host.error() != QHostInfo::NoError) {
-        qDebug() << "DNS Reverse resolution error: " << host.errorString();
+        emit ipResolved(ip, {});
         return;
     }
 
-    const QString hostname = host.hostName();
+    const QString hostname = isUsefulHostName(host.hostName(), ip)
-
+        ? host.hostName()
-    qDebug() << Q_FUNC_INFO << ip << QString("->") << hostname;
+        : QString();
     m_cache.insert(ip, new QString(hostname));
-    if (isUsefulHostName(hostname, ip))
+    emit ipResolved(ip, hostname);
-        emit ipResolved(ip, hostname);
 }

src/base/net/reverseresolution.h

@@ -32,6 +32,7 @@
 #include <QCache>
 #include <QObject>
 
+class QHostAddress;
 class QHostInfo;
 class QString;
 
@@ -46,17 +47,17 @@ namespace Net
         explicit ReverseResolution(QObject *parent = nullptr);
         ~ReverseResolution();
 
-        void resolve(const QString &ip);
+        void resolve(const QHostAddress &ip);
 
     signals:
-        void ipResolved(const QString &ip, const QString &hostname);
+        void ipResolved(const QHostAddress &ip, const QString &hostname);
 
     private slots:
         void hostResolved(const QHostInfo &host);
 
     private:
-        QHash<int /* LookupID */, QString /* IP */> m_lookups;
+        QHash<int, QHostAddress> m_lookups;  // <LookupID, IP>
-        QCache<QString /* IP */, QString /* HostName */> m_cache;
+        QCache<QHostAddress, QString> m_cache;  // <IP, HostName>
     };
 }
 

src/gui/properties/peerlistwidget.cpp

@@ -33,6 +33,7 @@
 #include <QApplication>
 #include <QClipboard>
 #include <QHeaderView>
+#include <QHostAddress>
 #include <QMenu>
 #include <QMessageBox>
 #include <QSet>
@@ -327,6 +328,7 @@ void PeerListWidget::copySelectedPeers()
 void PeerListWidget::clear()
 {
     m_peerItems.clear();
+    m_itemsByIP.clear();
     const int nbrows = m_listModel->rowCount();
     if (nbrows > 0)
         m_listModel->removeRows(0, nbrows);
@@ -364,7 +366,13 @@ void PeerListWidget::loadPeers(const BitTorrent::TorrentHandle *torrent)
 
     // Remove peers that are gone
     for (const PeerEndpoint &peerEndpoint : asConst(existingPeers)) {
-        const QStandardItem *item = m_peerItems.take(peerEndpoint);
+        QStandardItem *item = m_peerItems.take(peerEndpoint);
+
+        QSet<QStandardItem *> &items = m_itemsByIP[peerEndpoint.address.ip];
+        items.remove(item);
+        if (items.isEmpty())
+            m_itemsByIP.remove(peerEndpoint.address.ip);
+
         m_listModel->removeRow(item->row());
     }
 }
@@ -386,6 +394,7 @@ void PeerListWidget::updatePeer(const BitTorrent::TorrentHandle *torrent, const
         m_listModel->setData(m_listModel->index(row, PeerListDelegate::IP_HIDDEN), peerIp);
 
         itemIter = m_peerItems.insert(peerEndpoint, m_listModel->item(row, PeerListDelegate::IP));
+        m_itemsByIP[peerEndpoint.address.ip].insert(itemIter.value());
     }
 
     const int row = (*itemIter)->row();
@@ -405,7 +414,7 @@ void PeerListWidget::updatePeer(const BitTorrent::TorrentHandle *torrent, const
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::DOWNLOADING_PIECE), downloadingFiles.join('\n'), Qt::ToolTipRole);
 
     if (m_resolver)
-        m_resolver->resolve(peerIp);
+        m_resolver->resolve(peerEndpoint.address.ip);
 
     if (m_resolveCountries) {
         const QIcon icon = UIThemeManager::instance()->getFlagIcon(peer.country());
@@ -417,15 +426,14 @@ void PeerListWidget::updatePeer(const BitTorrent::TorrentHandle *torrent, const
     }
 }
 
-void PeerListWidget::handleResolved(const QString &ip, const QString &hostname)
+void PeerListWidget::handleResolved(const QHostAddress &ip, const QString &hostname) const
 {
-    for (auto iter = m_peerItems.cbegin(); iter != m_peerItems.cend(); ++iter) {
+    if (hostname.isEmpty())
-        if (iter.key().address.ip.toString() == ip) {
+        return;
-            QStandardItem *item {iter.value()};
+
-            item->setData(hostname, Qt::DisplayRole);
+    const QSet<QStandardItem *> items = m_itemsByIP.value(ip);
-            break;
+    for (QStandardItem *item : items)
-        }
+        item->setData(hostname, Qt::DisplayRole);
-    }
 }
 
 void PeerListWidget::handleSortColumnChanged(const int col)

src/gui/properties/peerlistwidget.h

@@ -30,8 +30,10 @@
 #define PEERLISTWIDGET_H
 
 #include <QHash>
+#include <QSet>
 #include <QTreeView>
 
+class QHostAddress;
 class QStandardItem;
 class QStandardItemModel;
 
@@ -72,7 +74,7 @@ private slots:
     void banSelectedPeers();
     void copySelectedPeers();
     void handleSortColumnChanged(int col);
-    void handleResolved(const QString &ip, const QString &hostname);
+    void handleResolved(const QHostAddress &ip, const QString &hostname) const;
 
 private:
     void updatePeer(const BitTorrent::TorrentHandle *torrent, const BitTorrent::PeerInfo &peer, bool &isNewPeer);
@@ -84,6 +86,7 @@ private:
     PropertiesWidget *m_properties = nullptr;
     Net::ReverseResolution *m_resolver = nullptr;
     QHash<PeerEndpoint, QStandardItem *> m_peerItems;
+    QHash<QHostAddress, QSet<QStandardItem *>> m_itemsByIP;  // must be kept in sync with `m_peerItems`
     bool m_resolveCountries;
 };