1108895 - FindObjectForHasInstance should have interrupt checks

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Try to put this in your web console:

     var o = {};
     var p = new Proxy(o, { get: function(o, p) { return o[p]; } });
     o.__proto__ = p;

and watch the process hang.

The reason for that is that we land in FindObjectForHasInstance with an obj for which JS_GetPrototypeOf(obj) == obj.  So we go into an infinite loop.

Though actually, this in a web page also hangs:

   var o = {};
   var p = new Proxy(o, { get: function(o, p) { return o[p]; } });
   o.__proto__ = p;
   alert(o instanceof Object);

and I expect so do our binding instanceof hooks.  So we probably need to fix them all... The question is how to do an interrupt check from outside the JS engine.

Comment 1

[Jason Orendorff [:jorendorff]]

Expose a non-inlined version of js::CheckForInterrupt, defined in jscntxt.h.

JS::CheckForInterrupt would be an OK name.

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK.  So here's another question.  Should all consumers that do a proto walk be responsible for doing the interrupt check, or should it just happen in getProto() or equivalent?  I guess the latter is inlined and supposed to be fast, but it'd prevent the whack-a-mole we need otherwise.

Comment 3

[Boris Zbarsky [:bzbarsky]]

On the other hand, it's not clear that all getProto() callers can deal with reentering script.  :(

Comment 4

[Bobby Holley (:bholley)]

I think that basically all of these callers want to bail in the case that the thing is a proxy. Can we just switch the C++ API to do that?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Which these callers?

I should note that if I do this:

     var o = {};
     var p = new Proxy(o, { get: function(o, p) { return o[p]; } });
     o.__proto__ = p;
     alert(o.foo);

I get an infinite recursion error.  But that may be an artefact of how gets are implemented such that they use up stack, instead of the proto walks that iterate in constant stack space.  And _that_ may be an implementation detail subject to change.  Adn I don't think we want to immediately throw on any get with a proxy on the proto chain, if that's what you meant.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #5)
> Which these callers?

Well, FindObjectForHasInstance, certainly. I don't have a good sense of what these bindings methods want to do when they encounter a scripted proxy:

http://mxr.mozilla.org/mozilla-central/search?string=js%3A%3AGetObjectProto

Comment 7

[Boris Zbarsky [:bzbarsky]]

You should also look at http://mxr.mozilla.org/mozilla-central/search?string=JS_GetPrototype plus whatever engine-internal stuff (e.g. the "o instanceof Object" case).

Comment 8

[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.