310620 - XPConnect sometimes encourages GC-unsafe setting of properties on wrapped natives

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug has a bit to do with XPConnect consumers but also relates to the API
XPConnect provides to consumers.

From bug 283129 comment 0:

> The problem of how to deal with garbage collection in a system where:
>  1. the GC runtime contains lazily-created wrappers for other data structures
>     not participating in the GC,
>  2. where those data structures are reachable from each other in a way that
>     is reflected through the wrappers, and
>  3. where the wrappers can carry additional information that is not in the
>     wrapped object
> is a difficult one.  XPConnect is such a system.  It creates wrappers around
> native objects that do not participate in the JS GC and which are reachable from
> each other outside of the knowledge of the JS engine (but through getters or
> functions that are accessible to JS users) and it allows these wrappers to be
> decorated with JS properties.  It is worth noting one very important use case
> for our toolkit:  XBL, which uses JS properties on the bound element for
> storing fields, methods, etc.
> 
> Our current solution to it is far from ideal, and takes two forms (I'll call
> them A and B), each of which has a major problem:
[...]
> Form B: For everything else, we let users set properties on wrappers but do
> nothing to protect those wrappers from garbage collection.
> 
> Problem with Form B: This means that we're leaving things wide open for users of
> our APIs to write code that relies on setting JS properties but breaks when GC
> happens at certain points.
> 
> 
> The solution to problem B is simple, assuming it's not too big a break with
> API compatibility:  for objects where we don't preserve JS properties set on
> wrappers, we simply shouldn't allow those properties to be set in the first
> place.  This at least protects users of our toolkit and web authors (for
> example, those dealing with stylesheet objects) from dealing with
> hard-to-reproduce GC-related bugs.

I'm filing this bug to suggest that allowing properties to be set and retrieved
when the success of retrieval of those properties is GC-dependent is a bug.  

First of all, XPConnect would need to know when the presence of properties
causes preservation from GC, something that's currently handled by callbacks in
nsDOMClassInfo, not by XPConnect.  The case where it doesn't is where this bug
needs to be fixed, and some possibilities for fixing it in that case are:
 * XPCWrappedNative::NewOrUsed shouldn't return cached wrappers when they have
properties set on them, but instead should create a new wrapper.  (I'm not sure
how hard this is or what technical obstacles it poses.)
 * Setting properties could fail, either silently or with an exception.

There are currently a whole bunch of nsIXPCScriptable flags that may be related
to this behavior:  (WANT|USE_JSSTUB_FOR)_(ADD|DEL|SET)PROPERTY,
ALLOW_PROP_MODS_DURING_RESOLVE, ALLOW_PROP_MODS_TO_PROTOTYPE.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this something we want to consider for 1.9, or more like 1.10 or later?

Comment 2

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.