Commit 7d66d47
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.
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>1 parent 24debc9 commit 7d66d47
2 files changed
Lines changed: 19 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -171,7 +171,6 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context( | |||
| 171 | 171 | Local<Context> ctx = NewContext(env->isolate(), object_template); | |
| 172 | 172 | ||
| 173 | 173 | if (ctx.IsEmpty()) { | |
| 174 | - env->ThrowError("Could not instantiate context"); | ||
| 175 | 174 | return MaybeLocal<Context>(); | |
| 176 | 175 | } | |
| 177 | 176 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,19 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + const assert = require('assert'); | ||
| 4 | + const vm = require('vm'); | ||
| 5 | + const { Worker } = require('worker_threads'); | ||
| 6 | + | ||
| 7 | + // Do not use isMainThread so that this test itself can be run inside a Worker. | ||
| 8 | + if (!process.env.HAS_STARTED_WORKER) { | ||
| 9 | + process.env.HAS_STARTED_WORKER = 1; | ||
| 10 | + const w = new Worker(__filename); | ||
| 11 | + w.on('online', common.mustCall(() => { | ||
| 12 | + setTimeout(() => w.terminate(), 50); | ||
| 13 | + })); | ||
| 14 | + w.on('error', common.mustNotCall()); | ||
| 15 | + w.on('exit', common.mustCall((code) => assert.strictEqual(code, 1))); | ||
| 16 | + } else { | ||
| 17 | + while (true) | ||
| 18 | + vm.runInNewContext(''); | ||
| 19 | + } | ||
0 commit comments