diff --git a/src/main/java/net/jodah/expiringmap/ExpiringMap.java b/src/main/java/net/jodah/expiringmap/ExpiringMap.java index 26dc4e9..dd0d0e8 100644 --- a/src/main/java/net/jodah/expiringmap/ExpiringMap.java +++ b/src/main/java/net/jodah/expiringmap/ExpiringMap.java @@ -74,10 +74,11 @@ * @param Key type * @param Value type */ -public class ExpiringMap implements ConcurrentMap { - static volatile ScheduledExecutorService EXPIRER; - static volatile ThreadPoolExecutor LISTENER_SERVICE; - static ThreadFactory THREAD_FACTORY; +public class ExpiringMap implements ConcurrentMap { + static volatile ScheduledExecutorService EXPIRER; + static volatile ThreadPoolExecutor LISTENER_SERVICE; + static ThreadFactory THREAD_FACTORY; + private static final long NO_EXPIRATION = Long.MAX_VALUE; List> expirationListeners; List> asyncExpirationListeners; @@ -505,8 +506,8 @@ public final Map.Entry next() { } } - /** Expiring map entry implementation. */ - static class ExpiringEntry implements Comparable> { + /** Expiring map entry implementation. */ + static class ExpiringEntry implements Comparable> { 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 entry, boolean scheduleFirstEntry) { * * @param entry Entry to schedule */ - void scheduleEntry(ExpiringEntry entry) { - if (entry == null || entry.scheduled) - return; - - Runnable runnable = null; - synchronized (entry) { - if (entry.scheduled) - return; - - final WeakReference> entryReference = new WeakReference>(entry); - runnable = new Runnable() { - @Override - public void run() { - ExpiringEntry entry = entryReference.get(); + void scheduleEntry(ExpiringEntry 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> entryReference = new WeakReference>(entry); + runnable = new Runnable() { + @Override + public void run() { + ExpiringEntry 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 Map.Entry mapEntryFor(final ExpiringEntry entry) { return new Map.Entry() { diff --git a/src/test/java/net/jodah/expiringmap/issues/Issue93.java b/src/test/java/net/jodah/expiringmap/issues/Issue93.java new file mode 100644 index 0000000..e23e684 --- /dev/null +++ b/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 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"); + } +}