Fix #93: handle very long expirations without blocking others

src/main/java/net/jodah/expiringmap/ExpiringMap.java

@@ -74,10 +74,11 @@
  * @param <K> Key type
  * @param <V> Value type
  */
-public class ExpiringMap<K, V> implements ConcurrentMap<K, V> {
-  static volatile ScheduledExecutorService EXPIRER;
-  static volatile ThreadPoolExecutor LISTENER_SERVICE;
-  static ThreadFactory THREAD_FACTORY;
+public class ExpiringMap<K, V> implements ConcurrentMap<K, V> {
+  static volatile ScheduledExecutorService EXPIRER;
+  static volatile ThreadPoolExecutor LISTENER_SERVICE;
+  static ThreadFactory THREAD_FACTORY;
+  private static final long NO_EXPIRATION = Long.MAX_VALUE;
 
   List<ExpirationListener<K, V>> expirationListeners;
   List<ExpirationListener<K, V>> asyncExpirationListeners;
@@ -505,8 +506,8 @@ public final Map.Entry<K, V> next() {
     }
   }
 
-  /** Expiring map entry implementation. */
-  static class ExpiringEntry<K, V> implements Comparable<ExpiringEntry<K, V>> {
+  /** Expiring map entry implementation. */
+  static class ExpiringEntry<K, V> implements Comparable<ExpiringEntry<K, V>> {
     final AtomicLong expirationNanos;
     /** Epoch time at which the entry is expected to expire */
     final AtomicLong expectedExpiration;
@@ -596,10 +597,11 @@ synchronized V getValue() {
       return value;
     }
 
-    /** Resets the entry's expected expiration. */
-    void resetExpiration() {
-      expectedExpiration.set(expirationNanos.get() + System.nanoTime());
-    }
+    /** Resets the entry's expected expiration. */
+    void resetExpiration() {
+      long duration = expirationNanos.get();
+      expectedExpiration.set(duration == NO_EXPIRATION ? NO_EXPIRATION : safeAdd(System.nanoTime(), duration));
+    }
 
     /** Marks the entry as scheduled. */
     synchronized void schedule(Future<?> entryFuture) {
@@ -611,7 +613,14 @@ synchronized void schedule(Future<?> entryFuture) {
     synchronized void setValue(V value) {
       this.value = value;
     }
-  }
+  }
+
+  private static long safeAdd(long base, long delta) {
+    long result = base + delta;
+    if (((base ^ result) & (delta ^ result)) < 0)
+      return Long.MAX_VALUE;
+    return result;
+  }
 
   /**
    * Creates an ExpiringMap builder.
@@ -1346,20 +1355,24 @@ void resetEntry(ExpiringEntry<K, V> entry, boolean scheduleFirstEntry) {
    * 
    * @param entry Entry to schedule
    */
-  void scheduleEntry(ExpiringEntry<K, V> entry) {
-    if (entry == null || entry.scheduled)
-      return;
-
-    Runnable runnable = null;
-    synchronized (entry) {
-      if (entry.scheduled)
-        return;
-
-      final WeakReference<ExpiringEntry<K, V>> entryReference = new WeakReference<ExpiringEntry<K, V>>(entry);
-      runnable = new Runnable() {
-        @Override
-        public void run() {
-          ExpiringEntry<K, V> entry = entryReference.get();
+  void scheduleEntry(ExpiringEntry<K, V> entry) {
+    if (entry == null || entry.scheduled)
+      return;
+
+    Runnable runnable = null;
+    synchronized (entry) {
+      if (entry.scheduled)
+        return;
+
+      long expectedExpiration = entry.expectedExpiration.get();
+      if (expectedExpiration == NO_EXPIRATION)
+        return;
+
+      final WeakReference<ExpiringEntry<K, V>> entryReference = new WeakReference<ExpiringEntry<K, V>>(entry);
+      runnable = new Runnable() {
+        @Override
+        public void run() {
+          ExpiringEntry<K, V> entry = entryReference.get();
 
           writeLock.lock();
           try {
@@ -1388,14 +1401,19 @@ public void run() {
           } finally {
             writeLock.unlock();
           }
-        }
-      };
-
-      Future<?> entryFuture = EXPIRER.schedule(runnable, entry.expectedExpiration.get() - System.nanoTime(),
-          TimeUnit.NANOSECONDS);
-      entry.schedule(entryFuture);
-    }
-  }
+        }
+      };
+
+      long now = System.nanoTime();
+      long delay = expectedExpiration - now;
+      if (expectedExpiration <= now)
+        delay = 0;
+      else if (delay < 0)
+        delay = Long.MAX_VALUE;
+      Future<?> entryFuture = EXPIRER.schedule(runnable, delay, TimeUnit.NANOSECONDS);
+      entry.schedule(entryFuture);
+    }
+  }
 
   private static <K, V> Map.Entry<K, V> mapEntryFor(final ExpiringEntry<K, V> entry) {
     return new Map.Entry<K, V>() {

src/test/java/net/jodah/expiringmap/issues/Issue93.java

@@ -0,0 +1,52 @@
+package net.jodah.expiringmap.issues;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import java.util.concurrent.TimeUnit;
+
+import org.testng.annotations.Test;
+
+import net.jodah.expiringmap.ExpirationPolicy;
+import net.jodah.expiringmap.ExpiringMap;
+
+@Test
+public class Issue93 {
+  private interface Condition {
+    boolean test();
+  }
+
+  private static void awaitTrue(Condition condition, long timeoutMillis) throws InterruptedException {
+    long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMillis);
+    while (System.nanoTime() < deadline) {
+      if (condition.test())
+        return;
+      Thread.sleep(10);
+    }
+    assertTrue(condition.test(), "condition not met within " + timeoutMillis + "ms");
+  }
+
+  public void testVeryLongExpirationDoesNotBlockShortExpirations() throws Exception {
+    ExpiringMap<String, String> map = ExpiringMap.builder()
+        .variableExpiration()
+        .build();
+
+    map.put("A", "alpha", ExpirationPolicy.CREATED, Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+    map.put("B", "bravo", ExpirationPolicy.CREATED, 200, TimeUnit.MILLISECONDS);
+    map.put("C", "charlie", ExpirationPolicy.CREATED, 400, TimeUnit.MILLISECONDS);
+
+    awaitTrue(new Condition() {
+      public boolean test() {
+        return map.get("B") == null;
+      }
+    }, 2000);
+    assertEquals(map.get("A"), "alpha");
+
+    awaitTrue(new Condition() {
+      public boolean test() {
+        return map.get("C") == null;
+      }
+    }, 2000);
+    assertEquals(map.get("A"), "alpha");
+  }
+}