Fixed synchronization problem in CertificateRepository.

Some synchronized methods in CertificateRepository may block
modifyCeritifcateRecord() too long, so they have been moved
into CRLIssuingPoint and CertStatusUpdateThread.

Ticket #313

3 files changed, 68 insertions, 70 deletions

diff --git a/base/ca/src/com/netscape/ca/CRLIssuingPoint.java b/base/ca/src/com/netscape/ca/CRLIssuingPoint.java
index c29bc326d..db30d3988 100644
--- a/base/ca/src/com/netscape/ca/CRLIssuingPoint.java
+++ b/base/ca/src/com/netscape/ca/CRLIssuingPoint.java
@@ -62,6 +62,8 @@ import com.netscape.certsrv.common.Constants;
 import com.netscape.certsrv.common.NameValuePairs;
 import com.netscape.certsrv.dbs.EDBNotAvailException;
 import com.netscape.certsrv.dbs.IElementProcessor;
+import com.netscape.certsrv.dbs.certdb.ICertRecord;
+import com.netscape.certsrv.dbs.certdb.ICertRecordList;
 import com.netscape.certsrv.dbs.certdb.ICertificateRepository;
 import com.netscape.certsrv.dbs.certdb.IRevocationInfo;
 import com.netscape.certsrv.dbs.crldb.ICRLIssuingPointRecord;
@@ -1868,7 +1870,29 @@ public class CRLIssuingPoint implements ICRLIssuingPoint, Runnable {
      */
     public void processRevokedCerts(IElementProcessor p)
             throws EBaseException {
-        mCertRepository.processRevokedCerts(p, getFilter(), mPageSize);
+        CertRecProcessor cp = (CertRecProcessor) p;
+        String filter = getFilter();
+
+        // NOTE: dangerous cast.
+        // correct way would be to modify interface and add
+        // accessor but we don't want to touch the interface
+        CertificateRepository cr = (CertificateRepository) mCertRepository;
+
+        synchronized (cr.certStatusUpdateTask) {
+            CMS.debug("Starting processRevokedCerts (entered lock)");
+            ICertRecordList list = mCertRepository.findCertRecordsInList(
+                    filter,
+                    new String[] {
+                            ICertRecord.ATTR_ID, ICertRecord.ATTR_REVO_INFO, "objectclass"
+                    },
+                    "serialno",
+                    mPageSize);
+
+            int totalSize = list.getSize();
+
+            list.processCertRecords(0, totalSize - 1, cp);
+            CMS.debug("processRevokedCerts done");
+        }
     }
 
     /**
diff --git a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
index d44280237..d54cfb353 100644
--- a/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
+++ b/base/common/src/com/netscape/certsrv/dbs/certdb/ICertificateRepository.java
@@ -28,7 +28,6 @@ import netscape.security.x509.X509CertImpl;
 
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.MetaInfo;
-import com.netscape.certsrv.dbs.IElementProcessor;
 import com.netscape.certsrv.dbs.ModificationSet;
 import com.netscape.certsrv.dbs.repository.IRepository;
 import com.netscape.cmscore.dbs.CertificateRepository.RenewableCertificateCollection;
@@ -513,16 +512,5 @@ public interface ICertificateRepository extends IRepository {
      */
     public void removeCertRecords(BigInteger beginS, BigInteger endS) throws EBaseException;
 
-    /**
-     * Builds a list of revoked certificates to put them into CRL.
-     * Calls certificate record processor to get necessary data
-     * from certificate records.
-     * This also regenerates CRL cache.
-     *
-     * @param cp certificate record processor
-     * @exception EBaseException if an error occurred in the database.
-     */
-    public void processRevokedCerts(IElementProcessor cp, String filter, int pageSize) throws EBaseException;
-
     public void shutdown();
 }
diff --git a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
index ea041bfe5..c2ecb8710 100644
--- a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
+++ b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
@@ -49,7 +49,6 @@ import com.netscape.certsrv.dbs.IDBSSession;
 import com.netscape.certsrv.dbs.IDBSearchResults;
 import com.netscape.certsrv.dbs.IDBSubsystem;
 import com.netscape.certsrv.dbs.IDBVirtualList;
-import com.netscape.certsrv.dbs.IElementProcessor;
 import com.netscape.certsrv.dbs.Modification;
 import com.netscape.certsrv.dbs.ModificationSet;
 import com.netscape.certsrv.dbs.certdb.ICertRecord;
@@ -86,10 +85,8 @@ public class CertificateRepository extends Repository
     private int mTransitMaxRecords = 1000000;
     private int mTransitRecordPageSize = 200;
 
-    CertStatusUpdateTask certStatusUpdateTask;
-    RetrieveModificationsTask retrieveModificationsTask;
-
-    IRepository requestRepository;
+    public CertStatusUpdateTask certStatusUpdateTask;
+    public RetrieveModificationsTask retrieveModificationsTask;
 
     /**
      * Constructs a certificate repository.
@@ -230,9 +227,6 @@ public class CertificateRepository extends Repository
             boolean listenToCloneModifications) {
 
         CMS.debug("In setCertStatusUpdateInterval " + interval);
-        synchronized (this) {
-            this.requestRepository = requestRepository;
-        }
 
         // stop running tasks
         if (certStatusUpdateTask != null) {
@@ -256,62 +250,31 @@ public class CertificateRepository extends Repository
         }
 
         CMS.debug("In setCertStatusUpdateInterval scheduling cert status update every " + interval + " seconds.");
-        certStatusUpdateTask = new CertStatusUpdateTask(this, interval);
+        certStatusUpdateTask = new CertStatusUpdateTask(this, requestRepository, interval);
         certStatusUpdateTask.start();
     }
 
-    /**
-     * This method blocks when another thread (such as the CRL Update) is running
-     */
-    public synchronized void updateCertStatus() {
+    public void updateCertStatus() throws EBaseException {
 
         CMS.debug("In updateCertStatus()");
 
-        try {
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_START_VALID_SEARCH"));
-            transitInvalidCertificates();
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_FINISH_VALID_SEARCH"));
-
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_START_EXPIRED_SEARCH"));
-            transitValidCertificates();
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_FINISH_EXPIRED_SEARCH"));
-
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_START_REVOKED_EXPIRED_SEARCH"));
-            transitRevokedExpiredCertificates();
-            CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
-                    CMS.getLogMessage("CMSCORE_DBS_FINISH_REVOKED_EXPIRED_SEARCH"));
-
-            CMS.debug("Starting cert checkRanges");
-            checkRanges();
-            CMS.debug("cert checkRanges done");
-
-            CMS.debug("Starting request checkRanges");
-            requestRepository.checkRanges();
-            CMS.debug("request checkRanges done");
-
-        } catch (Exception e) {
-            CMS.debug("updateCertStatus done: " + e.toString());
-        }
-    }
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_START_VALID_SEARCH"));
+        transitInvalidCertificates();
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_FINISH_VALID_SEARCH"));
 
-    public synchronized void processRevokedCerts(IElementProcessor p, String filter, int pageSize)
-            throws EBaseException {
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_START_EXPIRED_SEARCH"));
+        transitValidCertificates();
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_FINISH_EXPIRED_SEARCH"));
 
-        CMS.debug("Starting processRevokedCerts (entered lock)");
-        ICertRecordList list = findCertRecordsInList(filter,
-                new String[] { ICertRecord.ATTR_ID, ICertRecord.ATTR_REVO_INFO, "objectclass" },
-                "serialno",
-                pageSize);
-
-        int totalSize = list.getSize();
-
-        list.processCertRecords(0, totalSize - 1, p);
-        CMS.debug("processRevokedCerts done");
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_START_REVOKED_EXPIRED_SEARCH"));
+        transitRevokedExpiredCertificates();
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_FINISH_REVOKED_EXPIRED_SEARCH"));
     }
 
     /**
@@ -1929,12 +1892,15 @@ public class CertificateRepository extends Repository
 class CertStatusUpdateTask implements Runnable {
 
     CertificateRepository repository;
+    IRepository requestRepository;
+
     int interval;
 
     ScheduledExecutorService executorService;
 
-    public CertStatusUpdateTask(CertificateRepository repository, int interval) {
+    public CertStatusUpdateTask(CertificateRepository repository, IRepository requestRepository, int interval) {
         this.repository = repository;
+        this.requestRepository = requestRepository;
         this.interval = interval;
     }
 
@@ -1949,7 +1915,27 @@ class CertStatusUpdateTask implements Runnable {
     }
 
     public void run() {
+        try {
+            CMS.debug("About to start updateCertStatus");
+            updateCertStatus();
+
+        } catch (EBaseException e) {
+            CMS.debug("updateCertStatus done: " + e.toString());
+        }
+    }
+
+    public synchronized void updateCertStatus() throws EBaseException {
+        CMS.debug("Starting updateCertStatus (entered lock)");
         repository.updateCertStatus();
+        CMS.debug("updateCertStatus done");
+
+        CMS.debug("Starting cert checkRanges");
+        repository.checkRanges();
+        CMS.debug("cert checkRanges done");
+
+        CMS.debug("Starting request checkRanges");
+        requestRepository.checkRanges();
+        CMS.debug("request checkRanges done");
     }
 
     public void stop() {