[shim] add example of a frozen realm

[caridy]

• extend RealmShim
• provide a new method called patch that will be called during the initialization phase
• freeze intrinsics that cannot be polyfilled during the construction phase
• provide a patch method can mutate some intrinsics (e.g.: polyfill Array.prototype.includes)
• call patch during the init phase
• freeze the rest of the intrinsics after calling patch
• evaluate code that validate the frozen realm

[erights]

I have not tested it, but here's an attempt at fixing the deepFreeze from your frozen-realm.js. I tried to make it safe even when some of the objects are proxies, by relying only on the invariants we enforce. I'm not sure I succeeded but I am confident I am close.

This is probably not minimal but it should at least make the issues clear.
attn @caridy @dherman @tvcutsem @brabalan @edgemaster @allenwb @kpreid @FUDCo @dtribble

Not really blocked by #24 (comment) but it would help.

const getProto = Object.getPrototypeOf;
const freeze = Object.freeze;
const gopds = Object.getOwnPropertyDescriptors;
const ownKeys = Reflect.ownKeys;

// Objects that are deeply frozen
const frozenSet = new WeakSet();

/**
 * To deepFreeze an object is to freeze it and all objects
 * transitively reachable from it via transitive reflective
 * property and prototype traversal.
 */
function deepFreeze(node) {
  // Objects that we're attempting to freeze
  const freezingSet = new Set();
  
  // If val is something we should be freezing but aren't yet,
  // add it to freezingSet and return true.
  function addFreezing(val) {
    if (Object(val) !== val) {
      // ignore primitives
      return false;
    }
    const t = typeof val;
    if (t !== 'object' && t !== 'function') {
      // future proof: break until someone figures out what it should do
      throw new TypeError(`unexpected typeof: ${t}`);
    }
    if (frozenSet.has(val) || freezingSet.has(val)) {
      // Ignore if already frozen or freezing
      return false;
    }
    freezingSet.add(val);
    return true;
  }

  function recur(node) {
    if (!addFreezing(node)) { return; }
    if (!freeze(node)) {
      // Yuck. We allowed freeze to indicate failure by returning
      // false rather than throwing, so we need to check for that.
      throw new TypeError(`not frozen ${node}`);
    }
    recur(getProto(node));
    const descs = gopds(node);
    ownKeys(descs).forEach(key => {
      const desc = descs[key];
      if ('value' in desc) {
        recur(desc.value);
      } else {
        // On an accessor property, deepFreeze does not do a [[Get]]
        // or invoke the getter. Rather, it only recursively freezes
        // the getter and setter themselves.
        recur(desc.get);
        recur(desc.set);
      }
    });
  }

  recur(node);
  // This initial call to recur must be in a state where freezingSet
  // is empty, so a non-erroneous return from recur implies that node
  // and everything in freezingSet is successfully deeply frozen.
  for (let n of freezingSet) { frozenSet.add(n); }
  return node;
}

[caridy]

@erights I have added this code, it works :), I have also created an interactive console to play around with the frozen realm, here is the link: https://rawgit.com/caridy/proposal-realms/master/shim/examples/frozen.html

We can discuss the details tomorrow during our weekly meeting.

[erights]

Nice.

@caridy Curious why you're passing frozenSet in as an argument rather than letting deepFreeze encapsulate it?

[kpreid]

@erights You pinged me, you get a code review. I haven't looked at caridy's version.

    if (Object(val) !== val) {
      // ignore primitives

Does the specification guarantee that implementation extensions and future versions of the specification will not break the property this code assumes?

  function recur(node) {

It occurs to me that since you have freezingSet, you could write this using freezingSet as a work queue rather than the stack. This would have the advantage of not possibly overflowing the stack (which, as we know, is hazardous) due to a large (finite) structure.

    if (!addFreezing(node)) { return; }
    if (!freeze(node)) {
      // Yuck. We allowed freeze to indicate failure by returning
      // false rather than throwing, so we need to check for that.
      throw new TypeError(`not frozen ${node}`);
    }

You have defined freeze as Object.freeze, but Object.freeze(O) is specified to return O, not a boolean, so this check does not make sense. Did you perhaps confuse Object.freeze with the internal method it is specified in terms of?

        // On an accessor property, deepFreeze does not do a [[Get]]
        // or invoke the getter. Rather, it only recursively freezes
        // the getter and setter themselves.

This clarification should be promoted to the documentation (comment).

[ljharb]

@kpreid yes, Object(x) === x only for things that are Type "Object", forever.

[caridy]

@erights

@caridy Curious why you're passing frozenSet in as an argument rather than letting deepFreeze encapsulate it?

I'm creating a new frozenSet per instance, while deepFreeze is a function declaration that is not tied to the realm instance in any significant way. But I'm open for suggestions on how to handle this better.

[tvcutsem]

@erights LGTM. As Kevin indicated, Object.freeze can never return false. If node is a proxy, it can indeed make the freeze process fail, but Object.freeze will convert into TypeError.

[erights]

As Kevin indicated, Object.freeze can never return false. If node is a proxy, it can indeed make the freeze process fail, but Object.freeze will convert into TypeError.

Unless the object refusing to be frozen is the browser's bizarre global object, i.e., the one called WindowProxy (which is btw not a Proxy). For this case explicitly, we allow it to report its refusal to be frozen by returning false rather than throwing. This was the least damaging adjustment we could make that would not break the web. Everything else that was proposed would have compromised the invariants themselves.

[erights]

@kpreid

You have defined freeze as Object.freeze, but Object.freeze(O) is specified to return O, not a
boolean, so this check does not make sense. Did you perhaps confuse Object.freeze with the
internal method it is specified in terms of?

No, not a confusion. This code takes advantage of that fact that returning an object means returning something truthy. In that one bizarre case I mentioned (browser's WindowProxy), we allow Object.freeze to report failure by returning false rather than throwing, which its only possible falsey result when applied to an object.

[raulsebastianmihaila]

we allow Object.freeze to report failure by returning false rather than throwing, which its only possible falsey result when applied to an object.

I thought that was for defineProperty, not freeze. tc39/ecma262#672 (And it's not in the spec yet AFAICT.)

[erights]

@raulsebastianmihaila Yes!
I has misremembered that very badly. Thanks!
@kpreid @tvcutsem you are correct.

[jfparadis]

@kpreid, applying your idea of a work queue, we get:

const getProto = Object.getPrototypeOf;
const freeze = Object.freeze;
const gopds = Object.getOwnPropertyDescriptors;
const ownKeys = Reflect.ownKeys;

// Objects that are deeply frozen
const frozenSet = new WeakSet();

  function deepFreeze(node) {
    // Objects that we're attempting to freeze
    const freezingSet = new Set();
  
    // If val is something we should be freezing but aren't yet,
    // add it to freezingSet.
    function enqueueFreezing(val) {
      if (Object(val) !== val) {
        // ignore primitives
        return;
      }
      const t = typeof val;
      if (t !== 'object' && t !== 'function') {
        // future proof: break until someone figures out what it should do
        throw new TypeError(`unexpected typeof: ${t}`);
      }
      if (frozenSet.has(val) || freezingSet.has(val)) {
        // Ignore if already frozen or freezing
        return;
      }
      freezingSet.add(val);
    }
  
    // Process the freezingSet.
    function dequeueFreezing() {
      // New values added before forEach() has finished will be visited.
      freezingSet.forEach(val => {
  
        freeze(val);
        frozenSet.add(val);
  
        enqueueFreezing(getProto(val));
  
        const descs = gopds(val);
        ownKeys(descs).forEach(key => {
          const desc = descs[key];
          if ('value' in desc) {
            enqueueFreezing(desc.value);
          } else {
            // On an accessor property, deepFreeze does not do a [[Get]]
            // or invoke the getter. Rather, it only recursively freezes
            // the getter and setter themselves.
            enqueueFreezing(desc.get);
            enqueueFreezing(desc.set);
          }
        });
      });
    }
  
    enqueueFreezing(node);
    dequeueFreezing();
  
    return node;
  }

If that code is correct, there is a small performance gain as seen on JSPerf

[erights]

Is Set.prototype.forEach specified to enumerate elements added to the set during its enumeration? Deterministically?

[jfparadis]

Yes, that's the specified behavior for Map and Set:

New values added after the call to forEach begins are visited.

It's an incontinent behavior in loops, and that's why I have added the comment.

For example, it's not the case with loops on Array: Array.prototype.every, Array.prototype.map, etc. which are specified to behave the opposite way:

Elements which are appended to the array after the call to every begins will not be visited.

Other loops don't specify the behavior, for example Array.prototype.forEach.

[raulsebastianmihaila]

@jfparadis Array.prototype.forEach doesn't have the note that Array.prototype.map has, but they pretty much have the same algorithm, so that means that the new elements that are appended during forEach are not visited.

[jfparadis]

Thanks. We are Set.prototype.forEach (not Array.prototype.forEach). Do you know if the specs, which do state that new elements are visited, are respected by the algorithm used in that case?

[raulsebastianmihaila]

If Set.prototype.forEach didn't have the clarifying notes, I would have found it ambiguous because I wouldn't have been able to tell if For each e that is an element of entries includes newly added elements.

[caridy]

We have decided that the concept of a frozen realm should not be part of this proposal. Additionally, this issue is related to the Shim implementation, closing since it was moved into https://github.com/Agoric/realms-shim.