19474 - [MLK] Big nsCharsetAlias2 leak

Status: RESOLVED FIXED

Description

[Patrick C. Beard]

Because of the way you're using services, there's a particular execution path
that fails to release an nsCharsetAlias2 service. Here's a patch that fixes the
leak:

Index: mozilla/htmlparser/src/nsScanner.cpp
===================================================================
RCS file: /cvsroot/mozilla/htmlparser/src/nsScanner.cpp,v
retrieving revision 3.64
diff -c -2 -r3.64 nsScanner.cpp
*** nsScanner.cpp	1999/11/17 00:25:33	3.64
--- nsScanner.cpp	1999/11/21 00:52:56
***************
*** 136,144 ****
      return res;

!   nsICharsetAlias* calias = nsnull;
!   res = nsServiceManager::GetService(kCharsetAliasCID,
!                                        kICharsetAliasIID,
!                                        (nsISupports**)&calias);
!
    NS_ASSERTION( nsnull != calias, "cannot find charset alias");
    nsAutoString charsetName = aCharset;
--- 136,140 ----
      return res;

!   NS_WITH_SERVICE(nsICharsetAlias, calias, kCharsetAliasCID, &res);
    NS_ASSERTION( nsnull != calias, "cannot find charset alias");
    nsAutoString charsetName = aCharset;
***************
*** 153,157 ****
      // different, need to change it
      res = calias->GetPreferred(aCharset, charsetName);
-     nsServiceManager::ReleaseService(kCharsetAliasCID, calias);

      if(NS_FAILED(res) && (kCharsetUninitialized == mCharsetSource) )
--- 149,152 ----
***************
*** 163,170 ****
      mCharsetSource = aSource;

!     nsICharsetConverterManager * ccm = nsnull;
!     res = nsServiceManager::GetService(kCharsetConverterManagerCID,
!                                        nsCOMTypeInfo<
nsICharsetConverterManager>::GetIID(),
!                                        (nsISupports**)&ccm);
      if(NS_SUCCEEDED(res) && (nsnull != ccm))
      {
--- 158,162 ----
      mCharsetSource = aSource;

!     NS_WITH_SERVICE(nsICharsetConverterManager, ccm,
kCharsetConverterManagerCID, &res);
      if(NS_SUCCEEDED(res) && (nsnull != ccm))
      {
***************
*** 177,181 ****
           mUnicodeDecoder = decoder;
        }
-       nsServiceManager::ReleaseService(kCharsetConverterManagerCID, ccm);
      }
    }
--- 169,172 ----

Comment 1

[Patrick C. Beard]

In addition, you're keeping an unsafe reference to the service in
nsMetaCharsetObserver::nsMetaCharsetObserver(). This patch keeps an extra
reference around. In essence you were using nsServiceManager::GetService() when
you should have been using NS_WITH_SERVICE, and in this code you should really be
using nsServiceManager::GetService().

Index: mozilla/intl/chardet/src/nsMetaCharsetObserver.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/chardet/src/nsMetaCharsetObserver.cpp,v
retrieving revision 1.18
diff -c -2 -r1.18 nsMetaCharsetObserver.cpp
*** nsMetaCharsetObserver.cpp	1999/11/10 05:55:58	1.18
--- nsMetaCharsetObserver.cpp	1999/11/21 01:00:34
***************
*** 94,99 ****
    mAlias = nsnull;
    NS_WITH_SERVICE(nsICharsetAlias, calias, kCharsetAliasCID, &res);
!   if(NS_SUCCEEDED(res))
       mAlias = calias;
  }
  //-------------------------------------------------------------------------
--- 94,101 ----
    mAlias = nsnull;
    NS_WITH_SERVICE(nsICharsetAlias, calias, kCharsetAliasCID, &res);
!   if(NS_SUCCEEDED(res)) {
       mAlias = calias;
+      NS_ADDREF(mAlias);
+   }
  }
  //-------------------------------------------------------------------------
***************
*** 102,105 ****
--- 104,108 ----
    // should we release mAlias
    PR_AtomicDecrement(& g_InstanceCount);
+   NS_IF_RELEASE(mAlias);
  }

Comment 2

[Patrick C. Beard]

With both of these patches applied, viewer goes from having 567 leaks
(52724 bytes) down to only 1 when just starting up the test page.

Comment 3

[Frank Tang]

Thanks for beard's catch. Please mark this FIXED once you check it in. Thanks.

Comment 4

[Frank Tang]

beard check in Nov 28

Comment 5

[Frank Tang]

reassign the QA contact back to beard.