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

Cloning promises between realms #6

Open
domenic opened this issue Jun 13, 2014 · 23 comments
Open

Cloning promises between realms #6

domenic opened this issue Jun 13, 2014 · 23 comments

Comments

@domenic
Copy link

domenic commented Jun 13, 2014

First step in figuring out #5. The use case in question: postMessaging a promise from window A (where it was created) to window B.

Promises are tricky because they not only have internal state to encapsulate (see their internal slots), but also that internal state can be manipulated by their creator in window A.

For maps and sets, their internal state can also be manipulated, but once the clone happens, they are disconnected: window A and window B end up with completely different maps. Is this right for promises? There are a few reasons to think not:

  • Promises represent an async operation; maps represent a mutable data structure. If you clone a map and get a snapshot, you still have a mutable data structure on the other side; its "essence" has been maintained. But if you clone a pending promise and it then stays pending forever (i.e. is snapshotted), then you no longer have something representing that async operation on the other side: its "essence" has been lost.
  • Promises are inherently asynchronous, so it seems much more possible to maintain the link between the original and the clone.

Thus, I think the following would work:

  1. Promise B gets initialized with the same [[PromiseState]] and [[PromiseResult]] as promise A. If [[PromiseState]] is "fulfilled" or "rejected", then there's nothing to do. (If [[PromiseState]] is undefined, then someone tried to clone the promise before it was initialized, and we should throw.)

  2. The implementation internally adds something like:

    promiseA.then(v => resolvePromiseBWith(v), r => rejectPromiseBWith(r))

    As long as the implementation uses the original value of Promise.prototype.then, this would be unobservable in window A, which is good.

  3. The resolvePromiseBWith and rejectPromiseBWith meta-functions would go through the public API in the same way as all of the promise spec currently does. More formally, in step 1 we would save the resolve and reject functions passed to us by the constructor, and resolvePromiseBWith(v) would essentially be something like "post a message to window B to call the resolve function it saved with value v"

  4. If v or r are not structured-clonable, instead the implementation would post a message to window B to call the reject function it saved with an informative error. I guess we'd need to take this into account during step 1 as well.

Still to think about: how to match up constructors across realms? E.g. if someone subclasses Promise to CancelablePromise, how do we ensure that a clone of a cancelable promise from window A becomes a cancelable promise in window B? Should it even be such a cancelable promise? This seems to be a larger issue related to much of the things this repo is generally concerned with. For a first pass, I'd be happy with just reconstituting as normal Promise instances, as long as it's future-compatible. (E.g. in the future subclasses could have a "make clone data" and "initialize with clone data" hook that gets called if they are present?)

@tabatkins
Copy link

You've skipped a step, or some explanation, where the resolve/rejectPromiseBWith() functions to structured cloning on the passed values; without that extra detail, much of this would be simplified by just creating a new promise on the far end resolved to the original promise.

Otherwise, I agree with the overall thrust, that making separate promises completely ruins the promise itself; might as well just make promises error when cloned instead, rather than doing a completely useless and broken clone.

I also agree that starting from a basic Promise (losing any subclass features) is probably fine, with the option to upgrade in the future.

@domenic
Copy link
Author

domenic commented Jun 13, 2014

I kind of covered that in step 4, but only in an indirect way, I agree.

@tabatkins
Copy link

Yeah, if you intended Step 4 to implicitly handle that, then you can indeed just replace Steps 1-3 with "make a new promise on the far end, resolve it with the starting promise" and be done.

@domenic
Copy link
Author

domenic commented Jun 13, 2014

I don't understand how. You can only resolve with objects that are in your realm, and the entire purpose of steps 1-4 is to define how to get a (clone of) a promise for one realm to another.

@stefanpenner
Copy link

Let me know if I understood the cloning stuff correctly. If so, I have some concerns.

cloning promise that has not yet settled:

  • if it settles to a result which is not structured-clonable result we should reject the cloned promise.
  • if it settles to a structured-clonable result, all is well

but to defend against zalgo, cloning settled promises should have the exact same semantics.

I feel like the destination realm having to deal with the originating realm's mistakes is entirely non-actionable and potentially frustrating. Additionally, no feedback is provided to the originating realm.

@stefanpenner
Copy link

Also, is it even possible to structure clone an Error? Last time I tried I am pretty sure, it fell into the category of non cloneable.

If ^^ is correct, this would mean cloning fetch('user') would only work if the request was successful, otherwise you would get a structure clone error rejection, again rather un-actionable.

It may even cause an issue trying to push the structure clone error itself, Although I assume the internals could work around this.

If memories serves me, in a past project we ended up detecting error objects, during the clone phase, and replacing them with something sanitized but containing enough information to still be actionable. This was annoying, but enabled us to provide more actionable rejection reasons.

