Bug 308275 ? Leaks related to nssCKFWInstance_CreateMutex (edit)

destroy the token objects when the token goes away.
r=nelsonb

5 files changed, 34 insertions, 29 deletions

diff --git a/security/nss/lib/ckfw/ckfw.h b/security/nss/lib/ckfw/ckfw.h
index 1aa0b4545..a6dfcede3 100644
--- a/security/nss/lib/ckfw/ckfw.h
+++ b/security/nss/lib/ckfw/ckfw.h
@@ -2198,7 +2198,8 @@ nssCKFWObject_Create
 NSS_EXTERN void
 nssCKFWObject_Finalize
 (
-  NSSCKFWObject *fwObject
+  NSSCKFWObject *fwObject,
+  PRBool removeFromHash
 );
 
 /*
diff --git a/security/nss/lib/ckfw/object.c b/security/nss/lib/ckfw/object.c
index d8bace45c..df1e99e36 100644
--- a/security/nss/lib/ckfw/object.c
+++ b/security/nss/lib/ckfw/object.c
@@ -236,7 +236,8 @@ nssCKFWObject_Create
 NSS_IMPLEMENT void
 nssCKFWObject_Finalize
 (
-  NSSCKFWObject *fwObject
+  NSSCKFWObject *fwObject,
+  PRBool removeFromHash
 )
 {
   nssCKFWHash *mdObjectHash;
@@ -255,10 +256,12 @@ nssCKFWObject_Finalize
       fwObject->fwToken, fwObject->mdInstance, fwObject->fwInstance);
   }
 
-  mdObjectHash = nssCKFWToken_GetMDObjectHash(fwObject->fwToken);
-  if( (nssCKFWHash *)NULL != mdObjectHash ) {
-    nssCKFWHash_Remove(mdObjectHash, fwObject->mdObject);
-  }
+  if (removeFromHash) {
+    mdObjectHash = nssCKFWToken_GetMDObjectHash(fwObject->fwToken);
+    if( (nssCKFWHash *)NULL != mdObjectHash ) {
+      nssCKFWHash_Remove(mdObjectHash, fwObject->mdObject);
+    }
+ }
 
   if (fwObject->fwSession) {
     nssCKFWSession_DeregisterSessionObject(fwObject->fwSession, fwObject);
diff --git a/security/nss/lib/ckfw/session.c b/security/nss/lib/ckfw/session.c
index feacd49ba..2fc65c712 100644
--- a/security/nss/lib/ckfw/session.c
+++ b/security/nss/lib/ckfw/session.c
@@ -253,7 +253,7 @@ nss_ckfw_session_object_destroy_iterator
 )
 {
   NSSCKFWObject *fwObject = (NSSCKFWObject *)value;
-  nssCKFWObject_Finalize(fwObject);
+  nssCKFWObject_Finalize(fwObject, PR_TRUE);
 }
 
 /*
@@ -1384,7 +1384,7 @@ nssCKFWSession_CreateObject
     if( CK_FALSE == nssCKFWHash_Exists(fwSession->sessionObjectHash, fwObject) ) {
       *pError = nssCKFWHash_Add(fwSession->sessionObjectHash, fwObject, fwObject);
       if( CKR_OK != *pError ) {
-        nssCKFWObject_Finalize(fwObject);
+        nssCKFWObject_Finalize(fwObject, PR_TRUE);
         return (NSSCKFWObject *)NULL;
       }
     }
@@ -1500,7 +1500,7 @@ nssCKFWSession_CopyObject
       if( CK_FALSE == nssCKFWHash_Exists(fwSession->sessionObjectHash, rv) ) {
         *pError = nssCKFWHash_Add(fwSession->sessionObjectHash, rv, rv);
         if( CKR_OK != *pError ) {
-          nssCKFWObject_Finalize(rv);
+          nssCKFWObject_Finalize(rv, PR_TRUE);
           return (NSSCKFWObject *)NULL;
         }
       }
diff --git a/security/nss/lib/ckfw/token.c b/security/nss/lib/ckfw/token.c
index 055d8a82b..e12225532 100644
--- a/security/nss/lib/ckfw/token.c
+++ b/security/nss/lib/ckfw/token.c
@@ -336,6 +336,22 @@ nss_ckfwtoken_session_iterator
   return;
 }
 
+static void
+nss_ckfwtoken_object_iterator
+(
+  const void *key,
+  void *value,
+  void *closure
+)
+{
+  /*
+   * Remember that the fwToken->mutex is locked
+   */
+  NSSCKFWObject *fwObject = (NSSCKFWObject *)value;
+  (void)nssCKFWObject_Finalize(fwObject, CK_FALSE);
+  return;
+}
+
 /*
  * nssCKFWToken_Destroy
  *
@@ -368,9 +384,14 @@ nssCKFWToken_Destroy
                                                                 (void *)NULL);
   nssCKFWHash_Destroy(fwToken->sessions);
 
+  /* session objects go away when their sessions are removed */
   if (fwToken->sessionObjectHash) {
     nssCKFWHash_Destroy(fwToken->sessionObjectHash);
   }
+
+  /* free up the token objects */
+  nssCKFWHash_Iterate(fwToken->mdObjectHash, nss_ckfwtoken_object_iterator, 
+                                                                (void *)NULL);
   if (fwToken->mdObjectHash) {
     nssCKFWHash_Destroy(fwToken->mdObjectHash);
   }
diff --git a/security/nss/tests/memleak/ignored b/security/nss/tests/memleak/ignored
index 52f9bbb6a..f6f769fe1 100644
--- a/security/nss/tests/memleak/ignored
+++ b/security/nss/tests/memleak/ignored
@@ -3,26 +3,6 @@
 */SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/secmod_ModuleInit/builtinsC_Initialize/NSSCKFWC_Initialize/nssCKFWInstance_Create/NSSArena_Create/nssArena_Create/arena_add_pointer/nssPointerTracker_initialize/call_once/**
 */SECMOD_LoadModule/SECMOD_LoadPKCS11Module/secmod_ModuleInit/builtinsC_Initialize/NSSCKFWC_Initialize/nssCKFWInstance_Create/NSSArena_Create/nssArena_Create/arena_add_pointer/nssPointerTracker_initialize/call_once/PR_CallOnce/myOnceFunction/PR_NewLock/PR_Calloc/calloc
 
-#308275
-*/main/*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_isRootSlot/pk11_FindObjectByTemplate/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-*/main/*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_isRootSlot/pk11_FindObjectByTemplate/*/*/*/*/*/PR_NewLock/calloc
-*/main/*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_isRootSlot/pk11_FindObjectByTemplate/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_isRootSlot/pk11_FindObjectByTemplate/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-*/main/*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_FindObjectByTemplate/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-*/*/*/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_FindObjectByTemplate/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssPKIObjectCollection_Traverse/convert_cert/STAN_GetCERTCertificate/stan_GetCERTCertificate/fill_CERTCertificateFields/nssTrust_GetCERTCertTrustForCert/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/find_objects_by_template/find_objects/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssPKIObjectCollection_Traverse/convert_cert/STAN_GetCERTCertificate/stan_GetCERTCertificate/fill_CERTCertificateFields/nssTrust_GetCERTCertTrustForCert/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/find_objects_by_template/find_objects/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssToken_TraverseCertificates/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-selfserv/stan_GetCERTCertificate/fill_CERTCertificateFields/nssTrust_GetCERTCertTrustForCert/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/find_objects_by_template/find_objects/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssToken_TraverseCertificates/*/*/*/*/*/PR_NewLock/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssToken_TraverseCertificates/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-selfserv/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssToken_TraverseCertificates/builtinsC_FindObjects/NSSCKFWC_FindObjects/nssCKFWFindObjects_Next/nssCKFWObject_Create/nssCKFWInstance_CreateMutex/nssCKFWMutex_Create/PR_NewLock/PR_Calloc/calloc
-selfserv/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssToken_TraverseCertificates/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-selfserv/main/server_main/SSL_ConfigSecureServer/PR_CallOnceWithArg/serverCAListSetup/CERT_GetSSLCACerts/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssPKIObjectCollection_Traverse/convert_cert/stan_GetCERTCertificate/nssTrust_GetCERTCertTrustForCert/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/find_objects_by_template/find_objects/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-selfserv/PK11_TraverseSlotCerts/NSSTrustDomain_TraverseCertificates/nssPKIObjectCollection_Traverse/convert_cert/STAN_GetCERTCertificate/fill_CERTCertificateFields/nssTrust_GetCERTCertTrustForCert/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/*/*/*/*/*/PR_NewLock/calloc
-selfserv/NSSTrustDomain_TraverseCertificates/nssPKIObjectCollection_Traverse/convert_cert/STAN_GetCERTCertificate/fill_CERTCertificateFields/nssTrustDomain_FindTrustForCertificate/nssToken_FindTrustForCertificate/*/*/*/*/*/*/PR_NewLock/PR_Calloc/calloc
-ocspclnt/main/NSS_Init/SECMOD_LoadModule/SECMOD_LoadModule/SECMOD_LoadPKCS11Module/PK11_InitSlot/pk11_isRootSlot/pk11_FindObjectByTemplate/*/*/*/*/*/PR_NewLock/calloc
-
 #367374
 */main/PR_Init/_PR_ImplicitInitialization/**
 */main/PR_Init/_PR_InitCMon/ExpandMonitorCache/PR_Calloc/calloc