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

15 files changed, 75 insertions, 44 deletions

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<SimpleDateFormat> mFormatObject = new ThreadLocal<SimpleDateFormat>() {
-        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();
         }
     }