From d0892d28759b51680d488922a9ff6e901f87b195 Mon Sep 17 00:00:00 2001
From: Endi Sukma Dewata <edewata@redhat.com>
Date: Tue, 25 Sep 2012 21:40:04 -0500
Subject: [PATCH] Fixed synchronization problem in CertificateRepository.

Some synchronized methods in CertificateRepository have been moved
into CertStatusUpdateThread to avoid blocking other synchronized
methods too long.

Ticket #313
---
 .../cmscore/dbs/CertificateRepository.java         | 112 +++++++++++----------
 1 file changed, 61 insertions(+), 51 deletions(-)

diff --git a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
index 2d3dead75474406183274b8b4c2e580eb6bf473e..0cdd854b4657ed479ec85f67ba80d0aef816a0ee 100644
--- a/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
+++ b/base/common/src/com/netscape/cmscore/dbs/CertificateRepository.java
@@ -89,8 +89,6 @@ public class CertificateRepository extends Repository
     CertStatusUpdateTask certStatusUpdateTask;
     RetrieveModificationsTask retrieveModificationsTask;
 
-    IRepository requestRepository;
-
     /**
      * Constructs a certificate repository.
      */
@@ -230,9 +228,6 @@ public class CertificateRepository extends Repository
             boolean listenToCloneModifications) {
 
         CMS.debug("In setCertStatusUpdateInterval " + interval);
-        synchronized (this) {
-            this.requestRepository = requestRepository;
-        }
 
         // stop running tasks
         if (certStatusUpdateTask != null) {
@@ -261,62 +256,36 @@ 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_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_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_REVOKED_EXPIRED_SEARCH"));
+        transitRevokedExpiredCertificates();
+        CMS.getLogger().log(ILogger.EV_SYSTEM, ILogger.S_OTHER,
+                CMS.getLogMessage("CMSCORE_DBS_FINISH_REVOKED_EXPIRED_SEARCH"));
     }
 
-    public synchronized void processRevokedCerts(IElementProcessor p, String filter, int pageSize)
+    public void processRevokedCerts(IElementProcessor p, String filter, int pageSize)
             throws EBaseException {
-
-        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");
+        certStatusUpdateTask.processRevokedCerts(p, filter, pageSize);
     }
 
     /**
@@ -1943,12 +1912,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;
     }
 
@@ -1963,7 +1935,45 @@ 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 synchronized void processRevokedCerts(IElementProcessor p, String filter, int pageSize)
+            throws EBaseException {
+
+        CMS.debug("Starting processRevokedCerts (entered lock)");
+        ICertRecordList list = repository.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");
     }
 
     public void stop() {
-- 
1.7.11.2

