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

doc: document vm module limitation with timeouts and async code #8037

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 9, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc (vm)

Description of change

Fixes: #3020

@jasnell jasnell added doc Issues and PRs related to the documentations. vm Issues and PRs related to the vm subsystem. labels Aug 9, 2016
@bengl
Copy link
Member

bengl commented Aug 9, 2016

Hmm, is this accurate?

I put the code snippet in a file and ran it (in v6.3.1). Unable to reproduce. It stops due to timeout as expected.

It fails as documented here if used in the REPL though.

@jasnell
Copy link
Member Author

jasnell commented Aug 9, 2016

It fails consistently in the REPL... and, ugh, you're right that it appears to work as expected within a file. Sigh...

@23doors
Copy link

23doors commented Oct 24, 2016

If you skip the part outside of Promise, it will never stop though (as expected).

vm.runInNewContext(
  'Promise.resolve().then(()=>{while(1)console.log("foo", Date.now());});',
  {console:{log(){console.log.apply(console,arguments);}}},
  {timeout:5}
);

There is a known limitation when using Promises with the `vm` methods that
allow for `timeout` option. Specifically: the `timeout` is only effective on
code run *within* the VM context; Promises, and other kinds of asynchronous
operations that execute outside of that scope are not limited by the callback.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... are not limited by the timeout

@targos
Copy link
Member

targos commented Jan 2, 2017

If you skip the part outside of Promise, it will never stop though (as expected).

That's right. I suggest you just change the example to what @23doors wrote.

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Jan 7, 2017
@jasnell jasnell closed this Feb 15, 2017
@ghost
Copy link

ghost commented Mar 17, 2017

It's unfortunate that it wasn't merged, it would have saved me a few hours!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. stalled Issues and PRs that are stalled. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants