Maximum call stack size exceeded #13

peteruithoven · 2015-12-04T14:03:48Z

When I simply import es6-symbol into my code I get a Maximum call stack size exceeded error.
I'm not sure it's caused by es6-symbol. It could also be caused by the dependency es5-ext, but I can't test this separately because these two packages depend on each other.
I'm using es6-symbol v3.0.1 and Chrome 47.0.2526.73 (64-bit) (automatically updated).
I'm using SystemJS as module loader and JSPM as package manager.

Full error:

Uncaught RangeError: Maximum call stack size exceededmodule.exports @ validate-symbol.js:6
(anonymous function) @ polyfill.js:92
module.exports @ validate-symbol.js:6
(anonymous function) @ polyfill.js:92
module.exports @ validate-symbol.js:6
(anonymous function) @ polyfill.js:92
module.exports @ validate-symbol.js:6
(anonymous function) @ polyfill.js:92
module.exports @ validate-symbol.js:6
(anonymous function) @ polyfill.js:92

Reproduce:

npm install -g jspm
mkdir test
cd test
jspm init -y
jspm install import es6-symbol
Add index.html page:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title></title>
    <script src="jspm_packages/system.js" charset="utf-8"></script>
    <script src="config.js" charset="utf-8"></script>
    <script type="text/javascript">
      System.import('index.js');
    </script>
  </head>
  <body>

  </body>
</html>

Add javascript

import 'es6-symbol';

Start webserver (like browsify)
Visit in Chrome.

The text was updated successfully, but these errors were encountered:

peteruithoven · 2015-12-04T15:14:11Z

I've never worked with Object.defineProperty so I'm having a hard time getting through this code. But I did notice that disabling one of the following two parts in Polyfill.js prevents the issue:
On line 73:

toPrimitive: d('', (NativeSymbol && NativeSymbol.toPrimitive) || SymbolPolyfill('toPrimitive')),

On line 74:

toStringTag: d('', (NativeSymbol && NativeSymbol.toStringTag) || SymbolPolyfill('toStringTag')),

On line 92 - 94:

defineProperty(SymbolPolyfill.prototype, SymbolPolyfill.toPrimitive, d('', function() {
  return validateSymbol(this);
}));

peteruithoven · 2015-12-04T16:28:28Z

