Fix nsNetModuleMgr and nsNetModRegEntry leaks. r=valeski

netwerk/base/src/nsNetModRegEntry.cpp

@@ -100,8 +100,6 @@ nsNetModRegEntry::Equals(nsINetModRegEntry* aEntry, PRBool *_retVal)
     nsresult rv = NS_OK;
     *_retVal = PR_FALSE;
 
-    NS_ADDREF(aEntry);
-
     char* topic;
 
     rv = aEntry->GetTopic(&topic);

netwerk/base/src/nsNetModuleMgr.cpp

@@ -1,4 +1,4 @@
-/* -*- Mode: IDL; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
  *
  * The contents of this file are subject to the Netscape Public
  * License Version 1.1 (the "License"); you may not use this file
@@ -20,19 +20,15 @@
  * Contributor(s): 
  */
 
+#include "nsAutoLock.h"
 #include "nsNetModuleMgr.h"
 #include "nsNetModRegEntry.h"
 #include "nsEnumeratorUtils.h" // for nsArrayEnumerator
 #include "nsString2.h"
+#include "nsXPIDLString.h"
 #include "nsIEventQueue.h"
 
-// Entry routines.
-static PRBool DeleteEntry(nsISupports *aElement, void *aData) {
-    NS_ASSERTION(aElement, "null pointer");
-    NS_RELEASE(aElement);
-    return PR_TRUE;
-}
-
+nsNetModuleMgr* nsNetModuleMgr::gManager;
 
 ///////////////////////////////////
 //// nsISupports
@@ -55,48 +51,50 @@ nsNetModuleMgr::RegisterModule(const char *aTopic, nsINetNotify *aNotify)
     // XXX QI the nsINetNotify interface passed in for the interfaces
     // XXX supported by the topic.
 
-    PR_Lock(mLock);
-    nsCOMPtr<nsINetModRegEntry> newEntryI;
+    nsAutoLock lock(mLock);
     nsNetModRegEntry *newEntry = new nsNetModRegEntry(aTopic, aNotify, &rv);
     if (!newEntry)
         return NS_ERROR_OUT_OF_MEMORY;
     
-    if (NS_FAILED(rv)) return rv;
-    
-    rv = newEntry->QueryInterface(NS_GET_IID(nsINetModRegEntry), getter_AddRefs(newEntryI));
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv)) {
+        delete newEntry;
+        return rv;
+    }
+
+    nsCOMPtr<nsINetModRegEntry> newEntryI = do_QueryInterface(newEntry, &rv);
+    if (NS_FAILED(rv)) {
+        delete newEntry;
+        return rv;
+    }
 
     // Check for a previous registration
     mEntries->Count(&cnt);
     for (PRUint32 i = 0; i < cnt; i++) 
     {
-        nsINetModRegEntry* curEntry = NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i));
+        nsCOMPtr<nsINetModRegEntry> curEntry =
+            dont_AddRef(NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i)));
+
         PRBool same = PR_FALSE;
         rv = newEntryI->Equals(curEntry, &same);
-        if (NS_FAILED(rv)) {
-            PR_Unlock(mLock);
-            return rv;
-        }
+        if (NS_FAILED(rv)) return rv;
 
         // if we've already got this one registered, yank it, and replace it with the new one
         if (same) {
-            NS_RELEASE(curEntry);
             mEntries->DeleteElementAt(i);
             break;
         }
     }
 
     rv = mEntries->AppendElement(NS_STATIC_CAST(nsISupports*, newEntryI)) ? NS_OK : NS_ERROR_FAILURE;  // XXX this method incorrectly returns a bool
-    PR_Unlock(mLock);
     return rv;
 }
 
 NS_IMETHODIMP
 nsNetModuleMgr::UnregisterModule(const char *aTopic, nsINetNotify *aNotify) 
 {
-    PR_Lock(mLock);
+    nsAutoLock lock(mLock);
+
     nsresult rv;
-    PRUint32 cnt;
 
     nsCOMPtr<nsINetModRegEntry> tmpEntryI;
     nsNetModRegEntry *tmpEntry = new nsNetModRegEntry(aTopic, aNotify, &rv);
@@ -108,24 +106,21 @@ nsNetModuleMgr::UnregisterModule(const char *aTopic, nsINetNotify *aNotify)
     rv = tmpEntry->QueryInterface(NS_GET_IID(nsINetModRegEntry), getter_AddRefs(tmpEntryI));
     if (NS_FAILED(rv)) return rv;
 
+    PRUint32 cnt;
     mEntries->Count(&cnt);
     for (PRUint32 i = 0; i < cnt; i++) {
-        nsINetModRegEntry* curEntry = NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i));
-        NS_ADDREF(curEntry); // get our ref to it
+        nsCOMPtr<nsINetModRegEntry> curEntry = 
+            dont_AddRef(NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i)));
+
         PRBool same = PR_FALSE;
         rv = tmpEntryI->Equals(curEntry, &same);
-        if (NS_FAILED(rv)) {
-            PR_Unlock(mLock);
-            return rv;
-        }
+        if (NS_FAILED(rv)) return rv;
+
         if (same) {
-            NS_RELEASE(curEntry);
             mEntries->DeleteElementAt(i);
             break;
         }
-        NS_RELEASE(curEntry); // ditch our ref to it
     }
-    PR_Unlock(mLock);
     return NS_OK;
 }
 
@@ -133,64 +128,41 @@ NS_IMETHODIMP
 nsNetModuleMgr::EnumerateModules(const char *aTopic, nsISimpleEnumerator **aEnumerator) {
 
     nsresult rv;
-    PRUint32 cnt;
-    char *topic = nsnull;
-
     // get all the entries for this topic
     
-    PR_Lock(mLock);
+    nsAutoLock lock(mLock);
+
+    PRUint32 cnt;
     rv = mEntries->Count(&cnt);
     if (NS_FAILED(rv)) return rv;
 
     // create the new array
-    nsISupportsArray *topicEntries = nsnull;
-    rv = NS_NewISupportsArray(&topicEntries);
+    nsCOMPtr<nsISupportsArray> topicEntries;
+    rv = NS_NewISupportsArray(getter_AddRefs(topicEntries));
     if (NS_FAILED(rv)) return rv;
 
     // run through the main entry array looking for topic matches.
     for (PRUint32 i = 0; i < cnt; i++) {
-        nsINetModRegEntry *entry = NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i));
+        nsCOMPtr<nsINetModRegEntry> entry = 
+            dont_AddRef(NS_STATIC_CAST(nsINetModRegEntry*, mEntries->ElementAt(i)));
 
-        rv = entry->GetTopic(&topic);
-        if (NS_FAILED(rv)) {
-            NS_RELEASE(topicEntries);
-            NS_RELEASE(entry);
-            PR_Unlock(mLock);
-            return rv;
-        }
+        nsXPIDLCString topic;
+        rv = entry->GetTopic(getter_Copies(topic));
+        if (NS_FAILED(rv)) return rv;
 
-        if (!PL_strcmp(aTopic, topic)) {
-            nsCRT::free(topic);
-            topic = nsnull;
+        if (0 == PL_strcmp(aTopic, topic)) {
             // found a match, add it to the list
             rv = topicEntries->AppendElement(NS_STATIC_CAST(nsISupports*, entry)) ? NS_OK : NS_ERROR_FAILURE;  // XXX this method incorrectly returns a bool
-            if (NS_FAILED(rv)) {
-                NS_RELEASE(topicEntries);
-                NS_RELEASE(entry);
-                PR_Unlock(mLock);
-                return rv;
-            }
+            if (NS_FAILED(rv)) return rv;
         }
-        nsCRT::free(topic);
-        topic = nsnull;
-        NS_RELEASE(entry);
     }
 
-    nsISimpleEnumerator *outEnum = nsnull;
-    nsArrayEnumerator *arrEnum = new nsArrayEnumerator(topicEntries);
-    NS_RELEASE(topicEntries);
+    nsCOMPtr<nsISimpleEnumerator> enumerator;
+    rv = NS_NewArrayEnumerator(getter_AddRefs(enumerator), topicEntries);
+    if (NS_FAILED(rv)) return rv;
 
