669730 - Firebug causes immortal zombie compartments

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

STR:

 * Install Firebug (I have 1.8.0b4).
 * Visit http://nytimes.com in a new tab.
 * Close the tab.
 * Open about:memory, notice that nytimes.com compartment is still there.
 * Click <minimize memory usage>.  Notice that nytimes.com compartment is still there.
 * Wait a while.  The compartment never seems to disappear for me.

This may be a dupe of bug 669096, but during yesterday's MemShrink meeting, we decided to file it separately.

Comment 1

[Justin Lebar (not reading bugmail)]

This is particularly bad, because you don't have to enable Firebug for the specific page to cause a leak -- just having the extension installed and enabled is enough.

Comment 2

[Justin Lebar (not reading bugmail)]

I believe we decided this was a P1; please correct me if I'm wrong!

Comment 3

[Unknown W. Brackets]

(In reply to comment #0)

Please see:

http://groups.google.com/group/firebug/browse_thread/thread/82d393507908d496#msg_b94c8d92820aba4d

They say they fixed a memory related bug.

-[Unknown]

Comment 4

[:glob ✱]

(In reply to comment #3)
> They say they fixed a memory related bug.

i can still reproduce this issue with 1.8.0b5

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Maybe this is why Firebug uses so much memory!

Comment 6

[George K.]

I just followed the steps to reproduce with Firebug 1.80b6 (released July 18) and:

Mozilla/5.0 (Windows NT 5.1; rv:7.0a2) Gecko/20110719 Firefox/7.0a2

... and I was unable to reproduce. The nytimes.com compartment hangs around for a few seconds, but repeated refreshes of "about:memory?verbose" show it disappears quickly even without manually triggering a GC/GC+CC.

I don't see anything obvious in the Firebug tickets marked fixed in 1.8.0b6, but I'm probably only qualified to notice exceptionally obvious notes like "Fixed Mozilla Bug 669730".

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yes, we have fixed one memory leak in Firebug 1.8b5, but it doesn't seem to be related since comment #4 says it's still reproducible.

Re comment #6:
I am able to reproduce the problem only if Firebug is actually opened for the page (F12).

I have been investigating Firebug code last week, but I don't see anything that could cause the compartment to leak.

Honza

Comment 8

[Steve Fink [:sfink] [:s:]]

I get the same results. If I follow the STR in comment 0, the compartment lingers for 10 seconds or so, but then goes away (without me clicking on any of the about:memory buttons.)

If I enable Firebug for the http://nytimes.com page before closing it, I get the immortal zombie compartment. Well, ok, it's only 6 minutes old right now. But bouncing on about:memory buttons doesn't make it go away.

This is with 1.8b5 on the nightly from 4f38df646524.

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can reproduce it as well, I am working on it, I *think* I am very close.
Honza

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I have made good progress yesterday and fixed one memory leak related to the test case above.

To see the leaked-compartment in about:memory is actually the first really useful tool for finding memory leaks!

However, the compartment is still there if you have the Script panel or Console panel enabled - since both panels enable JSD debugging (Firebug debugger).

Unfortunately, I am not able to find the problem in the Firebug-JSD related code. Would it be possible to see what actual object (its interfaces or shape in case of JS objet) is holding the compartment in memory?

Anybody involved in Firebug development: I am just in the middle of releasing Firebug 1.8 final. Let me know if you think this should block the release.

Honza

Comment 11

[Nicholas Nethercote [inactive]]

(In reply to comment #10)
> 
> To see the leaked-compartment in about:memory is actually the first really
> useful tool for finding memory leaks!

Great! :)
 
> Unfortunately, I am not able to find the problem in the Firebug-JSD related
> code. Would it be possible to see what actual object (its interfaces or
> shape in case of JS objet) is holding the compartment in memory?

Not that I know of, sorry.
 
> Anybody involved in Firebug development: I am just in the middle of
> releasing Firebug 1.8 final. Let me know if you think this should block the
> release.

I'm not involved, but as a Firefox dev I would love to see this fixed, because Firefox always gets blamed for leaks even if it's the fault of the add-on.  And this one is particularly bad because, as comment 1 says, it happens even for pages that you haven't activated Firebug on.  Users always assume that if you don't activate Firebug on a page that it won't behave any differently, so it's bad when that's not true.

Comment 12

[Nicholas Nethercote [inactive]]

(In reply to comment #10)
> I have made good progress yesterday and fixed one memory leak related to the
> test case above.

Do you have a link to a bug tracker or a commit log or something?

Comment 13

[Ed Morley [:emorley]]

(In reply to comment #12)
> Do you have a link to a bug tracker or a commit log or something?

There doesn't appear to be a bug on the Firebug tracker, but there is this commit:
http://code.google.com/p/fbug/source/detail?r=11403

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to comment #11)
> I'm not involved, but as a Firefox dev I would love to see this fixed,
> because Firefox always gets blamed for leaks even if it's the fault of the
> add-on.  And this one is particularly bad because, as comment 1 says, it
> happens even for pages that you haven't activated Firebug on.  Users always
> assume that if you don't activate Firebug on a page that it won't behave any
> differently, so it's bad when that's not true.
I totally agree. Firebug Memory leaks have high priority for me and I am regularly working on finding them (this is the second memory leak I have fixed recently). I'll continue on this, but I need more time since the Firebug-JSD code (which I think is related to the leak) has been written/maintained by John J. Barton who left the project.

(In reply to comment #13)
> There doesn't appear to be a bug on the Firebug tracker, but there is this
> commit:
> http://code.google.com/p/fbug/source/detail?r=11403
That's correct. There is no bug for this in Firebug issue list and this commit is the one.

Honza

Comment 15

[Steve Fink [:sfink] [:s:]]

Note that the severity of this bug has dropped since we initially nominated as a MemShrink:P1 because initially it appeared to leak even when Firebug was not enabled for a page. Now it isn't as bad because you have to enable Firebug to trigger the leak.

But it's still very bad. Requiring a restart to free up memory when using Firebug isn't so great.

Now, to actually be helpful:

I can't guarantee there aren't other references holding the compartment alive, but there is at least a manually-added root named "JSDValue" that is holding onto an Error object with a 'fileName' property set to 'http://www.nytimes.com/':

object 0x7fffd0a99d78
class 0x7ffff64da6e0 Error
flags: none
proto <Error object at 0x7fffd0a72c58>
parent <Window object at 0x7fffd4cfa088>
private 0x7fffcfecea90
properties:
    ((Shape *) 0x7fffd4481cc0) enumerate "fileName": slot 0 = "http://www.nytimes.com/"

This root was added by this line in jsd_val.c, which is in jsd_NewValue:

        ok = JS_AddNamedValueRoot(jsdc->dumbContext, &jsdval->val, "JSDValue");

So it looks like Firebug or JSD is hanging onto one of the page's Error objects.

Comment 16

[Steve Fink [:sfink] [:s:]]

http://blog.mozilla.com/sfink/2011/08/02/zombie-hunting/

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

> So it looks like Firebug or JSD is hanging onto one of the page's Error objects.
This is great, that was actually one of my theories too! As soon as I am done with Firebug 1.8, I'll get back to this.

Thanks Steve!

Honza

Comment 18

[John J. Barton]

(In reply to comment #15)
> I can't guarantee there aren't other references holding the compartment
> alive, but there is at least a manually-added root named "JSDValue" that is
> holding onto an Error object with a 'fileName' property set to
> 'http://www.nytimes.com/':

Could be

http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#2063

Looks like errorInfo should be nulled right after use in:
http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#1848 

However, onDebug simply is not called by jsd in every case where it should. So try null errorInfo in disableDebugger as well: http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#1621 

Finally, the values in onThrow should also be nulled whenever Firebug is turned off.
http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#2003

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks John for the pointers!

I did too changes:

1) In onError
Since onDebug is not called in all cases, I think safer is *not* to store the exception object into the 'errorInfo' structure (this exception could be the object that holds the nytimes.com page in memory). Also, I think that disableDebugger is too late since it's called if the application quits, no?

It's the |exc| member here: http://code.google.com/p/fbug/source/browse/branches/firebug1.9/modules/firebug-service.js#2063

2) In onThrow
I am seeing following values set:

fbs._lastErrorScript = frame.script;
fbs._lastErrorLine = frame.line;
fbs._lastErrorDebuggr = debuggr;
fbs._lastErrorContext = debuggr.breakContext; // XXXjjb this is bad API

But these are not used anywhere in Firebug. John, do we need them?

I removed them for testing purposes

This is what I have changed, I am attaching a test build.

Honza

---

Index: modules/firebug-service.js
===================================================================
--- modules/firebug-service.js	(revision 11532)
+++ modules/firebug-service.js	(working copy)
@@ -2000,10 +2000,10 @@
             var debuggr = this.findDebugger(frame);  // sets debuggr.breakContext
             if (debuggr)
             {
-                fbs._lastErrorScript = frame.script;
-                fbs._lastErrorLine = frame.line;
-                fbs._lastErrorDebuggr = debuggr;
-                fbs._lastErrorContext = debuggr.breakContext; // XXXjjb this is bad API
+                //fbs._lastErrorScript = frame.script;
+                //fbs._lastErrorLine = frame.line;
+                //fbs._lastErrorDebuggr = debuggr;
+                //fbs._lastErrorContext = debuggr.breakContext; // XXXjjb this is bad API
             }
             else
                 delete fbs._lastErrorDebuggr;
@@ -2062,7 +2062,7 @@
         // global to pass info to onDebug. Some duplicate values to support different apis
         errorInfo = { errorMessage: message, sourceName: fileName, lineNumber: lineNo,
                 message: message, fileName: fileName, lineNo: lineNo,
-                columnNumber: pos, flags: flags, category: "js", errnum: errnum, exc: exc };
+                columnNumber: pos, flags: flags, category: "js", errnum: errnum };
 
         if (message=="out of memory")  // bail
         {

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Firebug test build 1

Steve, do you see any difference? What object holds the page in memory now?
Honza

Comment 21

[Steve Fink [:sfink] [:s:]]

Unfortunately, I still see the compartment persist. The root that I observed previously, however, is gone. So this is progress! Now I need to expand my instrumentation to see if I can find the new root.

Comment 22

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #21)
> Unfortunately, I still see the compartment persist. The root that I observed
> previously, however, is gone. So this is progress! Now I need to expand my
> instrumentation to see if I can find the new root.
Sounds good! I am also seeing the compartment persist, but this is the first real effective strategy how to find mem-leaks in Firebug.

Thanks Steve for the help!
Honza

Comment 23

[Steve Fink [:sfink] [:s:]]

I am seeing no explicitly named roots (like the "JSDValue" one from before.) I do see a bunch of cross-compartment wrappers (which aren't real root global roots, but if they survive a global GC they are real compartmental roots).

Here's what the latest iteration of my root dumping tool says:

[{name:"global object", pointer:"0x7fffd417c028"},
{name:"nsXPCWrappedJS[nsIDOMEventListener,0x7fffd0b9e800:0x7fffd0b2eb60].mJSObj", pointer:"0x7fffd0aaee48"},
{name:"mScopeObject", pointer:"0x7fffd4175088"},
{name:"mScopeObject", pointer:"0x7fffd4175088"}]
                        
The nsIDOMEventListener seems promising. I don't know how to open that up to figure out what event handler it is.

Comment 24

[Steve Fink [:sfink] [:s:]]

(gdb) p ((nsXPCWrappedJS*)0x7fffd0b9e800)->mJSObj
$80 = 0x7fffd0aaee48 [Object Function "fireContentLoadedEvent"] COMPARTMENT(principals=0x7fffda6b7828 "http://www.nytimes.com/")

So apparently it's a listener for a "ContentLoadedEvent" or something with a similar name.

The handler function itself is unnamed and empty (it decompiles to a single bytecode: 'stop'.)

Comment 25

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Fink [:sfink] from comment #24)
> (gdb) p ((nsXPCWrappedJS*)0x7fffd0b9e800)->mJSObj
> $80 = 0x7fffd0aaee48 [Object Function "fireContentLoadedEvent"]
> COMPARTMENT(principals=0x7fffda6b7828 "http://www.nytimes.com/")
fireContentLoadedEvent listener is implemented on the nytimes.com page (comes from prototype lib). Isn't there yet another root?

Honza

Comment 27

[Andrew McCreight [:mccr8]]

I'll try dredging this with my heap dumping tools in the next couple of weeks.

Comment 28

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yeah, any pointer to what object could be left by Firebug (e.g what kind of a listener) would be great help.

Thanks!
Honza

Comment 29

[Nicholas Nethercote [inactive]]

Bug 669096 just got fixed and it supposedly was causing Firebug to leak sometimes.  Can anyone confirm if it has fixed any/all of the remaining leaks?

Comment 30

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Unfortunately, Firebug is still holding the compartment in memory. My current steps are:

1) Install Firebug 1.9a5 (1.9b1 will be released on Friday)
2) Load www.google.com
3) Press F12 to open Firebug UI
4) The HTML panel should be selected by default (Console, Script, Net should be disabled)
5) Just close the tab
6) Check about:memory (www.google.com compartment still there, even if pressing Minimize Memory Usage, several times)

