You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2020/03/17 15:52:42 UTC

[geode] branch feature/GEODE-7884 created (now 13741e7)

This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a change to branch feature/GEODE-7884
in repository https://gitbox.apache.org/repos/asf/geode.git.


      at 13741e7  GEODE-7884: server hangs due to IllegalStateException

This branch includes the following new commits:

     new 13741e7  GEODE-7884: server hangs due to IllegalStateException

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[geode] 01/01: GEODE-7884: server hangs due to IllegalStateException

Posted by bs...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-7884
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 13741e70b4f640b67519e78bb5e4bfbee00f03de
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Tue Mar 17 08:49:26 2020 -0700

    GEODE-7884: server hangs due to IllegalStateException
    
    Added cancellation check before scheduling an idle-timeout or
    ack-wait-threshold timer task.  I had to add a new method to
    SystemTimerTask and then noticed there were no tests for SystemTimer, so
    I cleaned up that class and added tests.
---
 .../org/apache/geode/internal/SystemTimer.java     | 168 ++++-----------------
 .../geode/internal/admin/StatAlertsManager.java    |   2 +-
 .../geode/internal/cache/ExpirationScheduler.java  |   2 +-
 .../geode/internal/cache/GemFireCacheImpl.java     |   2 +-
 .../cache/partitioned/PRSanityCheckMessage.java    |   2 +-
 .../internal/cache/tier/sockets/AcceptorImpl.java  |   2 +-
 .../org/apache/geode/internal/tcp/Connection.java  |  20 ++-
 .../apache/geode/internal/tcp/ConnectionTable.java |  22 +--
 .../org/apache/geode/internal/SystemTimerTest.java | 146 ++++++++++++++++++
 9 files changed, 211 insertions(+), 155 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/SystemTimer.java b/geode-core/src/main/java/org/apache/geode/internal/SystemTimer.java
index 1feba43..2029926 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/SystemTimer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/SystemTimer.java
@@ -15,11 +15,11 @@
 package org.apache.geode.internal;
 
 import java.lang.ref.WeakReference;
-import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Timer;
 import java.util.TimerTask;
@@ -27,20 +27,17 @@ import java.util.TimerTask;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.CancelException;
-import org.apache.geode.SystemFailure;
 import org.apache.geode.annotations.internal.MakeNotStatic;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 
 /**
  * Instances of this class are like {@link Timer}, but are associated with a "swarm", which can be
- * cancelled as a group with {@link #cancelSwarm(Object)}.
+ * cancelled as a group with {@link #cancelSwarm(DistributedSystem)}.
  *
  * @see Timer
  * @see TimerTask
  *
- *      TODO -- with Java 1.5, this will be a template type so that the swarm's class can be
- *      specified.
  */
 public class SystemTimer {
   private static final Logger logger = LogService.getLogger();
@@ -49,12 +46,6 @@ public class SystemTimer {
       "IBM Corporation".equals(System.getProperty("java.vm.vendor"));
 
   /**
-   * Extra debugging for this class
-   */
-  // private static final boolean DEBUG = true;
-  static final boolean DEBUG = false;
-
-  /**
    * the underlying {@link Timer}
    */
   private final Timer timer;
@@ -62,19 +53,18 @@ public class SystemTimer {
   /**
    * True if this timer has been cancelled
    */
-  private boolean cancelled = false;
+  private volatile boolean cancelled = false;
 
   /**
    * the swarm to which this timer belongs
    */
-  private final Object /* T */ swarm;
+  private final DistributedSystem swarm;
 
   @Override
   public String toString() {
     StringBuffer sb = new StringBuffer();
     sb.append("SystemTimer[");
     sb.append("swarm = " + swarm);
-    // sb.append("; timer = " + timer);
     sb.append("]");
     return sb.toString();
   }
@@ -83,7 +73,7 @@ public class SystemTimer {
    * List of all of the swarms in the system
    */
   @MakeNotStatic
-  private static final HashMap allSwarms = new HashMap();
+  private static final HashMap<DistributedSystem, List> allSwarms = new HashMap();
 
   /**
    * Add the given timer is in the given swarm. Used only by constructors.
@@ -91,25 +81,18 @@ public class SystemTimer {
    * @param swarm swarm to add the timer to
    * @param t timer to add
    */
-  private static void addToSwarm(Object /* T */ swarm, SystemTimer t) {
-    final boolean isDebugEnabled = logger.isTraceEnabled();
+  private static void addToSwarm(DistributedSystem swarm, SystemTimer t) {
     // Get or add list of timers for this swarm...
     ArrayList /* ArrayList<WeakReference<SystemTimer>> */ swarmSet;
     synchronized (allSwarms) {
       swarmSet = (ArrayList) allSwarms.get(swarm);
       if (swarmSet == null) {
-        if (isDebugEnabled) {
-          logger.trace("SystemTimer#addToSwarm: created swarm {}", swarm);
-        }
         swarmSet = new ArrayList();
         allSwarms.put(swarm, swarmSet);
       }
     } // synchronized
 
     // Add the timer to the swarm's list
-    if (isDebugEnabled) {
-      logger.trace("SystemTimer#addToSwarm: adding timer <{}>", t);
-    }
     WeakReference /* WeakReference<SystemTimer> */ wr = new WeakReference(t);
     synchronized (swarmSet) {
       swarmSet.add(wr);
@@ -186,21 +169,14 @@ public class SystemTimer {
    * @see #cancel()
    */
   private static void removeFromSwarm(SystemTimer t) {
-    final boolean isDebugEnabled = logger.isTraceEnabled();
     synchronized (allSwarms) {
       // Get timer's swarm
-      ArrayList swarmSet = (ArrayList) allSwarms.get(t.swarm);
+      List swarmSet = (ArrayList) allSwarms.get(t.swarm);
       if (swarmSet == null) {
-        if (isDebugEnabled) {
-          logger.trace("SystemTimer#removeFromSwarm: timer already removed: {}", t);
-        }
         return; // already gone
       }
 
       // Remove timer from swarm
-      if (isDebugEnabled) {
-        logger.trace("SystemTimer#removeFromSwarm: removing timer <{}>", t);
-      }
       synchronized (swarmSet) {
         Iterator it = swarmSet.iterator();
         while (it.hasNext()) {
@@ -228,14 +204,11 @@ public class SystemTimer {
         // we should remove it.
         if (swarmSet.size() == 0) {
           allSwarms.remove(t.swarm); // last reference
-          if (isDebugEnabled) {
-            logger.trace("SystemTimer#removeFromSwarm: removed last reference to {}", t.swarm);
-          }
         }
       } // synchronized swarmSet
     } // synchronized allSwarms
 
-    sweepAllSwarms(); // Occasionally check global list, use any available logger :-)
+    sweepAllSwarms(); // Occasionally check global list
   }
 
   /**
@@ -243,12 +216,11 @@ public class SystemTimer {
    *
    * @param swarm the swarm to cancel
    */
-  public static void cancelSwarm(Object /* T */ swarm) {
-    Assert.assertTrue(swarm instanceof InternalDistributedSystem); // TODO
+  public static void cancelSwarm(DistributedSystem swarm) {
     // Find the swarmSet and remove it
-    ArrayList swarmSet;
+    List<WeakReference> swarmSet;
     synchronized (allSwarms) {
-      swarmSet = (ArrayList) allSwarms.get(swarm);
+      swarmSet = allSwarms.get(swarm);
       if (swarmSet == null) {
         return; // already cancelled
       }
@@ -259,9 +231,7 @@ public class SystemTimer {
 
     // Empty the swarmSet
     synchronized (swarmSet) {
-      Iterator it = swarmSet.iterator();
-      while (it.hasNext()) {
-        WeakReference wr = (WeakReference) it.next();
+      for (WeakReference wr : swarmSet) {
         SystemTimer st = (SystemTimer) wr.get();
         // it.remove(); Not necessary, we're emptying the list...
         if (st != null) {
@@ -273,10 +243,6 @@ public class SystemTimer {
   }
 
   public int timerPurge() {
-    if (logger.isTraceEnabled()) {
-      logger.trace("SystemTimer#timerPurge of {}", this);
-    }
-
     // Fix 39585, IBM's java.util.timer's purge() has stack overflow issue
     if (isIBM) {
       return 0;
@@ -284,42 +250,12 @@ public class SystemTimer {
     return this.timer.purge();
   }
 
-  // This creates a non-daemon timer thread. We don't EVER do this...
-  // /**
-  // * @see Timer#Timer()
-  // *
-  // * @param swarm the swarm this timer belongs to
-  // */
-  // public SystemTimer(DistributedSystem swarm) {
-  // this.timer = new Timer();
-  // this.swarm = swarm;
-  // addToSwarm(swarm, this);
-  // }
-
   /**
    * @see Timer#Timer(boolean)
    * @param swarm the swarm this timer belongs to, currently must be a DistributedSystem
-   * @param isDaemon whether the timer is a daemon. Must be true for GemFire use.
-   */
-  public SystemTimer(Object /* T */ swarm, boolean isDaemon) {
-    Assert.assertTrue(isDaemon); // we don't currently allow non-daemon timers
-    Assert.assertTrue(swarm instanceof InternalDistributedSystem,
-        "Attempt to create swarm on " + swarm); // TODO allow template class?
-    this.timer = new Timer(isDaemon);
-    this.swarm = swarm;
-    addToSwarm(swarm, this);
-  }
-
-  /**
-   * @param name the name to give the timer thread
-   * @param swarm the swarm this timer belongs to, currently must be a DistributedMember
-   * @param isDaemon whether the timer is a daemon. Must be true for GemFire use.
    */
-  public SystemTimer(String name, Object /* T */ swarm, boolean isDaemon) {
-    Assert.assertTrue(isDaemon); // we don't currently allow non-daemon timers
-    Assert.assertTrue(swarm instanceof InternalDistributedSystem,
-        "Attempt to create swarm on " + swarm); // TODO allow template class?
-    this.timer = new Timer(name, isDaemon);
+  public SystemTimer(DistributedSystem swarm) {
+    this.timer = new Timer(true);
     this.swarm = swarm;
     addToSwarm(swarm, this);
   }
@@ -335,12 +271,6 @@ public class SystemTimer {
    */
   public void schedule(SystemTimerTask task, long delay) {
     checkCancelled();
-    if (logger.isTraceEnabled()) {
-      Date tilt = new Date(System.currentTimeMillis() + delay);
-      SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
-      logger.trace("SystemTimer#schedule (long): {}: expect task {} to fire around {}", this, task,
-          sdf.format(tilt));
-    }
     timer.schedule(task, delay);
   }
 
@@ -349,39 +279,13 @@ public class SystemTimer {
    */
   public void schedule(SystemTimerTask task, Date time) {
     checkCancelled();
-    if (logger.isTraceEnabled()) {
-      SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
-      logger.trace("SystemTimer#schedule (Date): {}: expect task {} to fire around {}", this, task,
-          sdf.format(time));
-    }
     timer.schedule(task, time);
   }
 
-  // Not currently used, so don't complicate things
-  // /**
-  // * @see Timer#schedule(TimerTask, long, long)
-  // */
-  // public void schedule(SystemTimerTask task, long delay, long period) {
-  // // TODO add debug statement
-  // checkCancelled();
-  // timer.schedule(task, delay, period);
-  // }
-
-  // Not currently used, so don't complicate things
-  // /**
-  // * @see Timer#schedule(TimerTask, Date, long)
-  // */
-  // public void schedule(SystemTimerTask task, Date firstTime, long period) {
-  // // TODO add debug statement
-  // checkCancelled();
-  // timer.schedule(task, firstTime, period);
-  // }
-
   /**
    * @see Timer#scheduleAtFixedRate(TimerTask, long, long)
    */
   public void scheduleAtFixedRate(SystemTimerTask task, long delay, long period) {
-    // TODO add debug statement
     checkCancelled();
     timer.scheduleAtFixedRate(task, delay, period);
   }
@@ -390,23 +294,10 @@ public class SystemTimer {
    * @see Timer#schedule(TimerTask, long, long)
    */
   public void schedule(SystemTimerTask task, long delay, long period) {
-    // TODO add debug statement
     checkCancelled();
     timer.schedule(task, delay, period);
   }
 
-  // Not currently used, so don't complicate things
-  // /**
-  // * @see Timer#scheduleAtFixedRate(TimerTask, Date, long)
-  // */
-  // public void scheduleAtFixedRate(SystemTimerTask task, Date firstTime,
-  // long period) {
-  // // TODO add debug statement
-  // checkCancelled();
-  // timer.scheduleAtFixedRate(task, firstTime, period);
-  // }
-
-
   /**
    * @see Timer#cancel()
    */
@@ -417,12 +308,30 @@ public class SystemTimer {
   }
 
   /**
+   * has this timer been cancelled?
+   */
+  public boolean isCancelled() {
+    return cancelled;
+  }
+
+  /**
    * Cover class to track behavior of scheduled tasks
    *
    * @see TimerTask
    */
   public abstract static class SystemTimerTask extends TimerTask {
     protected static final Logger logger = LogService.getLogger();
+    private volatile boolean cancelled;
+
+    public boolean isCancelled() {
+      return cancelled;
+    }
+
+    @Override
+    public boolean cancel() {
+      cancelled = true;
+      return super.cancel();
+    }
 
     /**
      * This is your executed action
@@ -434,25 +343,14 @@ public class SystemTimer {
      */
     @Override
     public void run() {
-      final boolean isDebugEnabled = logger.isTraceEnabled();
-      if (isDebugEnabled) {
-        logger.trace("SystemTimer.MyTask: starting {}", this);
-      }
       try {
         this.run2();
       } catch (CancelException ignore) {
         // ignore: TimerThreads can fire during or near cache closure
-      } catch (VirtualMachineError e) {
-        SystemFailure.initiateFailure(e);
-        throw e;
       } catch (Throwable t) {
-        SystemFailure.checkFailure();
         logger.warn(String.format("Timer task <%s> encountered exception", this), t);
         // Don't rethrow, it will just get eaten and kill the timer
       }
-      if (isDebugEnabled) {
-        logger.trace("SystemTimer.MyTask: finished {}", this);
-      }
     }
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/StatAlertsManager.java b/geode-core/src/main/java/org/apache/geode/internal/admin/StatAlertsManager.java
index 7fbbcb3..0205339 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/admin/StatAlertsManager.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/admin/StatAlertsManager.java
@@ -175,7 +175,7 @@ public class StatAlertsManager {
           "This manager has been cancelled");
     }
     // start and schedule new timer
-    timer = new SystemTimer(system /* swarm */, true);
+    timer = new SystemTimer(system /* swarm */);
 
     EvaluateAlertDefnsTask task = new EvaluateAlertDefnsTask();
     if (refreshAtFixedRate) {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpirationScheduler.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpirationScheduler.java
index e4bf8c9..0698e26 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/ExpirationScheduler.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ExpirationScheduler.java
@@ -38,7 +38,7 @@ public class ExpirationScheduler {
       .getInteger(GeodeGlossary.GEMFIRE_PREFIX + "MAX_PENDING_CANCELS", 10000).intValue();
 
   public ExpirationScheduler(InternalDistributedSystem ds) {
-    this.timer = new SystemTimer(ds, true);
+    this.timer = new SystemTimer(ds);
   }
 
   public void forcePurge() {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index 639dd8a..bf366b4 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -889,7 +889,7 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
         TypeRegistry::new,
         HARegionQueue::setMessageSyncInterval,
         FunctionService::registerFunction,
-        object -> new SystemTimer(object, true),
+        object -> new SystemTimer((DistributedSystem) object),
         TombstoneService::initialize,
         ExpirationScheduler::new,
         DiskStoreMonitor::new,
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PRSanityCheckMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PRSanityCheckMessage.java
index 596429b..1d87d1d 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PRSanityCheckMessage.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PRSanityCheckMessage.java
@@ -124,7 +124,7 @@ public class PRSanityCheckMessage extends PartitionMessage {
       int sanityCheckInterval = Integer
           .getInteger(GeodeGlossary.GEMFIRE_PREFIX + "PRSanityCheckInterval", 5000).intValue();
       if (sanityCheckInterval != 0) {
-        final SystemTimer tm = new SystemTimer(dm.getSystem(), true);
+        final SystemTimer tm = new SystemTimer(dm.getSystem());
         SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
           @Override
           public void run2() {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index f15e518..d1a61fb 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -499,7 +499,7 @@ public class AcceptorImpl implements Acceptor, Runnable {
         tmp_q = new LinkedBlockingQueue<>();
         tmp_commQ = new LinkedBlockingQueue<>();
         tmp_hs = new HashSet<>(512);
-        tmp_timer = new SystemTimer(internalCache.getDistributedSystem(), true);
+        tmp_timer = new SystemTimer(internalCache.getDistributedSystem());
       }
       selector = tmp_s;
       selectorQueue = tmp_q;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
index 8c8a2fc..eddf1dc 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java
@@ -1411,11 +1411,15 @@ public class Connection implements Runnable {
     // This cancels the idle timer task, but it also removes the tasks reference to this connection,
     // freeing up the connection (and it's buffers for GC sooner.
     if (idleTask != null) {
-      idleTask.cancel();
+      synchronized (idleTask) {
+        idleTask.cancel();
+      }
     }
 
     if (ackTimeoutTask != null) {
-      ackTimeoutTask.cancel();
+      synchronized (ackTimeoutTask) {
+        ackTimeoutTask.cancel();
+      }
     }
   }
 
@@ -1950,10 +1954,14 @@ public class Connection implements Runnable {
       synchronized (owner) {
         SystemTimer timer = owner.getIdleConnTimer();
         if (timer != null) {
-          if (msSA > 0) {
-            timer.scheduleAtFixedRate(ackTimeoutTask, msAW, Math.min(msAW, msSA));
-          } else {
-            timer.schedule(ackTimeoutTask, msAW);
+          synchronized (ackTimeoutTask) {
+            if (!ackTimeoutTask.isCancelled()) {
+              if (msSA > 0) {
+                timer.scheduleAtFixedRate(ackTimeoutTask, msAW, Math.min(msAW, msSA));
+              } else {
+                timer.schedule(ackTimeoutTask, msAW);
+              }
+            }
           }
         }
       }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java b/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
index 0c098d1..a5c43f9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java
@@ -199,7 +199,7 @@ public class ConnectionTable {
   private ConnectionTable(TCPConduit conduit) {
     owner = conduit;
     idleConnTimer = owner.idleConnectionTimeout != 0
-        ? new SystemTimer(conduit.getDM().getSystem(), true) : null;
+        ? new SystemTimer(conduit.getDM().getSystem()) : null;
     threadConnMaps = new ArrayList();
     threadConnectionMap = new ConcurrentHashMap();
     p2pReaderThreadPool = createThreadPoolForIO(conduit.getDM().getSystem().isShareSockets());
@@ -519,8 +519,12 @@ public class ConnectionTable {
           if (!closed) {
             IdleConnTT task = new IdleConnTT(conn);
             conn.setIdleTimeoutTask(task);
-            getIdleConnTimer().scheduleAtFixedRate(task, owner.idleConnectionTimeout,
-                owner.idleConnectionTimeout);
+            synchronized (task) {
+              if (!task.isCancelled()) {
+                getIdleConnTimer().scheduleAtFixedRate(task, owner.idleConnectionTimeout,
+                    owner.idleConnectionTimeout);
+              }
+            }
           }
         }
       } catch (IllegalStateException e) {
@@ -620,7 +624,7 @@ public class ConnectionTable {
       return null;
     }
     if (idleConnTimer == null) {
-      idleConnTimer = new SystemTimer(getDM().getSystem(), true);
+      idleConnTimer = new SystemTimer(getDM().getSystem());
     }
     return idleConnTimer;
   }
@@ -1216,25 +1220,25 @@ public class ConnectionTable {
 
   private static class IdleConnTT extends SystemTimer.SystemTimerTask {
 
-    private Connection c;
+    private Connection connection;
 
     private IdleConnTT(Connection c) {
-      this.c = c;
+      this.connection = c;
     }
 
     @Override
     public boolean cancel() {
-      Connection con = c;
+      Connection con = connection;
       if (con != null) {
         con.cleanUpOnIdleTaskCancel();
       }
-      c = null;
+      connection = null;
       return super.cancel();
     }
 
     @Override
     public void run2() {
-      Connection con = c;
+      Connection con = connection;
       if (con != null) {
         if (con.checkForIdleTimeout()) {
           cancel();
diff --git a/geode-core/src/test/java/org/apache/geode/internal/SystemTimerTest.java b/geode-core/src/test/java/org/apache/geode/internal/SystemTimerTest.java
new file mode 100644
index 0000000..a3a219c
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/SystemTimerTest.java
@@ -0,0 +1,146 @@
+package org.apache.geode.internal;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+
+import java.util.Date;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.distributed.DistributedSystem;
+
+public class SystemTimerTest {
+
+  private DistributedSystem swarm;
+  private SystemTimer systemTimer;
+
+  @Before
+  public void setup() {
+    this.swarm = mock(DistributedSystem.class);
+    this.systemTimer = new SystemTimer(swarm);
+  }
+
+  @After
+  public void teardown() {
+    if (!systemTimer.isCancelled()) {
+      systemTimer.cancel();
+    }
+  }
+
+  @Test
+  public void cancelSwarm() {
+    assertThat(systemTimer.isCancelled()).isFalse();
+    SystemTimer.cancelSwarm(swarm);
+    assertThat(systemTimer.isCancelled()).isTrue();
+  }
+
+  @Test
+  public void cancel() {
+    assertThat(systemTimer.isCancelled()).isFalse();
+    systemTimer.cancel();
+    assertThat(systemTimer.isCancelled()).isTrue();
+  }
+
+  @Test
+  public void scheduleNow() {
+    AtomicBoolean hasRun = new AtomicBoolean(false);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        hasRun.set(true);
+      }
+    };
+    systemTimer.schedule(task, 0);
+    await().until(() -> hasRun.get());
+  }
+
+  @Test
+  public void scheduleWithDelay() {
+    AtomicBoolean hasRun = new AtomicBoolean(false);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        hasRun.set(true);
+      }
+    };
+    final long millis = System.currentTimeMillis();
+    final int delay = 1000;
+    systemTimer.schedule(task, delay);
+    await().until(() -> hasRun.get());
+    assertThat(System.currentTimeMillis()).isGreaterThanOrEqualTo(millis + delay);
+  }
+
+  @Test
+  public void scheduleWithDate() {
+    AtomicBoolean hasRun = new AtomicBoolean(false);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        hasRun.set(true);
+      }
+    };
+    final long millis = System.currentTimeMillis();
+    final long delay = 1000;
+    final Date scheduleTime = new Date(System.currentTimeMillis() + delay);
+    systemTimer.schedule(task, scheduleTime);
+    await().until(() -> hasRun.get());
+    assertThat(System.currentTimeMillis()).isGreaterThanOrEqualTo(millis + delay);
+  }
+
+  @Test
+  public void scheduleRepeatedWithDelay() {
+    AtomicInteger invocations = new AtomicInteger(0);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        invocations.incrementAndGet();
+      }
+    };
+    final long millis = System.currentTimeMillis();
+    final int delay = 1000;
+    final int period = 500;
+    systemTimer.schedule(task, delay, period);
+    await().untilAsserted(() -> assertThat(invocations.get()).isGreaterThanOrEqualTo(2));
+    assertThat(System.currentTimeMillis()).isGreaterThanOrEqualTo(millis + delay + period);
+  }
+
+  @Test
+  public void scheduleAtFixedRate() {
+    AtomicInteger invocations = new AtomicInteger(0);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        invocations.incrementAndGet();
+      }
+    };
+    final long millis = System.currentTimeMillis();
+    final int delay = 1000;
+    final int period = 500;
+    systemTimer.scheduleAtFixedRate(task, delay, period);
+    await().untilAsserted(() -> assertThat(invocations.get()).isGreaterThanOrEqualTo(2));
+    assertThat(System.currentTimeMillis()).isGreaterThanOrEqualTo(millis + delay + period);
+  }
+
+  @Test
+  public void cancelTask() {
+    AtomicInteger invocations = new AtomicInteger(0);
+    SystemTimer.SystemTimerTask task = new SystemTimer.SystemTimerTask() {
+      @Override
+      public void run2() {
+        invocations.incrementAndGet();
+      }
+    };
+    assertThat(task.isCancelled()).isFalse();
+    task.cancel();
+    assertThat(task.isCancelled()).isTrue();
+    assertThatThrownBy(() -> systemTimer.schedule(task, 0))
+        .isInstanceOf(IllegalStateException.class);
+  }
+
+}