From 23c3a6be99370f3bc4e4f5117d937610b884159b Mon Sep 17 00:00:00 2001
From: Endi Sukma Dewata <edewata@redhat.com>
Date: Wed, 28 Mar 2012 12:46:13 -0500
Subject: [PATCH] Replaced key status update thread with executor service.

The Thread.stop() is deprecated, so the key status update thread is now
implemented with executor service to allow stopping the task gracefully.

Ticket #3
---
 .../cmscore/dbs/CertificateRepository.java         |   46 ++++----
 .../com/netscape/cmscore/dbs/KeyRepository.java    |  129 +++++++++++---------
 2 files changed, 94 insertions(+), 81 deletions(-)

diff --git a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
index 0df563cad430be35898651a34162f67efd74c0b5..ebcd6cc5cde8222db0e697aab0e523d40f55f176 100644
--- a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
+++ b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
@@ -63,7 +63,7 @@ import com.netscape.certsrv.logging.ILogger;
  * A class represents a certificate repository. It
  * stores all the issued certificate.
  * <P>
- * 
+ *
  * @author thomask
  * @author kanda
  * @version $Revision$, $Date$
@@ -231,14 +231,16 @@ public class CertificateRepository extends Repository
 
         this.requestRepository = requestRepository;
 
+        // stop running tasks
+        if (certStatusUpdateTask != null) {
+            certStatusUpdateTask.stop();
+        }
+        if (retrieveModificationsTask != null) {
+            retrieveModificationsTask.stop();
+        }
+
         if (interval == 0) {
             CMS.debug("In setCertStatusUpdateInterval interval = 0");
-            if (certStatusUpdateTask != null) {
-                certStatusUpdateTask.stop();
-            }
-            if (retrieveModificationsTask != null) {
-                retrieveModificationsTask.stop();
-            }
             return;
         }
 
@@ -341,7 +343,7 @@ public class CertificateRepository extends Repository
      * record contains four parts: certificate, meta-attributes,
      * issue information and reovcation information.
      * <P>
-     * 
+     *
      * @param cert X.509 certificate
      * @exception EBaseException failed to add new certificate to
      *                the repository
@@ -369,7 +371,7 @@ public class CertificateRepository extends Repository
                 record.set(CertRecord.ATTR_ISSUED_BY, uid);
             }
 
