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

Optimisation bug in Node 5.x causes constructing ADTs to randomly fail #47

Closed
robotlolita opened this Issue Oct 11, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@robotlolita
Member

robotlolita commented Oct 11, 2016

data(typeId, patterns) fails in Node 5.x (v8 4.6.85.x) due to a bug in the optimising compiler. This bug has since been fixed (Node 6+ versions are okay), and does not affect older v8 versions (Node 4.x is also okay).

What's happening here?

Folktale uses an extend function to define properties on objects:

function extend(target, ...sources) {
  sources.forEach(source => {
    keys(source).forEach(key => {
      if (key === 'prototype') {
        target[key] = source[key];
      } else {
        defineProperty(target, key, property(source, key));
      }
    });
    symbols(source).forEach(symbol => {
      defineProperty(target, symbol, property(source, symbol));
    });
  });
  return target;
}

This function is then invoked in the defineVariants function to construct Variant objects. Certain optimisations to these two functions cause the expression:

Object.defineProperty(target, key, Object.getOwnPropertyDescriptor(source, key));

To be evaluated as:

Object.defineProperty(target, key, undefined);

Thus causing the native defineProperty function to fail.

What we're doing?

These optimisations can be prevented by using something like eval('true') somewhere in the defineVariants function. They might also not kick in if the code is written in a different way. Because these are unpredictable optimisation bugs, and because Node 5.x isn't a stable Node version anymore, we're dropping support for it — as optimisations in that version are not generally safe.

What platforms are affected?

The entire Node 5.x branch (v8 4.6.85.x) is affected by this issue.

Recommended action for affected users

If you're using Node 5.x, you should upgrade to Node 6.x (latest stable branch), or downgrade to Node 4.x (LTS version)

@robotlolita robotlolita added this to the v2.0.0 milestone Oct 11, 2016

robotlolita added a commit that referenced this issue Oct 11, 2016

@robotlolita robotlolita changed the title from Node v5.12.0 has an optimisation bug in v8 and causes tests to randomly fail to Optimisation bug in Node 5.x causes constructing ADTs to randomly fail Oct 12, 2016

@robotlolita robotlolita removed the a:Tests label Oct 12, 2016

robotlolita added a commit that referenced this issue Oct 12, 2016

(!!) Removes support for Node 5.x due to v8 bugs (#47)
v8 4.6.85.x (Node 5.x) has a bug in the optimiser where an invocation of Object.defineProperty passing the result of Object.getOwnPropertyDescriptor in Core.ADT's `extend` function ends up evaluating the latter to undefined in the absence of other side-effects.

See #47 for more details.

@robotlolita robotlolita removed this from the v2.0.0 milestone Oct 12, 2016

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