231266 - leak on starting and exiting Firebird

Status: RESOLVED FIXED

(Firefox :: General, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Starting and exiting firebird, there's a bunch of stuff leaked.  I think the
root is a cycle through JS involving the observer service and an implementation
of nsIObserver in JS.  The wrapped JS object in this cycle roots its global
object, etc.

The information I have on this cycle is:

08741478 object 0x880f808 XPCWrappedNative_NoHelper via
nsXPCWrappedJS::mJSObj(Object).observe(Function).__parent__(Call).service(XPCWrappedNative_NoHelper).

plus a pair of stacks that I'll attach.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: stacks for creation and registration as an observer

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This looks like a problem:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.265&mark=201,211-212#198

The closure creates a cycle.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

...which would make it basically the same as bug 170022.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, this patch (and the one in bug 170022) isn't really enough.

Some simple use of |dump| shows that we're leaving one |observer| object around
for the lifetime of the app for each browser window opened.  I also see some
pretty nasty assertions in XPConnect:

This is definitely related:
###!!! ASSERTION: Potential deadlock between
XPCJSRuntime::mMapLockMonitor@8789238 and Monitor@bfeaa830: 'Error', file
/builds/trunk/mozilla/xpcom/threads/nsAutoLock.cpp, line 299
Break: at file /builds/trunk/mozilla/xpcom/threads/nsAutoLock.cpp, line 299
###!!! ASSERTION:
XPConnect is being called on a scope without a 'Components' property!
 
This is pretty much always bad. It usually means that native code is
making a callback to an interface implemented in JavaScript, but the
document where the JS object was created has already been cleared and the
global properties of that document's window are *gone*. Generally this
indicates a problem that should be addressed in the design and use of the
callback code.
: 'Error', file
/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 554
Break: at file
/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 554


And these might be related as well (although I suspect they're not):
###!!! ASSERTION: !!! xpconnect/xbl check - wrapper has extra proto: 'proto2 &&
proto2 == our_proto', file
/builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 1386


If this object had a QueryInterface method that implement
nsISupportsWeakReference that might help -- but I fear it would help too much,
and the observer would go away before the window.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This patch fixes the suite as well.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 139294 [details] [diff] [review]
patch

Sure, the extra function wrapping the object initializer containing a method
was unnecessary, a garbage-entrainment hazard. Was this pattern copied/mutated
anywhere else?	sr=me.

/be

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2004-01-17 14:41 -0800.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

And I changed the second addObserver in navigator.js to removeObserver before I
checked in.

Comment 11

[Hussam Al-Tayeb]

any ideas when this bug appeared? was it on the 20030114 trunk?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's been around since before Firebird forked from Seamonkey.