[MLK] nsEventListenerManager is leaking a nsIPrincipal object

VERIFIED FIXED in M11

Status

()

Core
Security
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: Scott MacGregor, Assigned: Scott MacGregor)

Tracking

Trunk
All
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

18 years ago
I recently started running mailnews urls through the webshell when displaying a
message. We have a lot of things that hang off mailnews nsIURI subclasses. So
leaking a url is really expensive for us.

I noticed that urls which are run through the webshell all leak! I've spent the
last two days trying to figure out where they are and I now have fixes and am
filing bugs for these fixes so I can get them reviewed and checked in.

nsEventListenerManager::AddScriptEventListener
 leaks a nsIPrincipal object when it calls globalData->GetPrincipal because it
never releases the returned object.

This (after Bug #14815 is fixed) causes the nsCodebasePrincipal object
underneath the nsIPrincipal interface to get leaked. This in turn causes us to
leak the nsIURI for the document because the codebase principal holds onto the
uri.

Using a nsCOMPtr fixes the leak.

Norris, can I get a code review for this so I can check it into the tree?

I didn't bother attachig a patch...here's the change:
  nsCOMPtr<nsIPrincipal> prin;   // added a com ptr to prevent a leak
  *snip*
  if (NS_FAILED(globalData->GetPrincipal(getter_AddRefs(prin))))

Thanks,
-Scott
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: M11
(Assignee)

Comment 1

18 years ago
accepting...marking as m11.
(Assignee)

Updated

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

Comment 2

18 years ago
Turns out Brendan fixed this (and some other cleanup) stuff in the same file.
I'm checkin in his changes (with myself as reviewer).

Comment 3

18 years ago
Bulk moving all Browser Security bugs to new Security: General component.  The 
previous Security component for Browser will be deleted.
Component: Security → Security: General

Comment 4

18 years ago
Verified per mscott's comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.