tc39 / proposal-eventual-send

TC39 Eventual Send proposal
44 stars 6 forks source link

This proposal conflates a few of its subcomponents and could be a bit simpler #22

Closed dead-claudia closed 1 year ago

dead-claudia commented 3 years ago

The API currently looks like this:

type PresenceResolver = (presenceHandler) => presence
type DelegateExecutor = (resolve, reject, resolveWithPresence: PresenceResolver) => void
interface PromiseConstructor {
    delegate(executor: DelegateExecutor, unfulfilledHandler): Promise
}

This is rather clumsy to use, and it shows in the README. I feel this comes from conflating three pieces of closely related, but distinct subcomponents:

Other things I noticed about the proposal:

So instead, here's what I propose:

Names are crap, but that's not part of the point.

Consider this code snippet using the current proposal:

const executor = async (resolve, reject, resolveWithPresence) => {
  // Do something that may need a delay to complete.
  const { err, presenceHandler, other } = await determineResolution();
  if (presenceHandler) {
    // presence is a freshly-created Object.create(null) whose handler
    // is presenceHandler.  The targetP below will be resolved to this
    // presence.
    const presence = resolveWithPresence(presenceHandler);
    presence.toString = () => 'My Special Presence';
  } else if (err) {
    // Reject targetP with err.
    reject(err);
  } else {
    // Resolve targetP to other, using other's handler if there is one.
    resolve(other);
  }
};

// Create a delegted promise with initial handler.
// If unfulfilledHandler is not specified (i.e. undefined), it will use
// a builtin queueing handler until targetP is resolved/rejected.
//
// This default queueing handler would cause too much latency if the
// delegated promise actually triggers network traffic.  An actual
// unfulfilledHandler could speculatively send traffic to remote hosts.
const targetP = Promise.delegate(executor, unfulfilledHandler);
E(E(targetP).remoteMethod(someArg, someArg2)).callOnResult(...otherArgs);

With my suggestion here, it'd look more like this:

// Option 1: use a promise constructor
const executor = async (resolve, reject, resolveWithPresence) => {
  // Do something that may need a delay to complete.
  const { err, presenceHandler, other } = await determineResolution();
  if (presenceHandler) {
    // The targetP below will be resolved to a presence whose handler is
    // presenceHandler.
    const presence = {
      toString: () => 'My Special Presence',
    };
    resolveWithPresence(presence, presenceHandler);
  } else if (err) {
    // Reject targetP with err.
    reject(err);
  } else {
    // Resolve targetP to other, using other's handler if there is one.
    resolve(other);
  }
};

// Create a delegted promise with initial handler.
// If unfulfilledHandler is not specified (i.e. undefined), it will use
// a builtin queueing handler until targetP is resolved/rejected.
//
// This default queueing handler would cause too much latency if the
// delegated promise actually triggers network traffic.  An actual
// unfulfilledHandler could speculatively send traffic to remote hosts.
const targetP = Promise.delegate(executor).delegateUnhandled(unfulfilledHandler);;
E(E(targetP).remoteMethod(someArg, someArg2)).callOnResult(...otherArgs);

// Option 2: use the helper method
const executor = async () => {
  // Do something that may need a delay to complete.
  const { err, presenceHandler, other } = await determineResolution();
  if (presenceHandler) {
    // The targetP below will be resolved to a presence whose handler is
    // presenceHandler.
    const presence = {
      toString: () => 'My Special Presence',
    };
    return Promise.resolveWithPresence(presence, presenceHandler);
  } else if (err) {
    // Reject targetP with err.
    throw err;
  } else {
    // Resolve targetP to other, using other's handler if there is one.
    return other;
  }
};

// Create a delegted promise with initial handler.
// If unfulfilledHandler is not specified (i.e. undefined), it will use
// a builtin queueing handler until targetP is resolved/rejected.
//
// This default queueing handler would cause too much latency if the
// delegated promise actually triggers network traffic.  An actual
// unfulfilledHandler could speculatively send traffic to remote hosts.
const targetP = executor().delegateUnhandled(unfulfilledHandler);
E(E(targetP).remoteMethod(someArg, someArg2)).callOnResult(...otherArgs);

In reality, the original code would look like this if we go with more idiomatic error handling:

const executor = async (resolve, reject, resolveWithPresence) => {
  try {
    const { presenceHandler, other } = await determineResolution();
    if (presenceHandler) {
      const presence = resolveWithPresence(presenceHandler);
      presence.toString = () => 'My Special Presence';
    } else {
      resolve(other);
    }
  } catch (err) {
    reject(err);
  }
};

const targetP = Promise.delegate(executor, unfulfilledHandler);
E(E(targetP).remoteMethod(someArg, someArg2)).callOnResult(...otherArgs);

With my proposal, that's where it really shines:

