Last Comment Bug 231266 - leak on starting and exiting Firebird
: leak on starting and exiting Firebird
Status: RESOLVED FIXED
[patch]
: mlk
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86 Linux
: P2 normal (vote)
: Firefox0.9
Assigned To: David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-01-17 11:26 PST by David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
Modified: 2004-01-18 22:09 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stacks for creation and registration as an observer (9.84 KB, text/plain)
2004-01-17 11:27 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
no flags Details
patch (1.40 KB, patch)
2004-01-17 11:42 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
bryner: review+
Details | Diff | Splinter Review
patch (3.95 KB, patch)
2004-01-17 12:17 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
bryner: review+
Details | Diff | Splinter Review
patch (8.05 KB, patch)
2004-01-17 13:44 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
bryner: review+
brendan: superreview+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 11:26:21 PST
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-7 (busy September 14-25) 2004-01-17 11:27:32 PST
Created attachment 139280 [details]
stacks for creation and registration as an observer
Comment 2 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 11:32:08 PST
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-7 (busy September 14-25) 2004-01-17 11:35:21 PST
...which would make it basically the same as bug 170022.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 11:42:56 PST
Created attachment 139283 [details] [diff] [review]
patch
Comment 5 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 11:57:54 PST
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-7 (busy September 14-25) 2004-01-17 12:17:05 PST
Created attachment 139288 [details] [diff] [review]
patch
Comment 7 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 13:44:57 PST
Created attachment 139294 [details] [diff] [review]
patch

This patch fixes the suite as well.
Comment 8 Brendan Eich [:brendan] 2004-01-17 13:53:52 PST
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-7 (busy September 14-25) 2004-01-17 14:43:58 PST
Fix checked in to trunk, 2004-01-17 14:41 -0800.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-17 14:46:02 PST
And I changed the second addObserver in navigator.js to removeObserver before I
checked in.
Comment 11 Hussam Al-Tayeb 2004-01-18 22:02:31 PST
any ideas when this bug appeared? was it on the 20030114 trunk?
Comment 12 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2004-01-18 22:09:03 PST
It's been around since before Firebird forked from Seamonkey.

Note You need to log in before you can comment on or make changes to this bug.