From b3ac28c467ab0da367035975d6df3c86f10b0ab0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 14 Feb 2019 23:40:41 +0100 Subject: [PATCH 1/2] vm: do not overwrite error when creating context 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. --- src/node_contextify.cc | 1 - .../test-worker-vm-context-terminate.js | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-worker-vm-context-terminate.js diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 952acfe06f4b85..343342408b1a75 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -171,7 +171,6 @@ MaybeLocal ContextifyContext::CreateV8Context( Local ctx = NewContext(env->isolate(), object_template); if (ctx.IsEmpty()) { - env->ThrowError("Could not instantiate context"); return MaybeLocal(); } diff --git a/test/parallel/test-worker-vm-context-terminate.js b/test/parallel/test-worker-vm-context-terminate.js new file mode 100644 index 00000000000000..779f244dc6739c --- /dev/null +++ b/test/parallel/test-worker-vm-context-terminate.js @@ -0,0 +1,17 @@ +'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); + setTimeout(() => w.terminate(), 50); + w.on('error', common.mustNotCall()); + w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); +} else { + while (true) + vm.runInNewContext(''); +} From 5d74e49b8bc712828442036fc72d31fd34b7cac7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 15 Feb 2019 11:21:56 +0100 Subject: [PATCH 2/2] fixup! vm: do not overwrite error when creating context --- test/parallel/test-worker-vm-context-terminate.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-worker-vm-context-terminate.js b/test/parallel/test-worker-vm-context-terminate.js index 779f244dc6739c..23b58ba4db14d5 100644 --- a/test/parallel/test-worker-vm-context-terminate.js +++ b/test/parallel/test-worker-vm-context-terminate.js @@ -8,7 +8,9 @@ const { Worker } = require('worker_threads'); if (!process.env.HAS_STARTED_WORKER) { process.env.HAS_STARTED_WORKER = 1; const w = new Worker(__filename); - setTimeout(() => w.terminate(), 50); + 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 {