leak on starting and exiting Firebird

RESOLVED FIXED in Firefox0.9

Status

()

Firefox
General
P2
normal
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({mlk})

unspecified
Firefox0.9
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

14 years ago
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.
(Assignee)

Comment 1

14 years ago
Created attachment 139280 [details]
stacks for creation and registration as an observer
(Assignee)

Comment 2

14 years ago
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.
(Assignee)

Comment 3

14 years ago
...which would make it basically the same as bug 170022.
(Assignee)

Comment 4

14 years ago
Created attachment 139283 [details] [diff] [review]
patch
(Assignee)

Updated

14 years ago
Attachment #139283 - Flags: review?(bryner)
(Assignee)

Comment 5

14 years ago
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.
(Assignee)

Comment 6

14 years ago
Created attachment 139288 [details] [diff] [review]
patch
Attachment #139283 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #139283 - Flags: review?(bryner)
(Assignee)

Updated

14 years ago
Attachment #139288 - Flags: review?(bryner)
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → Firebird0.9
Attachment #139283 - Flags: review+
(Assignee)

Comment 7

14 years ago
Created attachment 139294 [details] [diff] [review]
patch

This patch fixes the suite as well.
Attachment #139288 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #139294 - Flags: review?(bryner)
(Assignee)

Updated

14 years ago
Attachment #139288 - Flags: review?(bryner)
Attachment #139288 - Flags: review+
Attachment #139294 - Flags: review?(bryner) → review+
(Assignee)

Updated

14 years ago
Attachment #139294 - Flags: superreview?(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
Attachment #139294 - Flags: superreview?(brendan) → superreview+
(Assignee)

Comment 9

14 years ago
Fix checked in to trunk, 2004-01-17 14:41 -0800.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

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

Comment 11

14 years ago
any ideas when this bug appeared? was it on the 20030114 trunk?
(Assignee)

Comment 12

14 years ago
It's been around since before Firebird forked from Seamonkey.
You need to log in before you can comment on or make changes to this bug.