I did some changes to avoid leaking references to the page document/window/elements (will be in b1), but there is still something holding the compartment in memory.

I am pretty sure that there is something related to the HTML panel (if different panel, e.g. DOM is selected by default, the leak is gone).

Does anyone has a chance to see more details about the reference that is holding the compartment? E.g. if it's a listener, variable, closure, etc.?

Honza

Comment 31

[Roman R.]

(In reply to Steve Fink [:sfink] from comment #15)
> But it's still very bad. Requiring a restart to free up memory when using
> Firebug isn't so great.

What seems to help me, instead of restarting Firefox, is disabling all Firebug's panels (maybe in combination with disabling JavaScript temporarily).

Comment 32

[(mostly gone) XtC4UaLL [:xtc4uall]]

FWIW, the Compartment is still an Issue using Firefox 13 + Firebug 1.10a2

│  ├───3,574,512 B (05.75%) -- compartment(http://www.nytimes.com/)

Comment 33

[Jorge Villalobos [:jorgev] (he/him)]

Honza, any updates on this?

Comment 34

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Still working on this. I am now using Cycle Collector listener (nsICycleCollectorListener) API to dynamically get CC object graph in the memory. 

The analysis of the graph can be done using this extension:
https://addons.mozilla.org/en-US/firefox/addon/cycle-collector-analyzer/

So, the whole process of mem-hunting is faster, but it's still hard to match objects in the graph with actual objects in the Firebug source...

Honza

Comment 35

[Danial Horton]

theres a tool for finding cc roots in bug 466157

Comment 36

[Andrew McCreight [:mccr8]]

Honza, is there any update here?  Is there something we can do to help you with this aside from bug 722749?

Comment 37

[Nicholas Nethercote [inactive]]

Now that bug 695480 has landed, the situation here may be greatly improved.  Honza, are you able to redo some testing?

Comment 38

[Nicholas Nethercote [inactive]]

The steps to reproduce in comment 30 worked for me.  I found that existing zombie compartments tended to be replaced when opening Firebug on a subsequent page.  However, for the Huffington Post that didn't happen, and all the zombie compartments from it stayed around even after opening Firebug on other sites.

However, with a recent build (post bug 695480) I wasn't able to reproduce any zombie compartments, even after doing a small amount of fiddling in the Firebug panel.

It would be great if someone who knows how to use Firebug more comprehensively than me could try to replicate my findings!

Comment 39

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Nicholas Nethercote [:njn] from comment #37)
> Now that bug 695480 has landed, the situation here may be greatly improved. 
> Honza, are you able to redo some testing?
yes, I did and I don't see any zombie compartments. I yet need some more feedback from Firebug users, but the situation looks much better
Honza

Comment 40

[Nicholas Nethercote [inactive]]

> yes, I did and I don't see any zombie compartments.

Great!  That matches my experience (bug 695480 comment 50).

Comment 41

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

If we really did fix the leaks in Firebug that's absolutely fantastic.

Comment 42

[Sebastian Zartner [:sebo]]

Another test case, which confirms that it's working now:

http://code.google.com/p/fbug/issues/detail?id=5582

Sebastian

Comment 43

[Nicholas Nethercote [inactive]]

The just-released Firefox 15 prevents this leak -- see bug 695480 comment 50.