Terrence Cole wrote:
On 08/12/2012 09:23 PM, Nicholas Nethercote wrote:
Hi,
Sharps were removed from SpiderMonkey a while ago; see
http://whereswalden.com/2012/01/25/spidermonkey-no-longer-supports-sharp-variables/
and https://bugzilla.mozilla.org/show_bug.cgi?id=566700.
But there's still some sharps-related code in SpiderMonkey, e.g.
JSContext::sharpObjectMap
I can't speak for the behavioural side of things, but I think
sharpObjectMap can go now. It was not clear that it could go before (I
cleaned up Object::toSource recently) because the comment explaining
what it is doing is not terribly clear about what the possible rooting
scenarios are and the code was (and still largely is) a forest of
confusion and despair.
There's always hope! Igor is not involved and I'm years away from this
code, but if you mean the comment from js_TraceSharpMap, a quick hg
annotate and then cvs blame session shows it came from
revision 3.265
date: 2006/06/15 10:14:42; author: igor.bukanov%gmail.com; state:
Exp; lines: +36 -0
Bug 340129: Bulletproof rooting of objects during sharp graph
construction/usage. r=mrbkap
https://bugzilla.mozilla.org/show_bug.cgi?id=340129 from 2006, based on
crash reports, is worth a read. I believe the patch did signfiicantly
reduce the crash signature's incidence.
We have deep history in SpiderMonkey.
After more thought, I can only see one situation it could be there to
protect us from: a getter prop (running in JS code) which removes two
very specific elements from the object being to-sourced, then GC's at
exactly the wrong moment. If the target of the sharp was on a different
branch recursion and was therefore not held alive by the stack (and not
by tracing, since it was removed), but the referring element was kept
alive because it is on the stack and we are about to trace it (if it
were alive from tracing we would have cleaned the sharp ref), then we
could try to toSource a dead object. At least, I think this is the gist
of the comment in the source: at least its the only feasible situation
that I see that would require the sharpObjectMap in any situation.
The general problem is that toSource's marking happens on a live heap,
subject to mutation (even with JS's single-threaded execution model) via
getters, including ones hiding inside wrapped objects.
Now that toSource is purely recursive, I think just using RootedObject
should be fine.
As long as you root everything and check stack depth, should be.
I'll craft a patch and get the fuzzers on it as soon as
I finish up some more critical work.
Thanks.
/be
and js_{Enter,Leave}SharpObject, which are
used in obj_toSource(). Bug 566700 had some discussion about
toSource(), and how sharps syntax shouldn't be used in it, but I
didn't understand the discussion.
Also, this doesn't look right:
js> let x = {};
js> x.a = x;
({a:{a:{}}})
js>
js> let y = []
js> y[0] = y;
too much recursion
Can anyone clarify if things are as they are supposed to be? Thanks.
Nick
_______________________________________________
dev-tech-js-engine-internals mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
_______________________________________________
dev-tech-js-engine-internals mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
_______________________________________________
dev-tech-js-engine-internals mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
