tc39 / proposal-shadowrealm

ECMAScript Proposal, specs, and reference implementation for Realms
https://tc39.es/proposal-shadowrealm/
1.43k stars 67 forks source link

Wrapping or unwrapping already-wrapped functions #343

Closed caitp closed 8 months ago

caitp commented 2 years ago

Suppose you have the following scenario:

let r = new ShadowRealm;
let wrapped1 = r.evaluate("(function(a, b) { return a(b); })");
let wrapped2 = r.evaluate("(function() { return globalThis.specialSauce++; })"

let result = wrapped1(wrapped2, 7);

Invoking wrapped1 results in re-wrapping the already wrapped wrapped2, but did any wrapping really need to happen? Would it make sense to simply pass the unwrapped target function as a parameter?

This difference would be observable, but I don't think it enables an avenue of cross-realm information sharing, and it could provide some heap savings

caitp commented 2 years ago

Also, multiply-wrapped functions can be a pain to query, so it might be nice if we could guarantee that a wrapped function is at most singly wrapped

mhofman commented 2 years ago

Besides the identity observability, which is better served by symbols, what would be the benefit to specifying an unwrapping or limiting the number of wrapping? The current spec doesn't prevent implementations from optimizing the calls of multi-wrapped functions and in effect flatten invocations.

See https://github.com/tc39/proposal-shadowrealm/issues/293 for previous discussion on the topic.

caitp commented 2 years ago

I do believe identity observability of functions created within the same shadow realm is more intuitive and easier to understand, and simpler to work with, for users.

But that isn't really the point, on the runtime side, I'm not a huge fan of allocating a new function wrapper every time you want to pass a function returned from a shadow realm, as an argument to a function within that shadow realm. I'm of the opinion that wrapping should only be done for cross-realm operations, but when passed a wrapper of a function originating in the same shadow realm, adding another layer of indirection doesn't make sense

ljharb commented 2 years ago

Can't the wrapper have properties added in the new realm?

caitp commented 2 years ago

Can't the wrapper have properties added in the new realm?

ShadowRealm provides a function to the Incubator realm (this is wrapped, the incubator realm can't operate on the inner function) Incubator realm passes wrapped function back to ShadowRealm as an argument (This is unwrapped, the ShadowRealm doesn't see anything added to the wrapper object by the IncubatorRealm)

I don't see a way for properties on the wrapper to be exposed to the ShadowRealm, or properties on the original object to be exposed to the incubator realm or different ShadowRealm -- I think, as far as each realm is concerned, they all are operating with distinct objects representing the same underlying function, and there's no real mechanism for sharing information with it --- but as far as one realm is concerned, a reference to that realm's functions always matches the identity of that realm's functions

ljharb commented 2 years ago

ok - so currently, if i have function F in the main realm, and i pass it into the shadow realm, and the shadow realm passes it back as G, and another round trip gives me H. F !== G && F !== H && G !== H.

With your proposal, would G === H?

mhofman commented 2 years ago

Identity stability of function through the boundary is not a design goal of this API, and I believe the champions would be against it.

In particular passing the same function twice from realm A -> B would result in 2 different wrappers, and doing otherwise would require the implementation to keep a WeakMap of callables and their wrapper. Since there is already identity instability in that case, there is no reason to have identity stability in round trips.

caitp commented 2 years ago

If I understood your example:

let r = new ShadowRealm;
let wrap = r.evaluate("(function(f) { return f; })");

let f = function() {}
let g = wrap(f);
let h = wrap(g);

// f == g == h
mhofman commented 2 years ago

My point is that you already have the following:

let f = function() {};
let r = new ShadowRealm;
const g = r.evaluate("(function(f1, f2) { return f1 === f2; })");

assert(g(f, f) === false);
ljharb commented 2 years ago

@caitp and here:

let r = new ShadowRealm;
let wrap = r.evaluate("globalThis.cache ||= new Set(); (function(f) { globalThis.cache.add(f); return f; })");

let f = function() {}
let g = wrap(f);
let h = wrap(g);

r.evaluate('globalThis.cache.size'); // ?

what would be the result?

caitp commented 2 years ago
r.evaluate('globalThis.cache.size'); // ?

My thing here is concerned with the way things are treated which originate from and are returned to a particular realm --- those would preserve their identity, but wrappers would still be created when crossing the realm boundary, so the globalThis cache would have 2 items in it at the end, and you'd really want it to be a WeakMap 👀

caitp commented 2 years ago

My point is that you already have the following:

let f = function() {};
let r = new ShadowRealm;
const g = r.evaluate("(function(f1, f2) { return f1 === f2; })");

assert(g(f, f) === false);

I think this would still be the case with the idea I presented here, this doesn't change

ljharb commented 2 years ago

WeakMaps don't have a size, otherwise i'd have used that :-)

caitp commented 2 years ago

so, it would enable a level of querying of the unwrapped identity of an object that wouldn't be possible currently, I guess there are some security implications. They do seem like the sorts of things which have other solutions though

caridy commented 8 months ago

We have discussed the identity of wrapped functions extensibly in various forms. I will close this assuming that there are no more open questions.