Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with callables, realms, and proxies #847

Open
domenic opened this issue Mar 6, 2020 · 6 comments
Open

Issues with callables, realms, and proxies #847

domenic opened this issue Mar 6, 2020 · 6 comments

Comments

@domenic
Copy link
Member

domenic commented Mar 6, 2020

Discovered by @syg in a different context: tc39/ecma262#1597 (comment)

First, an easy problem.

Although at the time of this writing the ECMAScript specification does not reflect this, every ECMAScript object must have an associated Realm. The mechanisms for associating objects with Realms are, for now, underspecified. However, we note that in the case of platform objects, the associated Realm is equal to the object’s relevant Realm, and for non-exotic function objects (i.e. not callable proxies, and not bound functions) the associated Realm is equal to the value of the function object's [[Realm]] internal slot.

This should probably use GetFunctionRealm, which handles callable proxies and bound function objects.

OK, but callable proxies have an additional issue. Given

const { proxy, revoke } = Proxy.revokable(() => {}, {});
revoke();

we have IsCallable(proxy) being true, but GetFunctionRealm(proxy) throws a TypeError. I don't think any uses of "associated Realm" throughout the web spec land are prepared for TypeErrors.

One particular case is in https://heycam.github.io/webidl/#es-invoking-callback-functions and its nearby counterparts. That should probably rethrow the TypeError, or convert it to a rejected Promise, as appropriate.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Mar 6, 2020

Interesting; I hadn't realized Proxy.revocable was a thing.

Gecko currently does not use GetFunctionRealm for the callback case. It uses the actual global of the object, which may not match what GetFunctionRealm returns. Now that SpiderMonkey exposes a GetFunctionRealm API, we could change that.... Does Chrome use GetFunctionRealm there? What about Safari?

Anyway, I agree that if this part is specced to call GetFunctionRealm then it can throw and we need to handle that.

@jorendorff @annevk @bholley @EdgarChen

@syg
Copy link
Contributor

syg commented Mar 6, 2020

That should probably rethrow the TypeError, or convert it to a rejected Promise, as appropriate.

Revoked Proxies ultimately do throw a TypeError when you actually attempt to call them in step 2 here. Naively I expect specs handling GetFunctionRealm's thrown TypeError to be not observably different from whatever happens today, unless the times of getting the Realm and the time of calling the callable differ.

One place where the times do differ is Promises, which raises another question. I'll continue the discussion back at tc39/ecma262#1597.

@syg
Copy link
Contributor

syg commented Mar 6, 2020

See tc39/ecma262#1597 (comment)

@TimothyGu
Copy link
Member

TimothyGu commented Mar 7, 2020

Gecko currently does not use GetFunctionRealm for the callback case. It uses the actual global of the object, which may not match what GetFunctionRealm returns. Now that SpiderMonkey exposes a GetFunctionRealm API, we could change that.... Does Chrome use GetFunctionRealm there?

For the purposes of Web IDL callback functions, Chrome also uses the realm in which the object was created. For the purposes of determining entry realm for promise handlers, Chrome (actually V8) uses GetFunctionRealm with a fallback to the current realm.

@syg
Copy link
Contributor

syg commented Mar 7, 2020

GetFunctionRealm already falls back to the current Realm, though there's a note saying that should only occur for "a non-standard function exotic object", whatever that is.

@TimothyGu
Copy link
Member

(GetFunctionRealm does not fall back to the current Realm for revoked proxies. V8 does however.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants