WFLY-11970 SFSB memory leak due to System.currentTimeMillis() usage

wildfly · Apr 11, 2019 · 7fff0e7 · 7fff0e7
1 parent f29ce49
commit 7fff0e7
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 35 deletions.
diff --git a/clustering/ejb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanEntry.java b/clustering/ejb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanEntry.java
@@ -21,7 +21,7 @@
  */
 package org.wildfly.clustering.ejb.infinispan;
 
-import java.util.Date;
+import java.time.Instant;
 
 /**
  * The cache entry for a bean.
@@ -35,6 +35,6 @@ public interface BeanEntry<G> {
     G getGroupId();
     String getBeanName();
 
-    Date getLastAccessedTime();
-    void setLastAccessedTime(Date time);
+    Instant getLastAccessedTime();
+    void setLastAccessedTime(Instant time);
 }
diff --git a/...finispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanExpirationScheduler.java b/...finispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanExpirationScheduler.java
@@ -114,18 +114,17 @@ private class ExpirationTask implements Runnable {
         @Override
         public void run() {
             InfinispanEjbLogger.ROOT_LOGGER.tracef("Expiring stateful session bean %s", this.id);
-            boolean complete = false;
+            boolean removed = false;
             try (Batch batch = BeanExpirationScheduler.this.batcher.createBatch()) {
                 try {
-                    BeanExpirationScheduler.this.remover.remove(this.id, BeanExpirationScheduler.this.expiration.getRemoveListener());
-                    complete = true;
+                    removed = BeanExpirationScheduler.this.remover.remove(this.id, BeanExpirationScheduler.this.expiration.getRemoveListener());
                 } catch (Throwable e) {
                     InfinispanEjbLogger.ROOT_LOGGER.failedToExpireBean(e, this.id);
                     batch.discard();
                 }
             } finally {
                 synchronized (this) {
-                    if (complete) {
+                    if (removed) {
                         BeanExpirationScheduler.this.expirationFutures.remove(this.id);
                     } else {
                         // If bean failed to expire, likely due to a lock timeout, just reschedule it

diff --git a/...ering/ejb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanRemover.java b/...ering/ejb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/BeanRemover.java
@@ -29,5 +29,11 @@
  * @author Paul Ferraro
  */
 public interface BeanRemover<K, V> {
-    void remove(K id, RemoveListener<V> listener);
+    /**
+     * Removes the specified bean, triggering the specified listener
+     * @param id a bean identifier
+     * @param listener a removal listener
+     * @return true, if the bean was (or was already) removed, false otherwise
+     */
+    boolean remove(K id, RemoveListener<V> listener);
 }
diff --git a/...jb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/ExpiredBeanRemover.java b/...jb/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/ExpiredBeanRemover.java
@@ -43,15 +43,17 @@ public ExpiredBeanRemover(BeanFactory<I, T> factory) {
     }
 
     @Override
-    public void remove(I id, RemoveListener<T> listener) {
+    public boolean remove(I id, RemoveListener<T> listener) {
         BeanEntry<I> entry = this.factory.findValue(id);
         @SuppressWarnings("resource")
         Bean<I, T> bean = (entry != null) ? this.factory.createBean(id, entry) : null;
         if (bean != null) {
             if (bean.isExpired()) {
                 InfinispanEjbLogger.ROOT_LOGGER.tracef("Removing expired bean %s", id);
-                this.factory.remove(id, listener);
+                return this.factory.remove(id, listener);
             }
+            return false;
         }
+        return true;
     }
 }
diff --git a/...b/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBean.java b/...b/infinispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBean.java
@@ -22,7 +22,7 @@
 package org.wildfly.clustering.ejb.infinispan.bean;
 
 import java.time.Duration;
-import java.util.Date;
+import java.time.Instant;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.wildfly.clustering.ee.Mutator;
@@ -76,10 +76,10 @@ public I getGroupId() {
 
     @Override
     public boolean isExpired() {
-        if (this.timeout == null) return false;
-        Date lastAccessedTime = this.entry.getLastAccessedTime();
-        long timeout = this.timeout.toMillis();
-        return (lastAccessedTime != null) && (timeout > 0) ? ((System.currentTimeMillis() - lastAccessedTime.getTime()) >= timeout) : false;
+        if ((this.timeout == null) || this.timeout.isNegative()) return false;
+        if (this.timeout.isZero()) return true;
+        Instant lastAccessedTime = this.entry.getLastAccessedTime();
+        return (lastAccessedTime != null) ? !lastAccessedTime.plus(this.timeout).isAfter(Instant.now()) : false;
     }
 
     @Override
@@ -110,8 +110,8 @@ public boolean release() {
     @Override
     public void close() {
         if (this.valid.get()) {
-            Date lastAccessedTime = this.entry.getLastAccessedTime();
-            this.entry.setLastAccessedTime(new Date());
+            Instant lastAccessedTime = this.entry.getLastAccessedTime();
+            this.entry.setLastAccessedTime(Instant.now());
             if (lastAccessedTime != null) {
                 this.mutator.mutate();
             }

diff --git a/...inispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanEntry.java b/...inispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanEntry.java
@@ -23,7 +23,7 @@
 
 import org.wildfly.clustering.ejb.infinispan.BeanEntry;
 
-import java.util.Date;
+import java.time.Instant;
 
 /**
  * The cache entry for a bean.
@@ -36,7 +36,7 @@ public class InfinispanBeanEntry<I> implements BeanEntry<I> {
 
     private final String beanName;
     private final I groupId;
-    private volatile Date lastAccessedTime;
+    private volatile Instant lastAccessedTime;
 
     public InfinispanBeanEntry(String beanName, I groupId) {
         this.beanName = beanName;
@@ -54,12 +54,12 @@ public I getGroupId() {
     }
 
     @Override
-    public Date getLastAccessedTime() {
+    public Instant getLastAccessedTime() {
         return this.lastAccessedTime;
     }
 
     @Override
-    public void setLastAccessedTime(Date time) {
+    public void setLastAccessedTime(Instant time) {
         this.lastAccessedTime = time;
     }
 }
diff --git a/...main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanEntryExternalizer.java b/...main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanEntryExternalizer.java
@@ -24,7 +24,7 @@
 import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
-import java.util.Date;
+import java.time.Instant;
 
 import org.jboss.ejb.client.SessionID;
 import org.kohsuke.MetaInfServices;
@@ -41,16 +41,16 @@ public class InfinispanBeanEntryExternalizer implements Externalizer<InfinispanB
     public void writeObject(ObjectOutput output, InfinispanBeanEntry<SessionID> entry) throws IOException {
         output.writeUTF(entry.getBeanName());
         SessionIDSerializer.INSTANCE.write(output, entry.getGroupId());
-        Date lastAccessedTime = entry.getLastAccessedTime();
-        output.writeLong((lastAccessedTime != null) ? lastAccessedTime.getTime() : 0);
+        Instant lastAccessedTime = entry.getLastAccessedTime();
+        output.writeLong((lastAccessedTime != null) ? lastAccessedTime.toEpochMilli() : 0L);
     }
 
     @Override
     public InfinispanBeanEntry<SessionID> readObject(ObjectInput input) throws IOException, ClassNotFoundException {
         InfinispanBeanEntry<SessionID> entry = new InfinispanBeanEntry<>(input.readUTF(), SessionIDSerializer.INSTANCE.read(input));
-        long time = input.readLong();
-        if (time > 0) {
-            entry.setLastAccessedTime(new Date(time));
+        long lastAccessedTime = input.readLong();
+        if (lastAccessedTime > 0L) {
+            entry.setLastAccessedTime(Instant.ofEpochMilli(lastAccessedTime));
         }
         return entry;
     }

diff --git a/...ispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanFactory.java b/...ispan/src/main/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanFactory.java
@@ -102,7 +102,7 @@ public BeanEntry<I> createValue(I id, I groupId) {
     }
 
     @Override
-    public void remove(I id, RemoveListener<T> listener) {
+    public boolean remove(I id, RemoveListener<T> listener) {
         BeanEntry<I> entry = this.cache.getAdvancedCache().withFlags(Flag.FORCE_SYNCHRONOUS).remove(this.createKey(id));
         if (entry != null) {
             I groupId = entry.getGroupId();
@@ -116,5 +116,6 @@ public void remove(I id, RemoveListener<T> listener) {
                 }
             }
         }
+        return true;
     }
 }
diff --git a/.../src/test/java/org/wildfly/clustering/ejb/infinispan/BeanExpirationSchedulerTestCase.java b/.../src/test/java/org/wildfly/clustering/ejb/infinispan/BeanExpirationSchedulerTestCase.java
@@ -73,6 +73,7 @@ public void testExpire() throws InterruptedException {
 
         when(config.getTimeout()).thenReturn(Duration.ofMillis(1L));
         when(config.getRemoveListener()).thenReturn(listener);
+        when(remover.remove(beanId, listener)).thenReturn(true);
 
         try (Scheduler<String> scheduler = new BeanExpirationScheduler<>(batcher, remover, config)) {
             scheduler.schedule(beanId);
@@ -96,6 +97,7 @@ public void testCancel() throws InterruptedException {
 
         when(config.getTimeout()).thenReturn(Duration.ofMinutes(1L));
         when(config.getRemoveListener()).thenReturn(listener);
+        when(remover.remove(beanId, listener)).thenReturn(true);
 
         try (Scheduler<String> scheduler = new BeanExpirationScheduler<>(batcher, remover, config)) {
             scheduler.schedule(beanId);

diff --git a/...span/src/test/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanTestCase.java b/...span/src/test/java/org/wildfly/clustering/ejb/infinispan/bean/InfinispanBeanTestCase.java
@@ -29,7 +29,7 @@
 import static org.mockito.Mockito.when;
 
 import java.time.Duration;
-import java.util.Date;
+import java.time.Instant;
 
 import org.junit.After;
 import org.junit.Assert;
@@ -93,11 +93,11 @@ public void isExpired() {
         when(this.entry.getLastAccessedTime()).thenReturn(null);
         Assert.assertFalse(this.bean.isExpired());
 
-        long now = System.currentTimeMillis();
-        when(this.entry.getLastAccessedTime()).thenReturn(new Date(now));
+        Instant now = Instant.now();
+        when(this.entry.getLastAccessedTime()).thenReturn(now);
         Assert.assertFalse(this.bean.isExpired());
 
-        when(this.entry.getLastAccessedTime()).thenReturn(new Date(now - this.timeout.toMillis() - 1));
+        when(this.entry.getLastAccessedTime()).thenReturn(now.minus(this.timeout));
         Assert.assertTrue(this.bean.isExpired());
     }
 
@@ -123,18 +123,18 @@ public void close() {
 
         this.bean.close();
 
-        verify(this.entry).setLastAccessedTime(ArgumentMatchers.<Date>any());
+        verify(this.entry).setLastAccessedTime(ArgumentMatchers.<Instant>any());
         verify(this.mutator, never()).mutate();
         verify(this.group, never()).close();
 
         reset(this.entry, this.mutator, this.group);
 
-        when(this.entry.getLastAccessedTime()).thenReturn(new Date());
+        when(this.entry.getLastAccessedTime()).thenReturn(Instant.now());
         when(this.group.isCloseable()).thenReturn(true);
 
         this.bean.close();
 
-        verify(this.entry).setLastAccessedTime(ArgumentMatchers.<Date>any());
+        verify(this.entry).setLastAccessedTime(ArgumentMatchers.<Instant>any());
         verify(this.mutator).mutate();
         verify(this.group).close();
     }