From b9f51eb366c98647544d1d090cb9dbd0d29c6e09 Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" Date: Tue, 16 Jun 2015 14:12:54 -0400 Subject: Fixed thread leaks during shutdown. Various codes have been modified to properly stop threads during shutdown. A new ID attribute has been added to the LDAP connection factory classes to help identify leaking threads. https://fedorahosted.org/pki/ticket/1327 --- .../src/com/netscape/cmscore/apps/CMSEngine.java | 18 +++++++++++----- .../authentication/PasswdUserDBAuthentication.java | 5 +++-- .../cmscore/cert/CrossCertPairSubsystem.java | 2 +- .../src/com/netscape/cmscore/dbs/DBSubsystem.java | 2 +- .../com/netscape/cmscore/ldap/LdapConnModule.java | 2 +- .../netscape/cmscore/ldap/LdapPublishModule.java | 4 ++-- .../cmscore/ldapconn/LdapAnonConnFactory.java | 16 ++++++++++++--- .../cmscore/ldapconn/LdapBoundConnFactory.java | 18 +++++++++++++--- .../src/com/netscape/cmscore/logging/LogQueue.java | 9 ++++---- .../cmscore/profile/LDAPProfileSubsystem.java | 2 +- .../cmscore/selftests/SelfTestSubsystem.java | 2 ++ .../session/LDAPSecurityDomainSessionTable.java | 10 ++++++++- .../session/SecurityDomainSessionTable.java | 3 +++ .../com/netscape/cmscore/usrgrp/UGSubsystem.java | 2 +- .../src/com/netscape/cmscore/util/Debug.java | 24 ++++++---------------- 15 files changed, 75 insertions(+), 44 deletions(-) (limited to 'base/server/cmscore/src/com/netscape/cmscore') diff --git a/base/server/cmscore/src/com/netscape/cmscore/apps/CMSEngine.java b/base/server/cmscore/src/com/netscape/cmscore/apps/CMSEngine.java index 02c8ab41d..fa2c8147f 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/apps/CMSEngine.java +++ b/base/server/cmscore/src/com/netscape/cmscore/apps/CMSEngine.java @@ -976,14 +976,14 @@ public class CMSEngine implements ICMSEngine { return new LdapAuthInfo(); } - public ILdapConnFactory getLdapBoundConnFactory() + public ILdapConnFactory getLdapBoundConnFactory(String id) throws ELdapException { - return new LdapBoundConnFactory(); + return new LdapBoundConnFactory(id); } - public ILdapConnFactory getLdapAnonConnFactory() + public ILdapConnFactory getLdapAnonConnFactory(String id) throws ELdapException { - return new LdapAnonConnFactory(); + return new LdapAnonConnFactory(id); } public IRequestEncoder getHttpRequestEncoder() { @@ -1079,7 +1079,7 @@ public class CMSEngine implements ICMSEngine { } } - public LDAPConnection getBoundConnection(String host, int port, + public LDAPConnection getBoundConnection(String id, String host, int port, int version, LDAPSSLSocketFactoryExt fac, String bindDN, String bindPW) throws LDAPException { return new LdapBoundConnection(host, port, version, fac, @@ -1796,6 +1796,14 @@ public class CMSEngine implements ICMSEngine { shutdownSubsystems(mFinalSubsystems); shutdownSubsystems(mDynSubsystems); shutdownSubsystems(mStaticSubsystems); + + if (mSDTimer != null) { + mSDTimer.cancel(); + } + + if (mSecurityDomainSessionTable != null) { + mSecurityDomainSessionTable.shutdown(); + } } /** diff --git a/base/server/cmscore/src/com/netscape/cmscore/authentication/PasswdUserDBAuthentication.java b/base/server/cmscore/src/com/netscape/cmscore/authentication/PasswdUserDBAuthentication.java index 2905f3c75..692dc49d6 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/authentication/PasswdUserDBAuthentication.java +++ b/base/server/cmscore/src/com/netscape/cmscore/authentication/PasswdUserDBAuthentication.java @@ -95,8 +95,8 @@ public class PasswdUserDBAuthentication implements IAuthManager, IPasswdUserDBAu return; mBaseDN = dbs.getBaseDN(); - mConnFactory = new LdapBoundConnFactory(3, 20, ldapinfo, dbs.getLdapAuthInfo()); - mAnonConnFactory = new LdapAnonConnFactory(3, 20, ldapinfo); + mConnFactory = new LdapBoundConnFactory("PasswdUserDBAuthentication", 3, 20, ldapinfo, dbs.getLdapAuthInfo()); + mAnonConnFactory = new LdapAnonConnFactory("PasswdUserDBAuthentication", 3, 20, ldapinfo); log(ILogger.LL_INFO, CMS.getLogMessage("CMSCORE_AUTH_INIT_AUTH", mName)); } @@ -242,6 +242,7 @@ public class PasswdUserDBAuthentication implements IAuthManager, IPasswdUserDBAu try { // disconnect all outstanding connections in the factory if (mConnFactory != null) mConnFactory.reset(); + if (mAnonConnFactory != null) mAnonConnFactory.reset(); } catch (ELdapException e) { log(ILogger.LL_FAILURE, e.toString()); } diff --git a/base/server/cmscore/src/com/netscape/cmscore/cert/CrossCertPairSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/cert/CrossCertPairSubsystem.java index b0feca8c2..29ec0f54f 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/cert/CrossCertPairSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/cert/CrossCertPairSubsystem.java @@ -118,7 +118,7 @@ public class CrossCertPairSubsystem implements ICrossCertPairSubsystem { mBaseDN = ldapConfig.getString(PROP_BASEDN, null); - mLdapConnFactory = new LdapBoundConnFactory(); + mLdapConnFactory = new LdapBoundConnFactory("CrossCertPairSubsystem"); if (mLdapConnFactory != null) mLdapConnFactory.init(ldapConfig); diff --git a/base/server/cmscore/src/com/netscape/cmscore/dbs/DBSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/dbs/DBSubsystem.java index a6133310c..2de9945f7 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/dbs/DBSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/dbs/DBSubsystem.java @@ -636,7 +636,7 @@ public class DBSubsystem implements IDBSubsystem { // initialize LDAP connection factory // by default return error if server is down at startup time. - mLdapConnFactory = new LdapBoundConnFactory(true); + mLdapConnFactory = new LdapBoundConnFactory("DBSubsystem", true); tmpConfig = (IConfigStore) (((PropConfigStore) mConfig).clone()); tmpConfig.putString(PROP_BASEDN, mBaseDN); diff --git a/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapConnModule.java b/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapConnModule.java index 859e442fa..3710b8a22 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapConnModule.java +++ b/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapConnModule.java @@ -92,7 +92,7 @@ public class LdapConnModule implements ILdapConnModule { CMS.debug("Creating LdapBoundConnFactory for LdapConnModule."); mLdapConnFactory = - new LdapBoundConnFactory(minConns, maxConns, (LdapConnInfo) connInfo, authInfo); + new LdapBoundConnFactory("LDAPConnModule", minConns, maxConns, (LdapConnInfo) connInfo, authInfo); mInited = true; diff --git a/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapPublishModule.java b/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapPublishModule.java index c4ff20515..c7a04601d 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapPublishModule.java +++ b/base/server/cmscore/src/com/netscape/cmscore/ldap/LdapPublishModule.java @@ -119,7 +119,7 @@ public class LdapPublishModule implements ILdapPublishModule { mAuthority = authority; mPubProcessor = p; mConfig = config; - mLdapConnFactory = new LdapBoundConnFactory(); + mLdapConnFactory = new LdapBoundConnFactory("LdapPublishModule"); mLdapConnFactory.init(mConfig.getSubStore("ldap")); // initMappers(config); @@ -135,7 +135,7 @@ public class LdapPublishModule implements ILdapPublishModule { mAuthority = authority; mConfig = config; - mLdapConnFactory = new LdapBoundConnFactory(); + mLdapConnFactory = new LdapBoundConnFactory("LdapPublishModule"); mLdapConnFactory.init(mConfig.getSubStore("ldap")); initMappers(config); diff --git a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapAnonConnFactory.java b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapAnonConnFactory.java index dfc974e0b..20e0d65b1 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapAnonConnFactory.java +++ b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapAnonConnFactory.java @@ -35,6 +35,9 @@ import com.netscape.certsrv.logging.ILogger; * authentication dn and password. */ public class LdapAnonConnFactory implements ILdapConnFactory { + + protected String id; + protected int mMinConns = 5; protected int mMaxConns = 1000; protected LdapConnInfo mConnInfo = null; @@ -60,10 +63,14 @@ public class LdapAnonConnFactory implements ILdapConnFactory { * Constructor for initializing from the config store. * must be followed by init(IConfigStore) */ - public LdapAnonConnFactory() { + public LdapAnonConnFactory(String id) { + CMS.debug("Creating LdapAnonConnFactory(" + id + ")"); + this.id = id; } - public LdapAnonConnFactory(boolean defErrorIfDown) { + public LdapAnonConnFactory(String id, boolean defErrorIfDown) { + CMS.debug("Creating LdapAnonConnFactory(" + id + ")"); + this.id = id; mDefErrorIfDown = defErrorIfDown; } @@ -75,8 +82,10 @@ public class LdapAnonConnFactory implements ILdapConnFactory { * the maximum number of clones of this connection one wants to allow. * @param serverInfo server connection info - host, port, etc. */ - public LdapAnonConnFactory(int minConns, int maxConns, + public LdapAnonConnFactory(String id, int minConns, int maxConns, LdapConnInfo connInfo) throws ELdapException { + CMS.debug("Creating LdapAnonConnFactory(" + id + ")"); + this.id = id; init(minConns, maxConns, connInfo); } @@ -405,6 +414,7 @@ public class LdapAnonConnFactory implements ILdapConnFactory { // ok only if no connections outstanding. public synchronized void reset() throws ELdapException { + CMS.debug("Destroying LdapAnonConnFactory(" + id + ")"); if (mNumConns == mTotal) { for (int i = 0; i < mNumConns; i++) { try { diff --git a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapBoundConnFactory.java b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapBoundConnFactory.java index 5be645b56..2ac4085c7 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapBoundConnFactory.java +++ b/base/server/cmscore/src/com/netscape/cmscore/ldapconn/LdapBoundConnFactory.java @@ -36,6 +36,9 @@ import com.netscape.certsrv.logging.ILogger; * be shared by multiple threads and cloned. */ public class LdapBoundConnFactory implements ILdapBoundConnFactory { + + protected String id; + protected int mMinConns = 5; protected int mMaxConns = 1000; protected LdapConnInfo mConnInfo = null; @@ -71,10 +74,14 @@ public class LdapBoundConnFactory implements ILdapBoundConnFactory { * Constructor for initializing from the config store. * must be followed by init(IConfigStore) */ - public LdapBoundConnFactory() { + public LdapBoundConnFactory(String id) { + CMS.debug("Creating LdapBoundConnFactor(" + id + ")"); + this.id = id; } - public LdapBoundConnFactory(boolean defErrorIfDown) { + public LdapBoundConnFactory(String id, boolean defErrorIfDown) { + CMS.debug("Creating LdapBoundConnFactor(" + id + ")"); + this.id = id; mDefErrorIfDown = defErrorIfDown; } @@ -98,8 +105,10 @@ public class LdapBoundConnFactory implements ILdapBoundConnFactory { * the maximum number of clones of this connection or separate connections one wants to allow. * @param serverInfo server connection info - host, port, etc. */ - public LdapBoundConnFactory(int minConns, int maxConns, + public LdapBoundConnFactory(String id, int minConns, int maxConns, LdapConnInfo connInfo, LdapAuthInfo authInfo) throws ELdapException { + CMS.debug("Creating LdapBoundConnFactory(" + id + ")"); + this.id = id; init(minConns, maxConns, connInfo, authInfo); } @@ -459,11 +468,13 @@ public class LdapBoundConnFactory implements ILdapBoundConnFactory { */ public synchronized void reset() throws ELdapException { + CMS.debug("Destroying LdapBoundConnFactory(" + id + ")"); if (mNumConns == mTotal) { for (int i = 0; i < mNumConns; i++) { try { mConns[i].disconnect(); } catch (LDAPException e) { + e.printStackTrace(); } mConns[i] = null; } @@ -472,6 +483,7 @@ public class LdapBoundConnFactory implements ILdapBoundConnFactory { log(ILogger.LL_INFO, "disconnecting masterConn"); mMasterConn.disconnect(); } catch (LDAPException e) { + e.printStackTrace(); log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_LDAPCONN_CANNOT_RESET", e.toString())); diff --git a/base/server/cmscore/src/com/netscape/cmscore/logging/LogQueue.java b/base/server/cmscore/src/com/netscape/cmscore/logging/LogQueue.java index 32af1bf2c..751a4cd63 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/logging/LogQueue.java +++ b/base/server/cmscore/src/com/netscape/cmscore/logging/LogQueue.java @@ -64,7 +64,8 @@ public class LogQueue implements ILogQueue { if (mListeners == null) return; for (int i = 0; i < mListeners.size(); i++) { - mListeners.elementAt(i).shutdown(); + ILogEventListener listener = mListeners.elementAt(i); + listener.shutdown(); } } @@ -75,8 +76,9 @@ public class LogQueue implements ILogQueue { */ public void addLogEventListener(ILogEventListener listener) { //Make sure we don't have duplicated listener - if (!mListeners.contains(listener)) + if (!mListeners.contains(listener)) { mListeners.addElement(listener); + } } /** @@ -104,9 +106,6 @@ public class LogQueue implements ILogQueue { // incorrect log message. // ConsoleError.send(new SystemEvent(CMS.getUserMessage("CMS_LOG_EVENT_FAILED", // event.getEventType(), e.toString()))); - - // Don't do this again. - removeLogEventListener(mListeners.elementAt(i)); } } } diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java index 83ae5078d..cc2e43dfa 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java @@ -77,7 +77,7 @@ public class LDAPProfileSubsystem IConfigStore cs = CMS.getConfigStore(); IConfigStore dbCfg = cs.getSubStore("internaldb"); - dbFactory = CMS.getLdapBoundConnFactory(); + dbFactory = CMS.getLdapBoundConnFactory("LDAPProfileSubsystem"); dbFactory.init(dbCfg); mConfig = config; diff --git a/base/server/cmscore/src/com/netscape/cmscore/selftests/SelfTestSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/selftests/SelfTestSubsystem.java index 2a2cf8b2c..ad1a1b0b8 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/selftests/SelfTestSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/selftests/SelfTestSubsystem.java @@ -1888,6 +1888,8 @@ public class SelfTestSubsystem instance.shutdownSelfTest(); } + + mLogger.shutdown(); } /** diff --git a/base/server/cmscore/src/com/netscape/cmscore/session/LDAPSecurityDomainSessionTable.java b/base/server/cmscore/src/com/netscape/cmscore/session/LDAPSecurityDomainSessionTable.java index 064ae7ecb..bbc9f1a79 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/session/LDAPSecurityDomainSessionTable.java +++ b/base/server/cmscore/src/com/netscape/cmscore/session/LDAPSecurityDomainSessionTable.java @@ -50,7 +50,7 @@ public class LDAPSecurityDomainSessionTable m_timeToLive = timeToLive; IConfigStore cs = CMS.getConfigStore(); IConfigStore internaldb = cs.getSubStore("internaldb"); - mLdapConnFactory = CMS.getLdapBoundConnFactory(); + mLdapConnFactory = CMS.getLdapBoundConnFactory("LDAPSecurityDomainSessionTable"); mLdapConnFactory.init(internaldb); } @@ -300,4 +300,12 @@ public class LDAPSecurityDomainSessionTable return ret; } + + public void shutdown() { + try { + mLdapConnFactory.reset(); + } catch (ELdapException e) { + CMS.debug(e); + } + } } diff --git a/base/server/cmscore/src/com/netscape/cmscore/session/SecurityDomainSessionTable.java b/base/server/cmscore/src/com/netscape/cmscore/session/SecurityDomainSessionTable.java index 497f42f7a..c7fe25599 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/session/SecurityDomainSessionTable.java +++ b/base/server/cmscore/src/com/netscape/cmscore/session/SecurityDomainSessionTable.java @@ -102,4 +102,7 @@ public class SecurityDomainSessionTable public int getSize() { return m_sessions.size(); } + + public void shutdown() { + } } diff --git a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java index a2655bf82..d1277279e 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java @@ -137,7 +137,7 @@ public final class UGSubsystem implements IUGSubsystem { mBaseDN = ldapConfig.getString(PROP_BASEDN, null); - mLdapConnFactory = new LdapBoundConnFactory(); + mLdapConnFactory = new LdapBoundConnFactory("UGSubsystem"); mLdapConnFactory.init(ldapConfig); } catch (EBaseException e) { if (CMS.isPreOpMode()) diff --git a/base/server/cmscore/src/com/netscape/cmscore/util/Debug.java b/base/server/cmscore/src/com/netscape/cmscore/util/Debug.java index 036573467..83c0e1b06 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/util/Debug.java +++ b/base/server/cmscore/src/com/netscape/cmscore/util/Debug.java @@ -20,11 +20,12 @@ package com.netscape.cmscore.util; import java.io.FileOutputStream; import java.io.OutputStream; import java.io.PrintStream; -import java.text.SimpleDateFormat; import java.util.Date; import java.util.Hashtable; import java.util.StringTokenizer; +import org.apache.commons.lang.time.FastDateFormat; + import com.netscape.certsrv.base.IConfigStore; import com.netscape.certsrv.base.ISubsystem; import com.netscape.cmsutil.util.Utils; @@ -35,21 +36,9 @@ public class Debug private static Debug mInstance = new Debug(); private static boolean mShowCaller = false; - /* This dateformatter is used to put the date on each - debug line. But the DateFormatter is not thread safe, - so I create a thread-local DateFormatter for each thread - */ + // FastDateFormat is a thread-safe replacement for SimpleDateFormat private static String DATE_PATTERN = "dd/MMM/yyyy:HH:mm:ss"; - private static ThreadLocal mFormatObject = new ThreadLocal() { - protected synchronized SimpleDateFormat initialValue() { - return new SimpleDateFormat(DATE_PATTERN); - } - }; - - /* the dateformatter should be accessed with this function */ - private static SimpleDateFormat getDateFormatter() { - return mFormatObject.get(); - } + private static FastDateFormat df = FastDateFormat.getInstance(DATE_PATTERN); public static final boolean ON = false; public static final int OBNOXIOUS = 1; @@ -146,9 +135,8 @@ public class Debug private static void outputTraceMessage(String t) { if (!TRACE_ON) return; - SimpleDateFormat d = getDateFormatter(); - if (mOut != null && d != null) { - mOut.println("[" + d.format(new Date()) + "][" + Thread.currentThread().getName() + "]: " + t); + if (mOut != null) { + mOut.println("[" + df.format(new Date()) + "][" + Thread.currentThread().getName() + "]: " + t); mOut.flush(); } } -- cgit