InitScopeForObject should never make an empty shape (it needs a new name, too)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
P2
normal
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

7 years ago
This non-native (null proto) or class-differs-between-proto-and-new-instance hazard makes for shape explosion (see bug 592001, bug 570435, bug 554626, and others I'm forgetting).

We want either (a) proto and instance to have same class; (b) singleton per runtime, thread-data, or compartment (runtime now, thread-data or compartment soon) shared among all instances.

I just noticed that the righteous fix for bug 438633, to give JSScript structs created for API callers of JS_Compile*Script* GC protection by giving each script an object wrapping it, walked into this InitScopeForObject hazard, because of course the class differs. So we end up creating an empty shape per script compiled!

I will try to fix this. I may need help with XBL.

/be
(Assignee)

Comment 1

7 years ago
(b) is a well-known singleton, that is, a dedicated C++ member of the appropriate VM data structure (JSRuntime, currently).

I noticed that JSC has memoized and well-typed structure pointers in its global object data structure. Lazily initialized, IIRC.

/be

Comment 2

7 years ago
(In reply to comment #0)
> We want either (a) proto and instance to have same class; (b) singleton per
> runtime, thread-data, or compartment (runtime now, thread-data or compartment
> soon) shared among all instances.

singleton needs some form of garbage collection as FF creates classes on the fly. But I guess this is not hard as GC would just need to enumerate singleton hash and remove unreachable shape instances.

Also the runtime version needs some locking. So why not start from a thread-based hash?
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> (In reply to comment #0)
> > We want either (a) proto and instance to have same class; (b) singleton per
> > runtime, thread-data, or compartment (runtime now, thread-data or compartment
> > soon) shared among all instances.
> 
> singleton needs some form of garbage collection as FF creates classes on the
> fly. But I guess this is not hard as GC would just need to enumerate singleton
> hash and remove unreachable shape instances.

It's even easier if the classes that XBL and XPConnect synthesizes are "empty shape compatible", which I believe they are. More in a bit.

> Also the runtime version needs some locking. So why not start from a
> thread-based hash?

Sure, I was starting from where we are today (rt->emptyArgumentsShape, etc.). Probably gonna move to compartments for isolation (avoid risking "JS capability leaks").

/be
(Assignee)

Updated

7 years ago
Target Milestone: mozilla2.0b7 → mozilla2.0b8
(Assignee)

Updated

6 years ago
Target Milestone: mozilla2.0b8 → ---
(Assignee)

Comment 4

5 years ago
I believe this is totally fixed by bhackett's work. Brian, let me know if not. Thanks,

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.