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

Array.prototype.toString does not have recursion stopper. #809

Closed
hashseed opened this Issue Feb 9, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@hashseed

hashseed commented Feb 9, 2017

Consider following code:

var a = [];
a.push(a);

a.toString()  // returns empty string.

The reason is that we track in a stack whether the array is cyclic. However, the spec doesn't prescribe this recursion check [0]. In fact, I would expect a stack overflow to occur.

Now consider following code:

var counter = 0;

var side_effect_observer = {
  toString() {
    if (counter++ == 0) {
      return array.toString();
    } else {
      return "a";
    }
  }
}

var array = [ side_effect_observer ];

array.toString();  // returns empty string.

Now arguably we do not have a cyclic array, just a recursion that does terminate. I would expect the result to be "a". V8 uses a global stack data structure to track recursion. Presumably this is implemented similarly in JSC and SpiderMonkey.

There is also this esdiscuss thread [1] that discusses this, with the conclusion that this should be spec'ed. Nothing happend since then.

Is this something we want to fix, or do we recoil in fear of breaking the web? If we want to fix, how do we deal with the second example?

[0] https://tc39.github.io/ecma262/#sec-array.prototype.join
[1] https://esdiscuss.org/topic/15-4-4-5-array-prototype-join

@hashseed

This comment has been minimized.

Show comment
Hide comment
@hashseed

hashseed commented Feb 9, 2017

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Feb 9, 2017

Contributor

Dup of #289?

Contributor

anba commented Feb 9, 2017

Dup of #289?

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Feb 9, 2017

Member

Thanks! Closing as a duplicate of #289.

Member

ljharb commented Feb 9, 2017

Thanks! Closing as a duplicate of #289.

@ljharb ljharb closed this Feb 9, 2017

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