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

vm: node crashes if timeout is set and nofile limit is reached #8555

Closed
chatziko opened this issue Sep 15, 2016 · 1 comment
Closed

vm: node crashes if timeout is set and nofile limit is reached #8555

chatziko opened this issue Sep 15, 2016 · 1 comment
Labels
vm Issues and PRs related to the vm subsystem.

Comments

@chatziko
Copy link

  • Version: v6.6.0 and v4.5.0
  • Platform: ubuntu trusty, 3.13.0-86-generic, x86_64
  • Subsystem: vm

If a timeout is given to one of vm's methods, and the nofile limit is reached, the node process crashes. It can be easily reproduced (on both v4.5.0 and v.6.6.0) with this 2-liner:

> require('posix').setrlimit('nofile', { soft: 0 })
> require('vm').runInNewContext('1', undefined, { timeout: 1000 });

/usr/bin/nodejs[5049]: ../src/node_watchdog.cc:16:node::Watchdog::Watchdog(v8::Isolate*, uint64_t): Assertion `(0) == (rc)' failed.
 1: node::Abort() [node]
 2: node::Assert(char const* const (*) [4]) [node]
 3: 0x10ea8d6 [node]
 4: node::ContextifyScript::EvalMachine(node::Environment*, long, bool, bool, v8::FunctionCallbackInfo<v8::Value> const&, v8::TryCatch*) [node]
 5: node::ContextifyScript::RunInContext(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 6: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]
 7: 0x9f3902 [node]
 8: 0x9f3f7e [node]
 9: 0x2c5c8fb092a7
Aborted (core dumped)

It seems that node::Watchdog tries to create a libuv thread to implement the timeout. uv_loop_init fails due to the nofile limit, causing this assertion to fail. The correct behaviour would be to throw some meaningful exception.

@imyller imyller added the vm Issues and PRs related to the vm subsystem. label Sep 16, 2016
@fhinkel
Copy link
Member

fhinkel commented Sep 17, 2016

Thanks for reporting this! I can reproduce the issue on Mac OS with Node 6.6.0 and on Ubuntu with current master 2a2ec9d.

fhinkel added a commit to fhinkel/node that referenced this issue Sep 20, 2016
Add an error message in watchdog if we abort because uv_loop_init fails.

Fixes nodejs#8555
jasnell pushed a commit that referenced this issue Sep 29, 2016
Add an error message in watchdog if we abort because uv_loop_init fails.

PR-URL: #8634
Fixes: #8555
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fishrock123 pushed a commit that referenced this issue Oct 11, 2016
Add an error message in watchdog if we abort because uv_loop_init fails.

PR-URL: #8634
Fixes: #8555
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
Add an error message in watchdog if we abort because uv_loop_init fails.

PR-URL: #8634
Fixes: #8555
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
Add an error message in watchdog if we abort because uv_loop_init fails.

PR-URL: #8634
Fixes: #8555
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
Add an error message in watchdog if we abort because uv_loop_init fails.

PR-URL: #8634
Fixes: #8555
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants