From 748605a324266bb515a3d1124bc55deb3be4df71 Mon Sep 17 00:00:00 2001 From: Endi Sukma Dewata Date: Tue, 25 Sep 2012 21:40:04 -0500 Subject: 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 --- base/ca/src/com/netscape/ca/CRLIssuingPoint.java | 26 +++++- .../certsrv/dbs/certdb/ICertificateRepository.java | 12 --- .../cmscore/dbs/CertificateRepository.java | 100 +++++++++------------ 3 files changed, 68 insertions(+), 70 deletions(-) (limited to 'base') 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() { -- cgit