@tabatkins
Copy link

I don't understand how. You can only resolve with objects that are in your realm, and the entire purpose of steps 1-4 is to define how to get a (clone of) a promise for one realm to another.

Ah, gotcha, didn't realize that the Step 2 methods were the magical "transport values across realms" part.

but to defend against zalgo, cloning settled promises should have the exact same semantics.

That'll happen anyway, because we're just using standard resolution to transport the values across; it doesn't matter whether the promise is settled or not.

Also, is it even possible to structure clone an Error? Last time I tried I am pretty sure, it fell into the category of non cloneable.

If not, this is something fixable, probably along the lines you outline.

@stefanpenner
Copy link

If not, this is something fixable, probably along the lines you outline.

Sounds good.

I suspect one of the reasons is to prevent accidentally leaking information between realms. RE: stacktraces and stuff

That'll happen anyway, because we're just using standard resolution to transport the values across; it doesn't matter whether the promise is settled or not.

👍

@stefanpenner
Copy link

Subclassing and constructor identity gets confusing quick as well.

If each realm's constructors are unique, this will result in costly assimilation when using a foreign thenable.

@domenic
Copy link
Author

domenic commented Jun 13, 2014

If each realm's constructors are unique, this will result in costly assimilation when using a foreign thenable.

No more costly than normal. Steps 1-4 are basically a mini-assimilation algorithm in themselves, albeit one that works only for real promises.

@tabatkins
Copy link

Arbitrary thenables aren't structured-clonable. Real promises are, which means the assimilation has alreayd happened.

@stefanpenner
Copy link

@domenic ah, 1-4 would be doing the assimilation. so CancelablePromise.resolve(foreignCancelable) === foreignCancelable

@domenic
Copy link
Author

domenic commented Jun 13, 2014

There's no way to create or get a reference to foreignCancelable in this outline. Once you clone something, on the other side it becomes a Promise.

@stefanpenner
Copy link

@domenic so it would be this realms constructor correct?

Sorry my variable naming was poor

var payload = /* from message channel or something */
CancelablePromise.resolve(payload.cancelable) === payload.cancelable

@domenic
Copy link
Author

domenic commented Jun 13, 2014

so it would be this realms constructor correct?

Yes, but

CancelablePromise.resolve(payload.cancelable) === payload.cancelable

No, since payload.cancelable is actually a Promise, not a CancelablePromise. Since, again, cloning a promise from A always results in a Promise in B.

