Closed
Bug 53298
Opened 24 years ago
Closed 24 years ago
nsXPCWrappedNative not being freed on JS attribute setter call
Categories
(Core :: XPConnect, defect, P3)
Core
XPConnect
Tracking
()
VERIFIED
INVALID
People
(Reporter: jonsmirl, Assigned: jband_mozilla)
Details
Attachments
(2 files)
My component has a method that starts out:
Component.prototype = {
get request() { return this.req; },
set request(newval) { return this.req = newval; },
get response() { return this.resp; },
set response(newval) { return this.resp = newval; },
setDocumentHandler: function (h) {
this.handler = h;
},
When exiting my app, the request and response objects are not freed, but
handler is freed. I've enabled refcount tracing and the best theory I can come
up with is that the nsXPCWrappedNative passed into a setter function does not
get freed.
refcount trace: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=15059
I believe the unpaired addref is the first AddRef 4. This is the addref for
mObject in nsXPCWrappedNative. I put a break point in ~nsXPCWrappedNative and
this instance is never destroyed.
The only other thing I can come up with is that 'handler' is getting freed
and 'request/response' and not. 'request/response' are attributes using get/set.
What are other reasons the nsXPCWrappedNative wouldn't be destroyed?
Did I play mahjong right and get the right addref? Ignore this bug if I got the
wrong addref.
Comment 1•24 years ago
|
||
cc self
Assignee | ||
Comment 2•24 years ago
|
||
Jon, It's hard to say what would do this without all the source (and then still
probably plenty hard). I'd be *very* surprised if a leak of this type
and magnitude were still lurking in xpconnect.
I compile xpconnect with XPC_DUMP_AT_SHUTDOWN defined. This dumps counts of
unreleased objects at shutdown time. It is common to see zero wrappers leaked.
Rarely more than a few.
The most likely thing is that these objects are reachable from roots in your JS
code.
If you recompile *both* js/src and xpconnect/src with GC_MARK_DEBUG #defined,
then xpconnect will try to force the js engine to dump the entire gc heap at
shutdown. See...
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/nsXPConnect.cpp#261
...There are shutdown ordering issue that can keep this from happening. But it
generally works.
You can add -DGC_MARK_DEBUG to the "DEFINES" list in each makefile.win
This gives a big list of what JS gcthings remain and how they are related to
some root that keeps them in place. Redirecting this from stdio to file makes
this easier to look at.
Also in xpconect you'll see XPC_DUMP_AT_SHUTDOWN. If you #define this and enable
it with an env setting (se xpclog.h) you should get more detailed info about
remaining xpconnect internal objects.
xpc wrapped natives are initially created with two refs - one held by the native
caller to "wrapNative" and the other waiting to be released by a call from a JS
finalizer. As long as the refcount is two or higher xpconnect will keep the
JSObject rooted. When all native refs are released (and the ref count
transitions from two to one) then the JS root is released and the wrapper is at
the mercy of the JS gc. If you see a wrapper with refcount of one then it is
most certainly still held is JS because it is reachable by some rooted thing
(not xpconnect itself).
Oh, given that JS finalization can pass through xpconnect to cause a
Release that can then change the JS reference graph, sometimes you will see
additional objects get free'd during a second consecutive GC call.
You can look at xpcshell...
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/shell/xpcshell.cpp#293
...for a Js callable example of how to look under the covers to get an idea of
how much stuff got freed in a call to gc.
Let me know.
Also, my app does not have any code interfacing directly to the JS engine. The
JS engine is only invoked via XPConnect to support components written in JS.
Another clue is that I get the nsComponentManager held past shut down error
when I load the JS component. Could the same thing that is holding the
component manager be stopping the garbage collection of my xpcwrappers?
Assignee | ||
Comment 5•24 years ago
|
||
The "nsComponentManager held past shut down" *assert* always happens on first
run when the JS loader gets and hold a reference to it early.
Not using the jsapi is no gaurantee that your JS code will not produce a
reference graph where some wrapped natives are reachable from roots.
I'm still working on this. After spending three days in the debugger looking at
XPConnect I'm starting to understand how it works.
My current theory is that it is a shutdown ordering problem. I have a bunch of
intertwined references between C and JS objects. It may take four or five
passes through the garbage collector to free them all. The chain is built on
forward references so it looks like each pass through GC only frees a single
object. Should JS keep looping through GC at shutdown as long as an object is
getting freed each pass?
The problem is much more complex than a simple bug in JS setter. Anyway, this
is just my current theory, I still haven't located the exact problem. I also
haven't proven to myself that I haven't managed to build a reference loop.
Assignee | ||
Comment 7•24 years ago
|
||
A few more points...
- You might notice in xpconnect some "SystemIsBeingShutDown" stuff. When the
xpconnect service gets released during xpcom shutdown and remaining connections
between JS and native objects via xpconnect are severed. Release calls are not
made and finalizers will not result in Release calls. There we far too many
problems with instability if these calls are made. The assumption is that the
application is reponsible for not leaving a mess before shutting down xpcom.
- Is it absolutely neessary to you that all things are cleaned up on exit? Or
are you just trying to be sure that things do not leak along the way during
normal execution?
- There are various roots in the JS engine. These include the concept of the
'newborn' gcthings. These are the most recent things created. The idea is that
you don't want the engine to collect something you;ve just created before you
have the chance to hook it up to something that is rooted.
I'm just trying to make sure everything doesn't leak. Web servers are different
than browsers, servers can run for months without being shutdown. They may also
create and run the same object a hundred times a second. In this kind of
environment even leaking ten bytes can be fatal after a couple of days.
Apache supports memory pools that are created and detroyed on each request thus
preventing leaks. I can't see how I can use these with the JS engine and
garbage collection.
I've narrowed this down a lot more.
1) I have an interface implemented in C.
2) I store this interface in a component written in JS.
3) I then retrieve the interface from the JS component back into another C
component. rv = m_chain->GetRequest(getter_AddRefs(request));
4) I then set the reference in the JS component to null.
Step 3 is what causes the problem. If I use another C object to temporarily
hold the reference or if I just skip step 3 the problem goes away.
Two theories:
1) the refcount on the xpcwrapper is wrong after step 3
2) All the refcounts are ok, but the linkage pattern is too complicated to GC
I'm checking out #1 right now.
Reporter | ||
Comment 10•24 years ago
|
||
This is really looking like a problem with JS get attribute. I implemented the
same operation two ways:
1) get request() { return this.req; },
2) getReq: function () { return this.req; },
I used identical calls from the C component
1) rv = m_chain->GetRequest(getter_AddRefs(request));
2) rv = m_chain->GetReq(getter_AddRefs(request));
Version one fails and version two works. Failure being that the XPCwrapper for
the request variable never gets freed.
Is there something special about refcounting XPCOM attributes versus methods
that I'm not aware of?
Reporter | ||
Comment 11•24 years ago
|
||
I give up on tracking down the source of this. This doesn't seem to be a fatal
problem in my situation. If I run my server in a loop for 1000 page requests
the same 34 objects are left that I get when I run it once.
I still think there is something wrong in the release of native wrappers during
shutdown and GC. For example, If I simply load a do-nothing js component from a
C app and then free it, a copy of the backstagepass object (mozjsloader) gets
stuck in memory. It's stuck because of the mObj reference in the wrapper class.
If I put a syntax error in the JS component, load the component and exit, the
backstagepass object gets created and released properly.
Only a few wrappers are getting stuck, not all of them. The ones getting stuck
have to do with the initial creatation of the component. The ones created by
running the component get freed.
Fixing this requires a better understanding of the Javascript implementation
than I currently have.
Reporter | ||
Comment 12•24 years ago
|
||
Here's part of the problem that is causing me to have 100s of leaks on exit.
The moz component loader is not unrooting the component. I'm still working on
why some of my instance variables don't get freed.
static PRIntn PR_CALLBACK
UnloadAndReleaseModules(PLHashEntry *he, PRIntn i, void *arg)
{
nsIModule *module = NS_STATIC_CAST(nsIModule *, he->value);
nsIComponentManager *mgr = NS_STATIC_CAST(nsIComponentManager *, arg);
PRBool canUnload;
if (NS_SUCCEEDED(module->CanUnload(mgr, &canUnload)) && canUnload) {
NS_RELEASE(module);
/* XXX need to unroot the global for the module as well */
nsCRT::free((char *)he->key);
return HT_ENUMERATE_REMOVE;
}
return HT_ENUMERATE_NEXT;
}
Assignee | ||
Comment 13•24 years ago
|
||
Jon, All the globals do eventually get unrooted in ~mozJSComponentLoader.
I think the thing that causes the most leaks is the funky shutdown order. As
I mentioned before, when xpconnect is shutdown it severs all the connections
between wrappers and objects (without releasing the native objects). This
protects us from a bunch of nasty bugs that bite when the JS finalizers get
called during shutdown when so much xpcom and application machinery is off.
Without this break the finalizers propagate to call xpcom Releases (as is
normal) and lots of code does things that cause crashes during the shutdown
state.
The thing is, xpconnect is shutdown before the JS loader is shutdown. So all the
stuff loaded earlier and still held either by the loader or the service manager
(or any other caller) is likely to be involved in shutdown leaks.
These are only shutdown leaks.
Assignee | ||
Comment 14•24 years ago
|
||
Assignee | ||
Comment 15•24 years ago
|
||
The file I just attached can be saved as getter.js and run as...
"xpcshell getter.js"
I wrote this a while back while experimenting. It uses the nsIXPCTestNoisy test
object to show lifetime of a wrapped native while setting and unsetting it as a
property of some other object. This is not an exact duplicate of the conditions
Jon suggested, but does somewhat refute the assertion of this bug's title. You
can change the 'if(0)' to 'if(1)' in the file and rerun it to show that using
functions or getter/setter has the same (apparently correct) lifetime
implications.
I'm marking this bug invalid because the problem it suggests is not (I believe)
a real problem. I think it has been determined that any lifetime problems Jon is
seeing are of other origins.
Status: UNCONFIRMED → RESOLVED
Closed: 24 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 16•24 years ago
|
||
I agree the problem is from another origin but I can't track down where it is
coming from. My solution was to just rewrite my code to avoid it.
The simple explaination of the my problem is that a bunch of my C objects that
I share with Javascript are not getting freed. They aren't getting freed
because their XPCNativeWrapper is holding them. My current best guess is some
interaction between my code and the 'newborn' gcthings code may be holding an
unwanted reference to one of my objects. This reference then causes a whole
chain of references to be held.
I don't believe this is a simple problem with getter/setter. I initially blamed
getter/setter because they were creating the XPCWrapper that didn't get freed.
You need to log in
before you can comment on or make changes to this bug.
Description
•