[Edbrowse-dev] window root

[Edbrowse-dev] window root

[Karl Dahlke]

> For example the JS_ValueToString function explicitly says
> that you should protect its return value with a GC root or the string is
> at risk of garbage collection at any stage.

Well edbrowse is not multithreaded.
Within the one and only thread, I immediately make that js value
a property in some other object; I immediately link it to the window tree.
That's why I made it a js string in the first place.
Thus it is referenced.
I keep thinking there should be no trouble.

> Theoretically most of our objects should be protected from GC

Theoretically all of our objects should be protected from GC, unless I did something wrong.

> Admittedly some of this rooting may be paranoia,

And that's ok, I guess, unless we are setting up excess reference counts,
and those objects never go away, even when they aren't used any more,
because the reference count never drops to 0,
and edbrowse has memory leaks.
People like me really are in edbrowse for hours at a time.
So I'm not sure we should swing the big hammer and root everything.
At an intellectual level I'd like to know why everything isn't
already rooted to jswin.

Of course you're doing all the work, not me,
and if rooting everything will get us off the ground,
and get rid of seg faults,
then we can watch for and fix memory leaks later.
I do understand that sometimes you just have to move forward.

Karl Dahlke

Re: [Edbrowse-dev] window root

[Adam Thompson]

On Mon, Jan 06, 2014 at 05:19:13AM -0500, Karl Dahlke wrote:
> > For example the JS_ValueToString function explicitly says
> > that you should protect its return value with a GC root or the string is
> > at risk of garbage collection at any stage.
> 
> Well edbrowse is not multithreaded.
> Within the one and only thread, I immediately make that js value
> a property in some other object; I immediately link it to the window tree.
> That's why I made it a js string in the first place.
> Thus it is referenced.
> I keep thinking there should be no trouble.

Maybe not, but the docs talk about the need for rooting in single-threaded
builds as well as thread safe ones.
It's not helped by the fact that the api seems to have changed between the
documents being written (even with the post 185 updates)
and the version of mozjs 24 I grabbed from the website.

It looks like the GC is ran in the same thread as everything else,
but is now called by more functions internally hence the need for more rooting.

> > Admittedly some of this rooting may be paranoia,
> 
> And that's ok, I guess, unless we are setting up excess reference counts,
> and those objects never go away, even when they aren't used any more,
> because the reference count never drops to 0,
> and edbrowse has memory leaks.
> People like me really are in edbrowse for hours at a time.
> So I'm not sure we should swing the big hammer and root everything.
> At an intellectual level I'd like to know why everything isn't
> already rooted to jswin.

Yeah, I know we have to be *very* careful about memory leaks,
particularly as I routinely use edbrowse for extended periods of time.
> 
> Of course you're doing all the work, not me,
> and if rooting everything will get us off the ground,
> and get rid of seg faults,
> then we can watch for and fix memory leaks later.
> I do understand that sometimes you just have to move forward.

Yeah, which is made harder by the fact that, by there own admition,
the mozilla devs regularly change the api in fundimental ways.

I'm just rebuilding a version of smjs 24 with as much debugging info as I can
so I can go through everything with gdb to try and work out exactly what's
still breaking.

Cheers,
Adam.
PS: the only place I can find any mention of SpiderMonkey 26 is in Debian
packages, and the version of the development headers for libmozjs26d (the
experimental debian package for SpiderMonkey 26) has a broken symlink instead of jsapi.h so building against it is currently impossible

[Edbrowse-dev] window root

[Karl Dahlke]

> But don't worry, I'm *pretty sure* we're ok.
> The global object (jwin) should be rooted.
> I'm pretty sure that the JS objects we create and use are reachable from
> the global object, aren't they?

Yes this is what I thought too.
Every object is created under another, linking all the way back to jswin.
And so I still don't understand why there should be an issue.

Karl Dahlke

Re: [Edbrowse-dev] window root

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> Every object is created under another, linking all the way back to jswin.
> And so I still don't understand why there should be an issue.

If I understand correctly, rooting is only necessary if you need an
unreferenced object to persist for some reason.  That's what I'm getting
from their documentation, at least.

-- Chris

Re: [Edbrowse-dev] window root

[Adam Thompson]

On Sun, Jan 05, 2014 at 05:24:44PM -0800, Chris Brannon wrote:
> Karl Dahlke <eklhad@comcast.net> writes:
> 
> > Every object is created under another, linking all the way back to jswin.
> > And so I still don't understand why there should be an issue.
> 
> If I understand correctly, rooting is only necessary if you need an
> unreferenced object to persist for some reason.  That's what I'm getting
> from their documentation, at least.

According to the latest docs I've been reading,
one shouldn't store unrooted pointers on the stack.
I think this has something to do with the way that the js pointers returned
from the various JS_* functions are managed in the smjs memory pool.
For example the JS_ValueToString function explicitly says (at the botom of the
page) that you should protect its return value with a GC root or the string is
at risk of garbage collection at any stage.

I think this is because the SpiderMonkey environment uses a managed memory pool
from whihch it allocates values, strings, objects etc.
Theoretically most of our objects should be protected from GC because they have
references linked to the window, however objects created by converting a jsval
to a JSObject pointer don't and thus can be garbage collected according to the docs.
I think this is slightly different to the way it used to work,
where lots more implicit rooting was performed.

Admittedly some of this rooting may be paranoia, but the GC rooting guide [1] is very explicit about the risks of not rooting c orrectly.

Cheers,
Adam.
[1] https://developer.mozilla.org/en-US/docs/SpiderMonkey/GC_Rooting_Guide