I'm having this problem with v3.0.0 and v3.0.1 but not with v2.0.1.
Working from v2.0.1 (using github's compare), I noticed that commit f37c066 (Do not shadow eventual global Symbol var) didn't break anything, but when applying the changes from commit d8e08fd (Reuse native symbols if available) I get the Maximum call stack size exceeded error.

peteruithoven · 2015-12-04T16:44:25Z

It's these changes:

toPrimitive: d('', SymbolPolyfill('toPrimitive')),
toStringTag: d('', SymbolPolyfill('toStringTag')),

To:

toPrimitive: d('', (NativeSymbol && NativeSymbol.toPrimitive) || SymbolPolyfill('toPrimitive')),
toStringTag: d('', (NativeSymbol && NativeSymbol.toStringTag) || SymbolPolyfill('toStringTag')),

Reverting one of these changes fixes the issue.

peteruithoven · 2015-12-07T10:13:43Z

This issue doesn't occur when using Webpack (instead of SystemJS) Even when using Babel 5.8.34 (instead of 6).
It still occurs when creating a self-executing bundle with JSPM.

medikoo · 2015-12-07T13:39:56Z

It looks as some definitive issue with SystemJS (even line number 92 doesn't match any line in polyfill.js file) so let's continue discussion over there at SystemJS bug tracker

peteruithoven · 2015-12-07T13:56:07Z

(It's at line 92 of the transpiled version, the sourcemap doesn't always work when there are errors)

peteruithoven · 2015-12-07T13:57:34Z

A temporary workaround when working with SystemJS is using only 2.0.1:
jspm resolve --only npm:es6-symbol@2.0.1

guybedford · 2015-12-11T15:30:18Z

While this does only seem to happen in SystemJS, the underlying cause seems to be a loop in the code logic of this repo itself.

Running through with the debugger, it shows that:

The error thrown by validate-symbol.js is:

module.exports = function(value) {
  if (!isSymbol(value))
    throw new TypeError(value + " is not a symbol");
  return value;
};

Where value is a symbol object looking like:

value = Object {__description__: "toStringTag", __name__: "@@toStringTag"}

This then cycles between that error and this single line of code:

module.exports = function (x) {
    return (x && ((typeof x === 'symbol') || (x['@@toStringTag'] === 'Symbol'))) || false;
};

Which seems to in turn trigger the error again.

@medikoo do you have any idea at all why such a loop might arise, and how SystemJS could have contributed to getting the logic into such a loop. It would be much easier to track down knowing what specific SystemJS environment changes might have created this in the first place.

medikoo · 2015-12-11T16:26:55Z

@guybedford If I understand right, you mean that following code:

return (x && ((typeof x === 'symbol') || (x['@@toStringTag'] === 'Symbol'))) || false;

Triggers invocation of function exported by validate-symbol.js?

To be honest I have no idea, how it could happen, the only thing that opens door for some hidden code being invoked is here: x['@@toStringTag'] where there might be a getter defined (and that you should be able to see with inspector).

In scope of es6-symbol, there's no such getter set, there's only a setter defined in one place.

Do SystemJS tries to change native Object.defineProperties or similar functions? Maybe issue lies because some low level functions are broken (?)

guybedford · 2015-12-11T16:38:43Z

@medikoo thanks for the quick response. Yes I guess it is that or an implicit toString causing the circularity?

x itself is just:

__description__: "toStringTag"
__name__: "@@toStringTag"
__proto__: Symbol
constructor: Symbol(description)
toString: ()
Symbol(Symbol.toPrimitive): ()
__proto__: Object

I don't see any getter on Symbol @@toStringTag, only a setter. There is a getter for __proto__ though on Symbol.

Looking at the stack again, the first invocation seems to come from the last line of polyfill.js itself:

defineProperty(HiddenSymbol.prototype, SymbolPolyfill.toStringTag, d('c', SymbolPolyfill.prototype[SymbolPolyfill.toStringTag]));

guybedford · 2015-12-11T16:42:29Z

Could the line throw new TypeError(value + " is not a symbol"); itself be throwing on the string operation if value.toString() triggers a getter?

medikoo · 2015-12-11T16:46:01Z

@guybedford can you prepare some smallest possible (but not minified) test case, that will include two ready files: index.html and index.js, so I can just direclty run it in Chrome, and see it?

guybedford · 2015-12-11T16:48:15Z

@medikoo thanks so much for offering to look into this further. There is already one at https://github.com/peteruithoven/systemjs-es6-symbol-issue, in the systemjs folder where the index.html file can be opened in a local web server. When the chrome://flags for Experimental JS are disabled it gives the error only.

medikoo · 2015-12-11T17:03:41Z

@guybedford instructions over there doesn't seem complete. I have no idea what's behind Start webserver (how?), and Visit page (what url?)

guybedford · 2015-12-11T17:09:41Z

@medikoo eg via cd systemjs && http-server, then visiting index.html through the server.

I've been thinking more about this, and I'm wondering if it could be due to SystemJS doing something like the following internally:

var es6SymbolModule = { default: require('es6-symbol') };
Object.keys(es6SymbolModule).forEach(function(key) {
  es6SymbolModule[key] = es6SymbolModule.default.key;
});

While this should normally be caught and ignored if it fails, it seems the circular throwing behaviour when trying to access Object.getPropertyDescriptor(require('es6-symbol'), '@@toStringTag') or similar may be the culprit.

medikoo · 2015-12-11T17:14:37Z

There's definitely something done that causes this. Still Object.keys, get only enumerable properties, and all constructor properties on Symbol are non enumerable.

I'm afraid that without a working test case I'm not able to help you (and I'm sorry, but I don't have time to learn how your utilities work just for sake of running simple test).

peteruithoven · 2015-12-12T00:14:04Z

I tried to clarify the readme of my example: https://github.com/peteruithoven/systemjs-es6-symbol-issue
Can't imagine it's far from your regular workflow, so there isn't really learning specific utilities involved.

guybedford · 2015-12-12T08:27:58Z

I managed to look into this further and the issue is simply that polyfill.js fails to run in Symbol environments if required. That is - require('es6-symbol/polyfill.js') fails in browsers with native Symbol support giving this error (which gets the issue to replicate in webpack too).

The reason this happens in SystemJS is due to our static analysis adding all requires as dependencies so that the CommonJS conditional branching breaks down.

It might be nice to give a better error in polyfill.js than this recursive one, but apart from that this is SystemJS-specific certainly.

medikoo · 2015-12-12T11:28:35Z

@peteruithoven good test case shouldn't require from me install of external utilities or browsing documentation on how to achieve certain things. What I need is plain simple index.html and index.js, that exposes the issue, such things are really easy to prepare with workflow I use, and I believe it's same case on your side.

That is - require('es6-symbol/polyfill.js') fails in browsers with native Symbol

@guybedford node.js v4 and v5 provide native Symbol and tests for polyfill.js run without issues in that environment, so it can't be that case.

guybedford · 2015-12-12T11:44:58Z

@medikoo a very simple replication would be browserify node_modules/es6-symbol/polyfill.js and run that in Chrome with experimental JavaScript flags disabled. It runs fine in Node though.

medikoo · 2015-12-12T12:38:44Z

Ok I got it.

Cause of that issue is uneven state of implementation in Chrome, which at current stage implements native toPrimitive symbol, but doesn't implement toStringTag symbol, in such case indeed script crashes here

es6-symbol/polyfill.js

Line 87 in 54d1436

d('c', SymbolPolyfill.prototype[SymbolPolyfill.toStringTag]));

Both Node.js v4 and v5 doesn't yet use version of V8 that implements toPrimitive, therefore there's no issue there.

Why it crashes:

At line SymbolPolyfill.prototype[SymbolPolyfill.toStringTag], engine tries to convert SymbolPolyfill.toStringTag mock symbol object into primitive, and does that by calling SymbolPolyfill.toStringTag[Symbol.toPrimitive] method (as it now supports Symbol.toPrimitive)

[Symbol.toPrimitive] method for this mock symbol object , was defined line above which turns to be an alias for one defined on SymbolPolyfill.prototype

This call tries to validate given symbol object, and validation works by either recognizing native symbol or recognizing mock by it's @@toStringTag property. As we're in middle of defintion of toStringTag, it's not there yet, so check for isSymbol returns false.
That invokes again toPrimitive convertion at value + ".." so we get into infite loop.

Fix would be to reorder definitions in polyfill, so definition of toPrimitive is made as last one.
It'll be published shortly

medikoo · 2015-12-12T12:46:16Z

Published as v3.0.2

guybedford · 2015-12-12T16:35:47Z

👍 thanks for the quick response here.

peteruithoven · 2015-12-12T18:41:09Z

Thanks for figuring this out guys!
I'll work on my test cases.

peteruithoven mentioned this issue Dec 7, 2015

SystemJS issue with es6-symbol? systemjs/systemjs#955

Closed

peteruithoven added a commit to Doodle3D/es6-symbol that referenced this issue Dec 7, 2015

Temp Fix for medikoo#13

f2b67e0

medikoo closed this as completed Dec 7, 2015

medikoo reopened this Dec 12, 2015

medikoo closed this as completed in ae2cd70 Dec 12, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Maximum call stack size exceeded #13

Maximum call stack size exceeded #13

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 7, 2015

medikoo commented Dec 7, 2015

peteruithoven commented Dec 7, 2015

peteruithoven commented Dec 7, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

peteruithoven commented Dec 12, 2015

guybedford commented Dec 12, 2015

medikoo commented Dec 12, 2015

guybedford commented Dec 12, 2015

medikoo commented Dec 12, 2015

medikoo commented Dec 12, 2015

guybedford commented Dec 12, 2015

peteruithoven commented Dec 12, 2015

Maximum call stack size exceeded #13

Maximum call stack size exceeded #13

Comments

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 4, 2015

peteruithoven commented Dec 7, 2015

medikoo commented Dec 7, 2015

peteruithoven commented Dec 7, 2015

peteruithoven commented Dec 7, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

guybedford commented Dec 11, 2015

medikoo commented Dec 11, 2015

peteruithoven commented Dec 12, 2015

guybedford commented Dec 12, 2015

medikoo commented Dec 12, 2015

guybedford commented Dec 12, 2015

medikoo commented Dec 12, 2015

medikoo commented Dec 12, 2015

guybedford commented Dec 12, 2015

peteruithoven commented Dec 12, 2015