-    if (!arrEnum) {
-        return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    rv = arrEnum->QueryInterface(NS_GET_IID(nsISimpleEnumerator), (void**)&outEnum);
-    if (NS_FAILED(rv)) {
-        delete arrEnum;
-        return rv;
-    }
-    *aEnumerator = outEnum;
-    PR_Unlock(mLock);
+    *aEnumerator = enumerator;
+    NS_ADDREF(*aEnumerator);
     return NS_OK;
 }
 
@@ -206,11 +178,9 @@ nsNetModuleMgr::nsNetModuleMgr() {
 }
 
 nsNetModuleMgr::~nsNetModuleMgr() {
-    if (mEntries) {
-        mEntries->EnumerateForwards(DeleteEntry, nsnull);
-        NS_RELEASE(mEntries);
-    }
+    NS_IF_RELEASE(mEntries);
     PR_DestroyLock(mLock);
+    gManager = nsnull;
 }
 
 NS_METHOD
@@ -219,13 +189,15 @@ nsNetModuleMgr::Create(nsISupports *aOuter, REFNSIID aIID, void **aResult)
     if (aOuter)
         return NS_ERROR_NO_AGGREGATION;
 
-    static nsNetModuleMgr* mgr = nsnull;
-    if (!mgr) mgr = new nsNetModuleMgr();
-    if (!mgr) return NS_ERROR_OUT_OF_MEMORY;
-    NS_ADDREF(mgr);
-    nsresult rv = mgr->QueryInterface(aIID, aResult);
+    if (! gManager) {
+        gManager = new nsNetModuleMgr();
+        if (! gManager)
+            return NS_ERROR_OUT_OF_MEMORY;
+    }
+
+    NS_ADDREF(gManager);
+    nsresult rv = gManager->QueryInterface(aIID, aResult);
+    NS_RELEASE(gManager);
 
-    // don't release our ref as this is a singleton service.
-    //NS_RELEASE(mgr);
     return rv;
 }

netwerk/base/src/nsNetModuleMgr.h

@@ -29,6 +29,9 @@
 #include "nsISupportsArray.h"
 
 class nsNetModuleMgr : public nsINetModuleMgr {
+protected:
+    static nsNetModuleMgr* gManager;
+
 public:
     // nsISupports
     NS_DECL_ISUPPORTS
@@ -44,7 +47,7 @@ public:
     Create(nsISupports *aOuter, REFNSIID aIID, void **aResult);
 
 
-    nsISupportsArray    *mEntries;
+    nsISupportsArray   *mEntries;
     PRLock             *mLock;
 };
 

netwerk/protocol/http/src/nsHTTPChannel.cpp

@@ -649,8 +649,8 @@ nsHTTPChannel::Open(void)
     if (NS_FAILED(rv)) return rv;
 
     // Go through the external modules and notify each one.
-    nsISupports *supEntry;
-    rv = pModules->GetNext(&supEntry);
+    nsCOMPtr<nsISupports> supEntry;
+    rv = pModules->GetNext(getter_AddRefs(supEntry));
     while (NS_SUCCEEDED(rv)) 
     {
         nsCOMPtr<nsINetModRegEntry> entry = do_QueryInterface(supEntry, &rv);
@@ -669,7 +669,7 @@ nsHTTPChannel::Open(void)
             // we could do something with the return code from the external
             // module, but what????            
         }
-        rv = pModules->GetNext(&supEntry); // go around again
+        rv = pModules->GetNext(getter_AddRefs(supEntry)); // go around again
     }
 
     if (transport) {
@@ -875,8 +875,8 @@ nsresult nsHTTPChannel::OnHeadersAvailable()
     if (NS_FAILED(rv)) return rv;
 
     // Go through the external modules and notify each one.
-    nsISupports *supEntry;
-    rv = pModules->GetNext(&supEntry);
+    nsCOMPtr<nsISupports> supEntry;
+    rv = pModules->GetNext(getter_AddRefs(supEntry));
     while (NS_SUCCEEDED(rv)) 
     {
         nsCOMPtr<nsINetModRegEntry> entry = do_QueryInterface(supEntry, &rv);
@@ -895,7 +895,7 @@ nsresult nsHTTPChannel::OnHeadersAvailable()
             // we could do something with the return code from the external
             // module, but what????            
         }
-        rv = pModules->GetNext(&supEntry); // go around again
+        rv = pModules->GetNext(getter_AddRefs(supEntry)); // go around again
     }
     return NS_OK;
 }