const executor = async () => {
  const { presenceHandler, other } = await determineResolution();
  if (presenceHandler) {
    const presence = {
      toString: () => 'My Special Presence',
    };
    return Promise.resolveWithPresence(presence, presenceHandler);
  } else {
    return other
  }
};

const targetP = executor().delegateUnhandled(unfulfilledHandler);
E(E(targetP).remoteMethod(someArg, someArg2)).callOnResult(...otherArgs);

As you can see, it's a lot less boilerplate and quite a bit easier to use. It also composes a lot more easily and can be introduced a lot more piecewise and easily.

erights commented 3 years ago

@michaelfig , I'll assign this to both of us. Please have a look when you can.

michaelfig commented 3 years ago

With my proposal, that's where it really shines:

Thank you for this proposal. I like its direction. I have just a few suggestions to help it fulfill some constraints influencing the original design.


const executor = async () => {
  const { presenceHandler, other } = await determineResolution();
  if (presenceHandler) {
    const presence = {
      toString: () => 'My Special Presence',
    };
    return Promise.resolveWithPresence(presence, presenceHandler);
  } else {
    return other
  }
};

const targetP = executor().delegateUnhandled(unfulfilledHandler);

If I understand this correctly, executor().delegateUnfulfilled(unfulfilledHandler) creates a new delegated Promise rather than reusing and mutating the executor() result directly? I would expect that since it was a key part of the original design to prevent arbitrary consumers of delegated promises from "reaching into" an existing promise to set its handlers.

That seems fine, and simpler, to me. Even with the existing API, the same effect would be possible by creating a second (wrapper) delegated promise for an existing one, so I'm sure that it doesn't pose a security leak

However, I'd propose to still do this from a static method on the Promise global (instead of a prototype method on every promise). The reason for this is to prevent patterns where the promise itself is trusted to be correct, as opposed to the Promise global. Also, it helps keep the Promise.prototype namespace clean. Something like:

const targetP = Promise.delegateUnfulfilled(executor(), unfulfilledHandler);

BTW, we also felt that adding another argument to the new Promise executor may cause backward compatibility problems. I like the idea of Promise.resolveWithPresence(presence, handler).

Another nice property of making these methods on the Promise global is that they can be easily censored in environments that don't want their functionality.

As you can see, it's a lot less boilerplate and quite a bit easier to use. It also composes a lot more easily and can be introduced a lot more piecewise and easily.

Thanks! I'll leave it up to @erights for other comments.

dead-claudia commented 3 years ago

@michaelfig Thanks for the feedback!

If I understand this correctly, executor().delegateUnfulfilled(unfulfilledHandler) creates a new delegated Promise rather than reusing and mutating the executor() result directly? I would expect that since it was a key part of the original design to prevent arbitrary consumers of delegated promises from "reaching into" an existing promise to set its handlers.

That seems fine, and simpler, to me. Even with the existing API, the same effect would be possible by creating a second (wrapper) delegated promise for an existing one, so I'm sure that it doesn't pose a security leak

Yeah, the intent is it returns a new promise with the desired semantics and just uses connecting logic similar to .then to set up.

And yeah, if there's security concerns, those are definitely easier to address with effectively immutable objects, too.

However, I'd propose to still do this from a static method on the Promise global (instead of a prototype method on every promise). The reason for this is to prevent patterns where the promise itself is trusted to be correct, as opposed to the Promise global. Also, it helps keep the Promise.prototype namespace clean. Something like:

const targetP = Promise.delegateUnfulfilled(executor(), unfulfilledHandler);

That's fine - I'm not really too attached to the syntax. I'm mostly focused on semantics and general abstraction design here.

BTW, we also felt that adding another argument to the new Promise executor may cause backward compatibility problems. I like the idea of Promise.resolveWithPresence(presence, handler).

That's fine, and I almost didn't even propose adding the extra parameter to the new Promise executor anyways. I only proposed it for consistency, and if that's not enough to justify it, I don't see any other reason to add it.

zarutian commented 3 years ago

Want to be clear here, when E(presence).toString() occurs the presenceHandler gets invoked with the apropos trap, yes?

And when presence.toString() occurs then the toString method is invoked on the presence, yes?

michaelfig commented 3 years ago

Want to be clear here, when E(presence).toString() occurs the presenceHandler gets invoked with the apropos trap, yes?

And when presence.toString() occurs then the toString method is invoked on the presence, yes?

That's correct.

dead-claudia commented 2 years ago

Closing in favor of https://github.com/tc39/proposal-eventual-send/issues/31

zarutian commented 2 years ago

I think #31 looses the seperation between immediate invocation and eventual send and the ability to detect which way of calling/invocation was used.

zarutian commented 2 years ago

To elaborate on my earlier comment. The presence object itself can be a Proxy whose handler handles all immediate invocations, gets, and sets, directed at the presence whilist all eventual sends, sendOnlys, gets, sets, and such the presenceHandler handles.

