[MLK] Big nsCharsetAlias2 leak

RESOLVED FIXED in M12

Status

()

P3
normal
RESOLVED FIXED
19 years ago
19 years ago

People

(Reporter: beard, Assigned: ftang)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

19 years ago
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 ----
(Reporter)

Comment 1

19 years ago
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);
  }
(Reporter)

Updated

19 years ago
Summary: Big nsCharsetAlias2 leak → [MLK] Big nsCharsetAlias2 leak
(Reporter)

Comment 2

19 years ago
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.
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

19 years ago
Thanks for beard's catch. Please mark this FIXED once you check it in. Thanks.
(Assignee)

Updated

19 years ago
Target Milestone: M12
(Assignee)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

19 years ago
beard check in Nov 28

Updated

19 years ago
QA Contact: teruko → ftang
(Assignee)

Comment 5

19 years ago
reassign the QA contact back to beard.
QA Contact: ftang → beard
You need to log in before you can comment on or make changes to this bug.