I suppose we could explicitly fail to transfer promise subclasses on the A side (i.e. throw when it's attempted), instead of attempt to inherit the Promise behavior.

@slightlyoff
Copy link

I'm not sold that this is a good thing for us to add to the platform. As you note, it's tricky because promises have deep connections to their environments. Why should we make these cloneable instead of providing a "remote promise" protocol?

@annevk
Copy link
Collaborator

annevk commented Jun 13, 2014

@slightlyoff if we cannot solve promises, how are we going to solve streams?

@sicking
Copy link

sicking commented Jun 14, 2014

The general approach here seems correct.

To be clear, the assumption here is that window A and window B might be running in different processes, right? So an implementation couldn't really create a new promise in window B and resolve it to the promise from window A.

So I like the use of the resolvePromiseBWith and rejectPromiseBWith meta-functions which are able to communicate across processes if needed.

There's a few of details I'd like to call out here though.

One is that the algorithms in the initial comment makes Promises always be clonable. Even if they have already been resolved to a unclonable value. Trying to clone a Promise which has been resolved to an unclonable value will result in a cloned promise which is rejected. It will not result in that cloning fails. I think this is fine, but I wanted to call it out.

Another detail is that even though a Promise might be initially resolved with a clonable value, that value might later be modified and become unclonable. So cloning a Promise twice might once result in a resolved Promise and once in a rejected Promise. For example:

p = Promise.resolve({ x: 5 });
someWorker.postMessage(p);
p.then((v) => {
  v.y = function() {};
  someWorker.postMessage(p);
});

Again, I think this is fine. Not very different from the fact that two clones of the same promise can yield two different values if the Promise's value is modified between the two clones.

What's somewhat more unfortunate is that cloning of a Promise's internal value happens asynchronously which makes it less predictable. Consider

res = { x: 5 }
p = Promise.resolve(res);
someWorker.postMessage(p);
res.x = 6

The cloned Promise will resolve to { x: 6 } which could be surprising and a source of bugs. And until the microtask spec has been implemented perfectly, different browsers will end up cloning at different points in time. Everywhere else cloning always happens at very predictable and obvious times as to avoid subtle bugs.

This would be possible to fix by making the cloning algorithm use the internal state of the promise. It seems like it would be possible to then clone the value exactly when the cloning algorithm is invoked, or when the promise is resolved, whichever happens later.

@tabatkins
Copy link

Trying to clone a Promise which has been resolved to an unclonable value will result in a cloned promise which is rejected. It will not result in that cloning fails. I think this is fine, but I wanted to call it out.

Yes, necessary to avoid craziness, because otherwise you have different behavior between "clone a pending promise, resolve it with something uncloneable" and the opposite ordering.

So cloning a Promise twice might once result in a resolved Promise and once in a rejected Promise.

No, promises never change value. .then() returns a new promise. Your example just send the original promise twice, both times with the same cloneable value.

The cloned Promise will resolve to { x: 6 } which could be surprising and a source of bugs.

So will the uncloned promise, though. The only source of non-determinism is if the mutation is made in another task, because then it's harder to tell whether the clone has happened yet or not.

@domenic
Copy link
Author

domenic commented Jun 14, 2014

@slightlyoff

I'm not sold that this is a good thing for us to add to the platform. As you note, it's tricky because promises have deep connections to their environments. Why should we make these cloneable instead of providing a "remote promise" protocol?

This is a good point, worth taking a step back to consider. Here is my perspective:

  • We're using promises as a proving ground for streams, as they have very similar problems. And from what I understand, a high-priority feature of service worker and related APIs is being able to cache a response or request stream, which I assume would be done through structured cloning. So in this sense the work being done here is really indirect to the actual use-case.
  • The proposed solution seems pretty reasonable, without any really surprising edge cases. So it doesn't seem like it hurts much, if anything.
  • On the other hand, in the same way you mention that we could do remote promises instead of clonable ones, we could similarly do some kind of remote streams.
  • All of this circles around the issue, which we haven't gotten to yet, which is that structured cloning something to the disk for serialization is very different from structured cloning things between realms. I started this thread first, to discuss the realms use case, since it seems simpler. But if our ultimate use case is disk-serialization, perhaps doing that separately, and leaving remote promises/streams to solve the cross-realm cases, is better.

@sicking

To be clear, the assumption here is that window A and window B might be running in different processes, right? So an implementation couldn't really create a new promise in window B and resolve it to the promise from window A.

Yes indeed.

So I like the use of the resolvePromiseBWith and rejectPromiseBWith meta-functions which are able to communicate across processes if needed.

Yeah. I like that they are not very magic; you could polyfill them with some cross-realm postMessaging. All the actual important stuff is done through the objects' public interfaces.

@tabatkins

So cloning a Promise twice might once result in a resolved Promise and once in a rejected Promise.

No, promises never change value. .then() returns a new promise. Your example just send the original promise twice, both times with the same cloneable value.

I think @sicking is correct; look at his example. The promise's fulfillment value does not change object identity; the promise is still fulfilled with the same value. But its clonability changes, since it acquired a method, and objects with methods aren't clonable.

This would be possible to fix by making the cloning algorithm use the internal state of the promise. It seems like it would be possible to then clone the value exactly when the cloning algorithm is invoked, or when the promise is resolved, whichever happens later.

Yeah, I think this is what I was trying to do with step 1. Unfortunately everything was a bit informal so I wasn't that clear. It's interesting, though, that in that example, if we did indeed snapshot immediately, then side B would observe { x: 5 }, but side A would never be able to observe anything but { x: 6 }.

@annevk
Copy link
Collaborator

annevk commented Jun 15, 2014

Note that with fetch() we have cross-realm semantics for Request and Response objects due to service workers. There are several ways we could define what happens there, but it would be nice if it ended up being compatible with structured cloning.

@tabatkins
Copy link

I think @sicking is correct; look at his example. The promise's fulfillment value does not change object identity; the promise is still fulfilled with the same value. But its clonability changes, since it acquired a method, and objects with methods aren't clonable.

Ah, I see now.

@dslomov
Copy link
Owner

dslomov commented Jun 23, 2014

(Just back from vacation) Dominic's proposal looks very sensible.
Clearly resolve/rejectPromiseBWith can be implemented using postMessage alone. The cleanest way will be to use message channels I guess:

  1. During cloning, generate a MessageChannel channel and communicate a receiving port to realm B
  2. then on sending side
promiseA.then(x => channel.port1.postMessage(x))

And on receiving side,

port2.onmessage = e => resolve(e.data)

(One issue with using ports is that ports are a DOM thing, not an ECMAScript thing)

So the whole thing is indeed polyfillable. It would be nice to get to the point where structured clone is extensible and this is just a definition on structured cloning for Promise (that would also solve the subtyping issues). I am not entirely happy with pushing more and more special cases into structured clone spec, and TC39 seems to be of the same opinion.

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

No branches or pull requests

7 participants