Ulrich von Zadow wrote:
Thanks for the fast reply :-).

John Bandhauer wrote:
 >
 > The thing holding the wrapper and thus your native object alive is the
 > JS engine. The JSObject that you get from GetJSObject and return via vp
 > is apparently reachable from some root object in JS (or JS garbage
 > collection is not occurring?).

This is the javascript code:

function test() {
    var myCamera = obj._myRenderer.getCamera();
    var myPosition = myCamera.position;
}

As you can see, we're not running in the browser - this is standalone xpconnect for a 3d engine. We're calling JS_GC regularly and myPosition doesn't get destroyed. myCamera - which we're getting using standard idl-based xpcom interfaces - is correctly destroyed when gc happens.

Well, I'm still not seeing why you should run into this problem.


Just to confirm that xpconnect is doing what I expect it to do in a similar test case, I hacked one of the test components (in mozilla/js/src/xpconnect/tests/components) to return a wrapped native object via nsIXPCScriptable::GetProperty.

This patch modifies http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/tests/components/xpctest_calljs.cpp

... and makes use of...
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/tests/components/xpctest_noisy.cpp
...to simulate your wrapped native object returned from a getter.

You can get the small diff at the following URL if you want...

http://www.rattlebrain.com/~jband/hacks/xpctest-Diff.txt

Using xpcshell I ran the following code...

const HasGetter = new Components.Constructor(
    "@mozilla.org/js/xpc/test/CallJS;1",
    "nsIXPCTestCallJS");

function test() {
    var hasGetter = new HasGetter();
    var noisy = hasGetter.noisy;
    noisy.squawk();
}

test();
dump("calling gc()\n");
gc();
dump("done\n");

The output is...

Noisy 1 - Created, 1 total
Noisy 1 - incremented refcount to 1
Noisy 1 - QueryInterface called and succeeding
Noisy 1 - incremented refcount to 2
Noisy 1 - decremented refcount to 1
Noisy 1 - QueryInterface called and succeeding
Noisy 1 - incremented refcount to 2
Noisy 1 - QueryInterface for interface I don't do
Noisy 1 - QueryInterface for interface I don't do
Noisy 1 - incremented refcount to 3
Noisy 1 - QueryInterface called and succeeding
Noisy 1 - incremented refcount to 4
Noisy 1 - QueryInterface for interface I don't do
Noisy 1 - decremented refcount to 3
Noisy 1 - decremented refcount to 2
Noisy 1 - Squawk called
calling gc()
Noisy 1 - decremented refcount to 1
Noisy 1 - decremented refcount to 0
Noisy 1 - Destroyed, 0 total
before 7767, after 7614, break 00000000
done

The line "Noisy 1 - Destroyed, 0 total" is emitted by the native object's destructor. There is no doubt that this is working correctly in my simplified test case. So, I just don't know what is different about your case that would make the native object not get released.



>> We're also unsure about what to pass as scope.
>
> Passing the obj you receive as a param to your GetProperty should be fine.
>
> FWIW... The 'scope' here is used as a scheme to partition wrapper
> spaces.


Ah... that clears a lot up. We were assuming that the scope was used to find a gc root for the return value.

 >> Since we're returning a
 >> value, the reference to that value should be gone when the stack frame
 >> is discarded. How is this accomplished?
[...]
 > Any JS object
 > (and other sorts of JSGC objects) that is reachable by a root object in
 > JS will continue to live.

My fault for being unclear, so I'll restate the question in a completely different way: Am I correct in assuming that the initial root object for the return value should be rval in the current stack frame?

That is correct. During the actual call to your getter the rval is rooted by the engine. And, while that JS stack frame is alive, the value is rooted because it occupies a var slot in an executing function.


After the function returns then the JS engine should not hold it unless it is findable via some other root. But, the code you showed does not show the val being in any way attached to another object.

One odd bit (that *might* be in effect here) is that the engine also roots the most recently created object for each JSContext. Internally, this is called a 'newborn'. The idea being that the object is protected from GC while it is enroute from creation to a safe place. So there is a small chance that you are tripping over this. But just adding a "var foo = new Object();" line anytime after you have created your wrapped native would defeat this. And, really, in the normal course of engine use, objects are created enough that your wrapped native is not likely to stay the most recently created object for the context for long. But, still, you *might* see this in an isolated case. Or, if you stop using the context immediately after wrapper creation.

FWIW, the more normal use of the GetProperty hook would be to also use a resolve hook. This allows the engine to better cache the property.

There is an example of this in xpccomponents.cpp...
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpccomponents.cpp#1616

But, this is not a requirement. And my little test case does not do it and does not leak.

Does WrapNative root the return value? How does it know what to use as root? Or is the return value rooted after we return from GetProperty?

The short answer is that the JSObject of the wrapper is rooted by xpconnect whenever there is an external refcount on the wrapper; i.e. in addition to the internal reference that xpconnect maintains. That single internal reference on the wrapper is released when the JSObject is collected by the engine (and then your native would be released).


The long answer is in the comment I wrote a couple of years ago:
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcwrappednative.cpp#786

Another thought is that if you happen to be using a gc hook in your embedding using JS_SetGCCallback then you had better be chaining the callback calls through to the previously existing hook else xpconnect will have *real* problems.

Anyway, it is still a mystery to me why you are seeing what you are seeing. As long as you are correctly balancing your references to the native object and the wrapper (and doing the right stuff on the JS side as previously discussed) I don't see why this would happen. But, keep asking questions if you have 'em.

John.


> Hope this helps you figure out what is going on.


Yup - we're getting there.

Uli





Reply via email to