dead-claudia commented 2 years ago

Re-opening while I look into that separation to see if/how that would be implemented.

erights commented 2 years ago

The freshness guarantee for presences was introduced so that the map from presence-to-remote-promise would not be a global communications channel. Last I looked, this simplification failed because it lost that safety property.

This is now a long thread. Can someone point me to a specific explanation of how this problem has been overcome? Or just write a self contained explanation of that specifically? Thanks.

dead-claudia commented 2 years ago

@erights Is there anywhere where all these concerns are drafted up, where I could look at them? I'd like to make sure I see everything that needs covered before I try addressing them, since clearly I don't have a complete picture of the concerns that need addressed with this proposal.

erights commented 2 years ago

I'll try to explain my concern in a self-contained manner here. I'm going to use a single compartment scenario because it is simpler, but the same concern can also be shown between compartments, which might seem more natural.

Under Hardened JS (aka SES), let's say that two subgraphs, Alice and Bob, are both in the same compartment, sharing only the same powerless primordials and the same compartment global object, which is frozen and does not lead to any mutable state. Alice and Bob should then be unable to communicate, even though they are in the same compartment. Let's say that one of the objects in that shared global is an empty object that could be a presence, but has not yet been made into a presence.

lockdown(); // everything further happens within Hardened JS
const empty = harden({});
const c = new Compartment({ empty }); // alice and bob share access to this object
harden(c.global);
const alice = c.evaluate(aliceSrc); // note alice can contain isolated mutable state
const bob = c.evaluate(bobSrc); // note bob can contain isolated mutable state
alice();
try {
  Math.random() > 0.5 && bob(); // Note that the `random` power is unavailable within the compartment
} catch {}
console.log(alice()); // alice tries to return a boolean saying whether bob was called

Assume that aliceSrc and bobSrc are strings co-authored to try to communicate, so that alice and bob are co-conspirators. Assume that the call bob(), if it happens, does eventually terminate without exhausting memory. It should be impossible for alice() to sense whether bob() was actually called. IOW, the code setup above should be adequate to guarantee that they are isolated, no matter what the code aliceSrc or bobSrc is as long as it satisfies these assumptions (primarily that bob() terminates without exhausting memory).

alice.js

() => HandledPromise.resolve(empty) === HandlePromise.resolve(empty);
// If `empty` is just a normal empty object, this `===` will be false, because
// the two `resolve`s produce two separate promises.
// If `empty` is a registered presence, then `===` will be true, because
// the two `resolve`s return the same handled promise.

bob.js

() => ... // make `empty` into a presence
erights commented 2 years ago

If the operation that makes something into a presence is always only making a fresh object into a presence, then that object begins its observable life as a presence. Nothing ever makes an observable transition from non-presence to presence. This prevents Alice and Bob from using it to communicate.

zarutian commented 2 years ago

An example of an presence that only allows eventual send invocations on it:

const makeEventualOnlyPresence = (oid, commlink) => {
  const unfulfilledHandler = nil; // promise-pipeling handling code goes here
  const eventualOnlyError = new Error("only eventual sends, gets, sets, and such supported");
  const immediatesHandler = {
     apply: (target, thisArg, args) => { throw eventualOnlyError; },
     construct: (target, args) => { throw eventualOnlyError; },
     defineProperty: (target, key, descriptor) => { throw eventualOnlyError; },
     deleteProperty: (target, key) => { throw eventualOnlyError; },
     get: (target, key, receiver) => { throw eventualOnlyError; },
     getOwnPropertyDescriptor: (target, key) => { throw eventualOnlyError; },
     // ... you get the idea
  };
  const targetP = Promise.delegatede((resolve, reject, rWp) =>  {
    const eventualsHandler = {
      eventualGet: (p, prop, opts) => commlink.eg(oid, prop),
      eventualSet: (p, prop, val, opts) => { commlink.eset(oid, prop, val); return val; }, // will probably omitt this one in real impl.
      eventualApply: (p, args, opts) => commlink.ea(oid, args),
      eventualSend: (p, prop, args, opts) => commlink.es(oid, prop, args),
      thenUsedTrap: (p, wantsValue) => { commlink.tut(oid); return undefined; }, // part of future idea of detecting if invoker is interested in the resolution/result for its value, that it resolved, or only for promise-pipelining purposes.
      thereTrap: (p, behaviour) => commlink.tt(oid, behaviour), // future support for E.there(p, behaviour) which I wont go into here.
    };
    const presence = new Proxy(Object.create(undefined), immediatesHandler);
    rWp(presence, eventualsHandler);
    return undefined;
  }, unfulfilledHandler);
  return targetP;
};

(PS. notice that no async/await syntax was used.)

dead-claudia commented 1 year ago

Closing this in favor of centralizing all the discussion in #31.