Skip to content
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

Standardize a RangeError for call stack overflow #319

Closed
wants to merge 1 commit into from

Conversation

jugglinmike
Copy link
Contributor

My goal is to standardize runtime behavior in response to stack overflows. I'm sure I got all sorts of things wrong, but there are two details I'm particularly concerned with:

  1. Is this a reasonable requirement for runtimes, generally? I could imagine
    that some implementations (or system achitectures) would make detecting this
    case difficult, or even impossible. I briefly spoke with @littledan about handling this
    in V8, but I'd appreciate input on anyone with experience in runtime
    memory management.
  2. Is this editorially sound? Should I attempt to expand the operation to
    encapsulate steps for suspending execution context? What about steps for
    pushing the new context onto the stack? Would it be more logical to organize
    this as a simple operation around pushing (e.g. "PushExecutionContext ( ctx
    )")?

Thanks for your consideration!

Commit message:

Introduce a new abstract operation for the the creation of new execution
contexts. Include a new algorithm step in this operation to issue an
abrupt completion in cases where the execution context stack cannot
support additional frames.

The behavior for this case was previously unspecified. Major runtimes
such as SpiderMonkey and V8 already return an abrupt completion
(although they differ in the completion's value). Standardizing this
behavior allows user code to consistently detect stack overflow
exceptions and respond accordingly.

Introduce a new abstract operation for the the creation of new execution
contexts. Include a new algorithm step in this operation to issue an
abrupt completion in cases where the execution context stack cannot
support additional frames.

The behavior for this case was previously unspecified. Major runtimes
such as SpiderMonkey and V8 already return an abrupt completion
(although they differ in the completion's value). Standardizing this
behavior allows user code to consistently detect stack overflow
exceptions and respond accordingly.
@bterlson
Copy link
Member

bterlson commented Feb 4, 2016

AWB suggested EvalError for this. In any case, I think we'll need Needs Consensus for this. There are other issues around resource exhaustion that this might get wrapped up in (eg. the suggestion that we should normatively require implementations to tear down the vat after an OOM).

@bterlson bterlson added the needs consensus This needs committee consensus before it can be eligible to be merged. label Feb 4, 2016
@littledan
Copy link
Member

I'd suggest keeping the error as RangeError, if this is the consensus among all browsers.

It could be hard for any implementation to provide deterministic behavior on out-of-resource issues, though, fundamentally due to things such as operating systems like Linux containing an OOM killer (which kills particular processes when trying to allocate too much memory under some conditions, rather than failing the allocation; this also affects ArrayBuffer allocation failure and makes it very hard to implement throwing the RangeError there correctly FWIW, in step 4 of https://tc39.github.io/ecma262/#sec-arraybuffer-length .)

@getify
Copy link
Contributor

getify commented May 22, 2016

In practice, don't implementations stop before they are actually OOM? Obviously, once memory has been exhausted, doing things would get touchy. But it seems like the requirement would reasonably imply "stop early enough to ensure the following..." and let diff browsers on diff platforms interpret what that means.

@bterlson
Copy link
Member

This proposal did not reach consensus. Many hope we can get away with specifying that resource exhaustion results in tearing down of the agent - implementations are encouraged to experiment. In the meantime, and given how a reliably implementable semantics is likely impossible, we do not want to specify any semantics. A pragmatic approach for test262 is fine, however there is some desire to either not test specwise untestable semantics like resource exhaustion in test262 or to move those tests to another location within the test suite. Discussion on this point will continue on GitHub!

@bterlson bterlson closed this May 23, 2016
@rossberg
Copy link
Member

@getify, no, because in general, OSes do not provide a way to check how much memory is "left". (And it would be subject to races.)

@getify
Copy link
Contributor

getify commented May 23, 2016

@rossberg-chromium all implementations that I'm aware of currently do check and throw a RangeError once the call-stack has gone beyond some limit. How does this currently work? Are they doing fixed call-stack depth limits (I recall IE used to statically limit it to 13 regardless of memory)? In my tests with Chrome, it seems to throw around 15k - 20k depth.

@jugglinmike
Copy link
Contributor Author

This all sounds reasonable to me. I'm happy to have some clarity on the issue
here!

Would there be any value in making some note of this decision in What do you
think about making this (lack of) expectation explicit in the specification? I
believe this could help readers understand the result of this discussion--that
the committee has recognized the issue, but is intentionally avoiding a
standard behavior.

For example, the definition of "Execution Contexts" could be extended with a sentence like:

[...] It is impossible for ECMAScript code to directly access or observe an
execution context.

NOTE If an implementation is at any time unable to allocate a new execution
context in accordance with this specification, the expected behavior is
host-defined.

@rossberg
Copy link
Member

@getify, stack overflow is somewhat different from OOM. In V8 we do stack address checks and throw a RangeError accordingly, but it is not reliable in all situations. How many calls actually fit on the stack depends on many factors, including function size, optimisation levels, parameter count (mis)matches, etc. The stack can also interleave JavaScript stack frames with C++ stack frames.

As an aside, there are implementation techniques that do not use a stack at all. For these, you could never diagnose a stack overflow, you only get OOM.

@rossberg
Copy link
Member

@jugglinmike, even though the spec says "host-defined" in a couple of places, it doesn't actually say what that means. I would rather say "undefined".

@jugglinmike
Copy link
Contributor Author

@rossberg-chromium Good point!

@allenwb
Copy link
Member

allenwb commented May 23, 2016

Traditionally, in language specifications, the phrase "implementation defined" means the behavior is up to the implementation and there is no requirement that implementations document their behavior. This is in contrast to "implementation specified" which means the behavior is up to the implementation and the implementable MUST document its behavior.

I generally used "host defined" to have a comparable meaning.

@jmdyck
Copy link
Collaborator

jmdyck commented May 23, 2016

(In my experience,

  • "implementation-defined" means "must be documented", and
  • "implementation-dependent" means "needn't be documented".

E.g., see http://clhs.lisp.se/Body/26_glo_i.htm#implementation-defined or https://www.w3.org/TR/xpath20/#dt-implementation-defined

Interestingly, the spec uses both these terms without definition.)

@allenwb
Copy link
Member

allenwb commented May 24, 2016

@jmdyck Yes, sorry that's what I meant (I'm a bit sleep deprived at the moment because of how I'm spending this week.

Whenever I used those terms in the spec. those are exactly the meanings I had in mind. However, some spec. test pre/post dates my editorship and sometimes it is a pretty arbitrary choice between the two.

@rossberg
Copy link
Member

@jmdyck, that's my reading, too. And in that sense, "implementation-defined" (alas "host-defined") arguably is a useless concept in the context of JavaScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants