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

Renderer process crashing when executing setTimeout in code exported by scripts in vm.runInNewContext() #15332

Closed
MMhunter opened this issue Oct 23, 2018 · 5 comments

Comments

@MMhunter
Copy link

MMhunter commented Oct 23, 2018

  • Output of node_modules/.bin/electron --version: both 3.0.5 and 2.0.8
  • Operating System (Platform and Version): OS X
  • Output of node_modules/.bin/electron --version on last known working Electron version (if applicable):

Expected Behavior
setTimeout should be called and process should not crash.

Actual behavior
renderer process crashes.

To Reproduce
Running the following code in renderer process:

const { Script } = require('vm');
const sandbox = {exports: {}, setTimeout, console};
new Script(`
    exports.test = () => { 
        setTimeout(()=>console.log('test')) 
    }
`, {})
.runInNewContext(sandbox);

sandbox.exports.test();

Additional Information

  • The above code works in main process and node environment.
  • If setTimeout is called just in the script , process will not crash, but the script in setTimeout will not be called.
// This is ok
const { Script } = require('vm');
const sandbox = {exports: {}, setTimeout, console};
new Script(`
    setTimeout(()=>console.log('test'))
`, {})
.runInNewContext(sandbox);
@ckerr
Copy link
Member

ckerr commented Oct 24, 2018

Happens on Linux in 2-0-x and 3-0-x as well, and still happening in master.

Not sure if this is related or not, testing with the 'this is ok' code sample above hits this check:

[7016:1024/164337.454170:FATAL:v8_binding_for_core.cc(637)] Check failed: window. 
#0 0x7f69bec3110d base::debug::StackTrace::StackTrace()
#1 0x7f69be914dec base::debug::StackTrace::StackTrace()
#2 0x7f69be943ffa logging::LogMessage::~LogMessage()
#3 0x7f69b11bca0e blink::EnteredDOMWindow()
#4 0x7f69b11bc845 blink::ScheduledAction::Create()
#5 0x7f69b1c06122 blink::DOMWindowTimers::setTimeout()
#6 0x7f69b2d2ec84 blink::DOMWindowV8Internal::setTimeout1Method()
#7 0x7f69b2cf1bba blink::DOMWindowV8Internal::setTimeoutMethod()
#8 0x7f69b2cf18aa blink::V8Window::setTimeoutMethodCallback()
#9 0x7f69b4929b8c v8::internal::FunctionCallbackArguments::Call()
#10 0x7f69b48cc1ff v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#11 0x7f69b48ca458 v8::internal::Builtin_Impl_HandleApiCall()
#12 0x7f69b48c9eb1 v8::internal::Builtin_HandleApiCall()
#13 0x7f69b5671275 <unknown>

@blizzardzheng
Copy link

still happening in master too

@dsanders11
Copy link
Member

@MMhunter, @blizzardzheng, this is because the example code is using the web version of setTimeout, inside a context with no widow.

To work the code would need to explicitly use the Node version of setTimeout, but that's currently broken in the renderer process, until #18938 is fixed.

Once that's fixed, the following code should work. It currently doesn't crash, but the timeout never fires.

const { Script } = require('vm');
const timers = require('timers');
const sandbox = {exports: {}, timers, console};
new Script(`
    exports.test = () => { 
        timers.setTimeout(()=>console.log('test')) 
    }
`, {})
.runInNewContext(sandbox);

sandbox.exports.test();

@sofianguy
Copy link
Contributor

Thank you for taking the time to report this issue and helping to make Electron better.

The version of Electron you reported this on has been superseded by newer releases.

If you're still experiencing this issue in Electron 6.x.y or later, please add a comment specifying the version you're testing with and any other new information that a maintainer trying to reproduce the issue should know.

I'm setting the blocked/need-info label for the above reasons. This issue will be closed 7 days from now if there is no response.

Thanks in advance! Your help is appreciated.

@sofianguy sofianguy added the blocked/need-info ❌ Cannot proceed without more information label Feb 14, 2020
@electron-triage
Copy link

Thank you for your issue!

We haven't gotten a response to our questions in our comment above. With only the information that is currently in the issue, we don't have enough information to take action. I'm going to close this but don't hesitate to reach out if you have or find the answers we need, we'll be happy to reopen the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
3.0.x / 3.1.x
Unsorted Issues
Development

No branches or pull requests

7 participants