-            // Check validity of this certificate. If it is not invalid, 
+            // Check validity of this certificate. If it is not invalid,
             // mark it so. We will have a thread to transit the status
             // from INVALID to VALID.
             X509CertImpl x509cert = (X509CertImpl) record.get(
@@ -396,7 +398,7 @@ public class CertificateRepository extends Repository
      * Used by the Clone Master (CLA) to add a revoked certificate
      * record to the repository.
      * <p>
-     * 
+     *
      * @param record a CertRecord
      * @exception EBaseException failed to add new certificate to
      *                the repository
@@ -804,7 +806,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Returns a list of X509CertImp that satisfies the filter.
-     * 
+     *
      * @deprecated replaced by <code>findCertificatesInList</code>
      */
     public Enumeration<Object> findCertRecs(String filter)
@@ -880,7 +882,7 @@ public class CertificateRepository extends Repository
     /**
      * Finds certificate records. Here is a list of filter
      * attribute can be used:
-     * 
+     *
      * <pre>
      *   certRecordId
      *   certMetaInfo
@@ -891,10 +893,10 @@ public class CertificateRepository extends Repository
      *   x509Cert.notAfter
      *   x509Cert.subject
      * </pre>
-     * 
+     *
      * The filter should follow RFC1558 LDAP filter syntax.
      * For example,
-     * 
+     *
      * <pre>
      *   (&(certRecordId=5)(x509Cert.notBefore=934398398))
      * </pre>
@@ -1118,7 +1120,7 @@ public class CertificateRepository extends Repository
     /**
      * Gets all valid and unexpired certificates pertaining
      * to a subject DN.
-     * 
+     *
      * @param subjectDN The distinguished name of the subject.
      * @param validityType The type of certificates to get.
      * @return An array of certificates.
@@ -1213,7 +1215,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Retrives all valid certificates excluding ones already revoked.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1309,7 +1311,7 @@ public class CertificateRepository extends Repository
     /**
      * Retrives all valid not published certificates
      * excluding ones already revoked.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1390,7 +1392,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Retrives all expired certificates.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1457,7 +1459,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Retrives all expired published certificates.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1631,7 +1633,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Retrieves all revoked certificates in the serial number range.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1692,7 +1694,7 @@ public class CertificateRepository extends Repository
 
     /**
      * Retrieves all revoked publishedcertificates in the serial number range.
-     * 
+     *
      * @param from The starting point of the serial number range.
      * @param to The ending point of the serial number range.
      */
@@ -1875,7 +1877,7 @@ public class CertificateRepository extends Repository
     /**
      * Checks if the presented certificate belongs to the repository
      * and is revoked.
-     * 
+     *
      * @param cert certificate to verify.
      * @return RevocationInfo if the presented certificate is revoked otherwise null.
      */
diff --git a/base/common/src/com/netscape/cmscore/dbs/KeyRepository.java b/base/common/src/com/netscape/cmscore/dbs/KeyRepository.java
index 3b2186b23b2f8d9ba776f823b17f5ea9c3c3c88d..3e7759772078f94ee1d8ec5437d0ef6fda93ef81 100644
--- a/base/common/src/com/netscape/cmscore/dbs/KeyRepository.java
+++ b/base/common/src/com/netscape/cmscore/dbs/KeyRepository.java
@@ -22,6 +22,10 @@ import java.security.PublicKey;
 import java.util.Date;
 import java.util.Enumeration;
 import java.util.Vector;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
 
 import netscape.security.x509.X500Name;
 
@@ -43,14 +47,16 @@ import com.netscape.certsrv.dbs.repository.IRepository;
  * A class represents a Key repository. This is the container of
  * archived keys.
  * <P>
- * 
+ *
  * @author thomask
  * @version $Revision$, $Date$
  */
 public class KeyRepository extends Repository implements IKeyRepository {
 
-    public KeyStatusUpdateThread mKeyStatusUpdateThread = null;
-    protected IDBSubsystem mDBService = null;
+    public KeyStatusUpdateTask mKeyStatusUpdateTask;
+    protected IDBSubsystem mDBService;
+
+    IRepository requestRepository;
 
     /**
      * Internal constants
@@ -61,7 +67,7 @@ public class KeyRepository extends Repository implements IKeyRepository {
      * Constructs a key repository. It checks if the key repository
      * does exist. If not, it creates the repository.
      * <P>
-     * 
+     *
      * @param service db service
      * @exception EBaseException failed to setup key repository
      */
@@ -146,26 +152,44 @@ public class KeyRepository extends Repository implements IKeyRepository {
     }
 
     public void setKeyStatusUpdateInterval(IRepository requestRepo, int interval) {
+
         CMS.debug("In setKeyStatusUpdateInterval " + interval);
+
+        this.requestRepository = requestRepo;
+
+        // stop running task
+        if (mKeyStatusUpdateTask != null) {
+            mKeyStatusUpdateTask.stop();
+        }
+
         // don't run the thread if serial management is disabled.
-        if ((interval == 0) || (!mDBService.getEnableSerialMgmt())) {
-            CMS.debug("In setKeyStatusUpdateInterval interval = 0" + interval);
-            if (mKeyStatusUpdateThread != null) {
-                mKeyStatusUpdateThread.stop();
-            }
+        if (interval == 0 || !mDBService.getEnableSerialMgmt()) {
+            CMS.debug("In setKeyStatusUpdateInterval interval = 0");
             return;
         }
 
-        CMS.debug("In setKeyStatusUpdateInterval  mKeyStatusUpdateThread " + mKeyStatusUpdateThread);
-        if (mKeyStatusUpdateThread == null) {
-            CMS.debug("In setKeyStatusUpdateInterval about to create KeyStatusUpdateThread ");
-            mKeyStatusUpdateThread = new KeyStatusUpdateThread(this, requestRepo, "KeyStatusUpdateThread");
-            mKeyStatusUpdateThread.setInterval(interval);
-            mKeyStatusUpdateThread.start();
-        } else {
-            CMS.debug("In setKeyStatusUpdateInterval it thinks the thread is up already ");
-            mKeyStatusUpdateThread.setInterval(interval);
-            // dont do anything if we have a thread running already
+        CMS.debug("In setKeyStatusUpdateInterval scheduling key status update every " + interval + " seconds.");
+        mKeyStatusUpdateTask = new KeyStatusUpdateTask(this, interval);
+        mKeyStatusUpdateTask.start();
+    }
+
+    /**
+     * This method blocks when another thread is running
+     */
+    public synchronized void updateKeyStatus() {
+        try {
+            CMS.debug("About to start checkRanges");
+
+            CMS.debug("Starting key checkRanges");
+            checkRanges();
+            CMS.debug("key checkRanges done");
+
+            CMS.debug("Starting request checkRanges");
+            requestRepository.checkRanges();
+            CMS.debug("request checkRanges done");
+
+        } catch (Exception e) {
+            CMS.debug("key checkRanges done: " + e.toString());
         }
     }
 
@@ -198,7 +222,7 @@ public class KeyRepository extends Repository implements IKeyRepository {
     /**
      * Archives a key to the repository.
      * <P>
-     * 
+     *
      * @param record key record
      * @exception EBaseException failed to archive key
      */
@@ -220,7 +244,7 @@ public class KeyRepository extends Repository implements IKeyRepository {
     /**
      * Recovers an archived key by serial number.
      * <P>
-     * 
+     *
      * @param serialNo serial number
      * @return key record
      * @exception EBaseException failed to recover key
@@ -246,7 +270,7 @@ public class KeyRepository extends Repository implements IKeyRepository {
     /**
      * Recovers an archived key by owner name.
      * <P>
-     * 
+     *
      * @param ownerName owner name
      * @return key record
      * @exception EBaseException failed to recover key
@@ -368,7 +392,7 @@ public class KeyRepository extends Repository implements IKeyRepository {
         String result = "";
 
         for (int i = 0; i < data.length; i++) {
-            result = result + "\\" + Integer.toHexString((int) data[i]);
+            result = result + "\\" + Integer.toHexString(data[i]);
         }
         return result;
     }
@@ -534,53 +558,40 @@ public class KeyRepository extends Repository implements IKeyRepository {
     }
 
     public void shutdown() {
-        //if (mKeyStatusUpdateThread != null) 
-        //        mKeyStatusUpdateThread.destroy();
+        if (mKeyStatusUpdateTask != null) {
+            mKeyStatusUpdateTask.stop();
+        }
     }
 
 }
 
-class KeyStatusUpdateThread extends Thread {
-    KeyRepository _kr = null;
-    IRepository _rr = null;
-    int _interval;
+class KeyStatusUpdateTask implements Runnable {
+    KeyRepository repository;
+    int interval;
 
-    KeyStatusUpdateThread(KeyRepository kr, IRepository rr, String name) {
-        super(name);
-        CMS.debug("new KeyStatusUpdateThread");
+    ScheduledExecutorService executorService;
 
-        _kr = kr;
-        _rr = rr;
+    public KeyStatusUpdateTask(KeyRepository repository, int interval) {
+        this.repository = repository;
+        this.interval = interval;
     }
 
-    public void setInterval(int interval) {
-        _interval = interval;
+    public void start() {
+        // schedule task to run immediately and repeat after specified interval
+        executorService = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() {
+            public Thread newThread(Runnable r) {
+                return new Thread(r, "KeyStatusUpdateTask");
+            }
+        });
+        executorService.scheduleWithFixedDelay(this, 0, interval, TimeUnit.SECONDS);
     }
 
     public void run() {
-        CMS.debug("Inside run method of KeyStatusUpdateThread");
+        repository.updateKeyStatus();
+    }
 
-        while (true) {
-            try {
-                // block the update while another thread
-                // (such as the CRL Update) is running
-                CMS.debug("About to start checkRanges");
-                synchronized (_kr.mKeyStatusUpdateThread) {
-                    CMS.debug("Starting key checkRanges");
-                    _kr.checkRanges();
-                    CMS.debug("key checkRanges done");
-
-                    CMS.debug("Starting request checkRanges");
-                    _rr.checkRanges();
-                    CMS.debug("request checkRanges done");
-                }
-            } catch (Exception e) {
-                CMS.debug("key checkRanges done");
-            }
-            try {
-                sleep(_interval * 1000);
-            } catch (InterruptedException e) {
-            }
-        }
+    public void stop() {
+        // shutdown executorService without interrupting running task
+        if (executorService != null) executorService.shutdown();
     }
 }
-- 
1.7.7.4

