[tor-commits] [onionoo/master] Always clear old reverse DNS lookups.

[karsten at torproject.org]

commit 6faa1ab757f16263f7925dbe4c49d6dfb698f265
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date:   Thu Oct 3 16:43:21 2019 +0200

    Always clear old reverse DNS lookups.
    
    The fix is to distinguish between lookup results being null for a
    given fingerprint (because we didn't have enough time to look up an
    address) and the empty set (because we received 0 verified or 0
    unverified host names in response).
    
    Fixes #31941.
---
 CHANGELOG.md                                               |  9 ++++++++-
 .../metrics/onionoo/updater/NodeDetailsStatusUpdater.java  |  4 ++--
 .../metrics/onionoo/updater/RdnsLookupRequest.java         | 14 +++++++-------
 .../metrics/onionoo/updater/RdnsLookupWorker.java          |  4 ++--
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 696b33d..3ab6329 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,11 @@
-# Changes in version 7.0-1.20.0 - 2019-0?-??
+# Changes in version 7.0-1.21.0 - 2019-1?-??
+
+ * Medium changes
+   - Fix a bug where old reverse DNS lookups were sometimes not
+     cleared properly.
+
+
+# Changes in version 7.0-1.20.0 - 2019-10-04
 
  * Medium changes
    - Use Ivy for resolving external dependencies rather than relying
diff --git a/src/main/java/org/torproject/metrics/onionoo/updater/NodeDetailsStatusUpdater.java b/src/main/java/org/torproject/metrics/onionoo/updater/NodeDetailsStatusUpdater.java
index 2bdf03f..a5f16e4 100644
--- a/src/main/java/org/torproject/metrics/onionoo/updater/NodeDetailsStatusUpdater.java
+++ b/src/main/java/org/torproject/metrics/onionoo/updater/NodeDetailsStatusUpdater.java
@@ -771,10 +771,10 @@ public class NodeDetailsStatusUpdater implements DescriptorListener,
               verifiedLookupResults.get(nodeStatus.getAddress());
       SortedSet<String> unverifiedHostNames =
               unverifiedLookupResults.get(nodeStatus.getAddress());
-      if (null != verifiedHostNames && !verifiedHostNames.isEmpty()) {
+      if (null != verifiedHostNames) {
         this.rdnsVerifiedLookupResults.put(fingerprint, verifiedHostNames);
       }
-      if (null != unverifiedHostNames && !unverifiedHostNames.isEmpty()) {
+      if (null != unverifiedHostNames) {
         this.rdnsUnverifiedLookupResults.put(fingerprint, unverifiedHostNames);
       }
     }
diff --git a/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupRequest.java b/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupRequest.java
index 2f598a4..53e3fa9 100644
--- a/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupRequest.java
+++ b/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupRequest.java
@@ -39,9 +39,9 @@ class RdnsLookupRequest extends Thread {
   @Override
   public void run() {
     this.lookupStartedMillis = System.currentTimeMillis();
+    final SortedSet<String> verifiedResults = new TreeSet<>();
+    final SortedSet<String> unverifiedResults = new TreeSet<>();
     try {
-      final SortedSet<String> verifiedResults = new TreeSet<>();
-      final SortedSet<String> unverifiedResults = new TreeSet<>();
       final String[] bytes = this.address.split("\\.");
       if (bytes.length == 4) {
         final String reverseDnsDomain =
@@ -69,15 +69,15 @@ class RdnsLookupRequest extends Thread {
           }
         }
       }
-      synchronized (this) {
-        this.verifiedHostNames = verifiedResults;
-        this.unverifiedHostNames = unverifiedResults;
-      }
     } catch (NamingException e) {
       /* The Onionoo field is omitted for both lookup failure and absence of
        * a host name. We'll try again the next time. */
     }
-    this.lookupCompletedMillis = System.currentTimeMillis();
+    synchronized (this) {
+      this.verifiedHostNames = verifiedResults;
+      this.unverifiedHostNames = unverifiedResults;
+      this.lookupCompletedMillis = System.currentTimeMillis();
+    }
     this.parent.interrupt();
   }
 
diff --git a/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupWorker.java b/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupWorker.java
index ab5360e..6353daa 100644
--- a/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupWorker.java
+++ b/src/main/java/org/torproject/metrics/onionoo/updater/RdnsLookupWorker.java
@@ -40,7 +40,7 @@ class RdnsLookupWorker extends Thread {
         /* Getting interrupted should be the default case. */
       }
       SortedSet<String> verifiedHostNames = request.getVerifiedHostNames();
-      if (null != verifiedHostNames && !verifiedHostNames.isEmpty()) {
+      if (null != verifiedHostNames) {
         synchronized (this.reverseDomainNameResolver
             .rdnsVerifiedLookupResults) {
           this.reverseDomainNameResolver.rdnsVerifiedLookupResults.put(
@@ -48,7 +48,7 @@ class RdnsLookupWorker extends Thread {
         }
       }
       SortedSet<String> unverifiedHostNames = request.getUnverifiedHostNames();
-      if (null != unverifiedHostNames && !unverifiedHostNames.isEmpty()) {
+      if (null != unverifiedHostNames) {
         synchronized (this.reverseDomainNameResolver
             .rdnsUnverifiedLookupResults) {
           this.reverseDomainNameResolver.rdnsUnverifiedLookupResults.put(