Skip to content

Commit

Permalink
vm: do not overwrite error when creating context
Browse files Browse the repository at this point in the history
An empty `Local<>` already indicates that an exception is pending,
so there is no need to throw an exception. In the case of Workers,
this could override a `.terminate()` call.

PR-URL: #26112
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and rvagg committed Feb 28, 2019
1 parent 24debc9 commit 7d66d47
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
1 change: 0 additions & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
Local<Context> ctx = NewContext(env->isolate(), object_template);

if (ctx.IsEmpty()) {
env->ThrowError("Could not instantiate context");
return MaybeLocal<Context>();
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-worker-vm-context-terminate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const vm = require('vm');
const { Worker } = require('worker_threads');

// Do not use isMainThread so that this test itself can be run inside a Worker.
if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = 1;
const w = new Worker(__filename);
w.on('online', common.mustCall(() => {
setTimeout(() => w.terminate(), 50);
}));
w.on('error', common.mustNotCall());
w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1)));
} else {
while (true)
vm.runInNewContext('');
}

0 comments on commit 7d66d47

Please sign in to comment.