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

Backport 6907 to v4.x (console.* not working when running out of stack space) #6957

Closed
wants to merge 4 commits into from

Conversation

addaleax
Copy link
Member

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

vm, module

Description of change

Backport of #6907 to v4.x

@addaleax addaleax added module Issues and PRs related to the module subsystem. vm Issues and PRs related to the vm subsystem. console Issues and PRs related to the console subsystem. v4.x labels May 24, 2016
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 24, 2016
@addaleax
Copy link
Member Author

Trott added 2 commits May 24, 2016 14:51
Correct alignment on variable assignments that span multiple lines in
preparation for lint rule to enforce such alignment.

PR-URL: nodejs#6869
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Enforce alignment/indentation on variable assignments that span multiple
lines.

PR-URL: nodejs#6869
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@jasnell
Copy link
Member

jasnell commented May 25, 2016

Are the two commits from @Trott necessary for this? At first glance those don't appear to be related but I could be missing something.

@jasnell
Copy link
Member

jasnell commented May 25, 2016

@nodejs/lts

@MylesBorins
Copy link
Contributor

@jasnell those two commits are at the head, @addaleax can you rebase?

Make less assumptions about what objects will be available when
vm context creation or error message printing fail because V8
runs out of JS stack space.

Ref: nodejs#6899
Don't cache the exported values of fully uninitialized builtins.
This works by adding an additional `loading` flag that is only
active during initial loading of an internal module and checking
that either the module is fully loaded or is in that state before
using its cached value.

This has the effect that builtins modules which could not be loaded
(e.g. because compilation failed due to missing stack space) can be
loaded at a later point.

Fixes: nodejs#6899
@addaleax
Copy link
Member Author

@thealphanerd sure, rebased. :)

@MylesBorins
Copy link
Contributor

MylesBorins commented Jun 30, 2016

landed in 395f4be...5ba807a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. console Issues and PRs related to the console subsystem. module Issues and PRs related to the module subsystem. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants