* NetDB: Prevent ExpireLeaseJob NPE (thanks sponge)

history.txt

@@ -1,3 +1,9 @@
+2011-02-13 zzz
+    * Connect Client: Minor NPE fix cleanup
+    * JobQueue: Prevet NPE at shutdown (thanks liberty)
+    * GeoIP: Prevent startup NPE (ticket #413, thanks RN)
+    * NetDB: Prevent ExpireLeaseJob NPE (thanks sponge)
+
 2011-02-11 Mathiasdm
     * routerconsole: fixed graphs using jrobin; and headless issue
           in general: no more switches between headless and non-headless.

router/java/src/net/i2p/router/RouterVersion.java

@@ -18,7 +18,7 @@ public class RouterVersion {
     /** deprecated */
     public final static String ID = "Monotone";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 9;
+    public final static long BUILD = 10;
 
     /** for example "-test" */
     public final static String EXTRA = "";

router/java/src/net/i2p/router/networkdb/kademlia/ExpireLeasesJob.java

@@ -9,7 +9,7 @@ package net.i2p.router.networkdb.kademlia;
  */
 
 import java.util.HashSet;
-import java.util.Iterator;
+import java.util.Map;
 import java.util.Set;
 
 import net.i2p.data.DatabaseEntry;
@@ -27,8 +27,8 @@ import net.i2p.util.Log;
  *
  */
 class ExpireLeasesJob extends JobImpl {
-    private Log _log;
+    private final Log _log;
-    private KademliaNetworkDatabaseFacade _facade;
+    private final KademliaNetworkDatabaseFacade _facade;
     
     private final static long RERUN_DELAY_MS = 1*60*1000;
     
@@ -39,11 +39,11 @@ class ExpireLeasesJob extends JobImpl {
     }
     
     public String getName() { return "Expire Lease Sets Job"; }
+
     public void runJob() {
-        Set toExpire = selectKeysToExpire();
+        Set<Hash> toExpire = selectKeysToExpire();
         _log.info("Leases to expire: " + toExpire);
-        for (Iterator iter = toExpire.iterator(); iter.hasNext(); ) {
+        for (Hash key : toExpire) {
-            Hash key = (Hash)iter.next();
             _facade.fail(key);
             //_log.info("Lease " + key + " is expiring, so lets look for it again", new Exception("Expire and search"));
             //_facade.lookupLeaseSet(key, null, null, RERUN_DELAY_MS);
@@ -57,17 +57,15 @@ class ExpireLeasesJob extends JobImpl {
      * don't have any leases that haven't yet passed, even with the CLOCK_FUDGE_FACTOR)
      *
      */
-    private Set selectKeysToExpire() {
+    private Set<Hash> selectKeysToExpire() {
-        Set keys = _facade.getDataStore().getKeys();
+        Set<Hash> toExpire = new HashSet(128);
-        Set toExpire = new HashSet(128);
+        for (Map.Entry<Hash, DatabaseEntry> entry : _facade.getDataStore().getMapEntries()) {
-        for (Iterator iter = keys.iterator(); iter.hasNext(); ) {
+            DatabaseEntry obj = entry.getValue();
-            Hash key = (Hash)iter.next();
-            DatabaseEntry obj = _facade.getDataStore().get(key);
             if (obj.getType() == DatabaseEntry.KEY_TYPE_LEASESET) {
                 LeaseSet ls = (LeaseSet)obj;
                 if (!ls.isCurrent(Router.CLOCK_FUDGE_FACTOR))
-                    toExpire.add(key);
+                    toExpire.add(entry.getKey());
-                else
+                else if (_log.shouldLog(Log.DEBUG))
                     _log.debug("Lease " + ls.getDestination().calculateHash() + " is current, no need to expire");
             }
         }

router/java/src/net/i2p/router/networkdb/kademlia/ExpireRoutersJob.java

@@ -9,7 +9,6 @@ package net.i2p.router.networkdb.kademlia;
  */
 
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.Set;
 
 import net.i2p.data.Hash;
@@ -28,8 +27,8 @@ import net.i2p.util.Log;
  *
  */
 class ExpireRoutersJob extends JobImpl {
-    private Log _log;
+    private final Log _log;
-    private KademliaNetworkDatabaseFacade _facade;
+    private final KademliaNetworkDatabaseFacade _facade;
     
     /** rerun fairly often, so the fails don't queue up too many netdb searches at once */
     private final static long RERUN_DELAY_MS = 120*1000;
@@ -41,11 +40,13 @@ class ExpireRoutersJob extends JobImpl {
     }
     
     public String getName() { return "Expire Routers Job"; }
+
     public void runJob() {
-        Set toExpire = selectKeysToExpire();
+        // this always returns an empty set (see below)
+        Set<Hash> toExpire = selectKeysToExpire();
+        if (_log.shouldLog(Log.INFO))
             _log.info("Routers to expire (drop and try to refetch): " + toExpire);
-        for (Iterator iter = toExpire.iterator(); iter.hasNext(); ) {
+        for (Hash key : toExpire) {
-            Hash key = (Hash)iter.next();
             _facade.fail(key);
         }
         _facade.queueForExploration(toExpire);
@@ -61,9 +62,8 @@ class ExpireRoutersJob extends JobImpl {
      *
      * @return nothing for now
      */
-    private Set selectKeysToExpire() {
+    private Set<Hash> selectKeysToExpire() {
-        for (Iterator iter = _facade.getAllRouters().iterator(); iter.hasNext(); ) {
+        for (Hash key : _facade.getAllRouters()) {
-            Hash key = (Hash)iter.next();
             // Don't expire anybody we are connected to
             if (!getContext().commSystem().isEstablished(key)) {
                 // This does a _facade.validate() and fail() which is sufficient...