If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

new leaks on tinderbox

RESOLVED FIXED in mozilla0.9.1

Status

()

Core
XUL
RESOLVED FIXED
17 years ago
9 years ago

People

(Reporter: dbaron, Assigned: David Hyatt)

Tracking

({mlk, topmlk})

Trunk
mozilla0.9.1
x86
Linux
mlk, topmlk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
After hewitt's carpool yesterday, leaks on tinderbox jumped from about 2K
to about 800K.  As I initially suggested to hewitt on IRC, the cause was
his change to nsOutlinerBoxObject.cpp.  The following patch (which just
undoes the fix to bug 76428) causes the leaks to go away:

Index: nsOutlinerBoxObject.cpp
===================================================================
RCS file:
/cvsroot/mozilla/layout/xul/base/src/outliner/src/nsOutlinerBoxObject.cpp,v
retrieving revision 1.10
diff -u -d -r1.10 nsOutlinerBoxObject.cpp
--- nsOutlinerBoxObject.cpp	2001/04/30 18:30:18	1.10
+++ nsOutlinerBoxObject.cpp	2001/05/01 10:18:11
@@ -149,11 +149,13 @@
   nsIOutlinerBoxObject* body = GetOutlinerBody();
   if (body)
     return body->SetView(aView);
+#if 0
   else {
     nsCOMPtr<nsISupports> suppView(do_QueryInterface(aView));
     SetPropertyAsSupports(NS_LITERAL_STRING("view").get(), suppView);
     return NS_OK;
   }
+#endif
 }
 
 NS_IMETHODIMP nsOutlinerBoxObject::GetFocused(PRBool* aFocused)
(Reporter)

Updated

17 years ago
Keywords: mlk, topmlk

Comment 1

17 years ago
nominating 091
Keywords: mozilla0.9.1
Tardy, lame attempt on my part to make up for lack of substantive review in bug
76428: nsBoxObject, inherited by nsOutlinerBoxObject, holds an mPresState strong
reference.  The PresState owns its property table, including all the strong refs
to the nsISupports values in it.  At least one nsIOutlinerView implementation
(nsMsgDBView) has a strong ref to its nsIOutlinerBoxObject (mOutliner).  Thus
the SetPropertyAsSupports completes a cycle.

Why does the view need to strongly refer to its viewed object?  Just curious.

/be

Comment 3

17 years ago
cc'ing Scott for that question. If we have to have a strong ref, we can easily
break the cycle in our ::Close method by setting mOutliner to nsnull.
(Assignee)

Comment 4

17 years ago
The cycle is normally broken by the outliner body.  Autocomplete is an unusual 
case.  Hewitt's patch added storage of the view when there is no body yet.  The 
problem is that the body is never created if you don't use the autocomplete 
widget, and so it never has an opportunity to break the cycle.
(Assignee)

Comment 5

17 years ago
When the box object dies, it should obtain the view and set the view's outliner 
box object to null.
(Assignee)

Comment 6

17 years ago
Eek, but it won't ever die.  Argh.

(Assignee)

Comment 7

17 years ago
I'll fix it.
Assignee: hewitt → hyatt
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Keep hewitt cc'd.

/be
(Assignee)

Comment 9

17 years ago
Created attachment 32898 [details] [diff] [review]
Fix leaks.
(Assignee)

Comment 10

17 years ago
fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.