Skip to content

Commit

Permalink
worker: handle calling terminate when kHandler is null
Browse files Browse the repository at this point in the history
This PR makes a change to the Worker.terminate() when called if the
kHandler is null. Before this pull request it was returning undefined,
but the API is expecting a promise. With the changes in this PR if
terminate is called a Promise.resolve() is returned, unless a callback
is passed in which case the old behavior stays (returns undefined).

PR-URL: #28370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
elyalvarado authored and targos committed Aug 2, 2019
1 parent 464136f commit 43acce1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,19 @@ class Worker extends EventEmitter {
}

terminate(callback) {
if (this[kHandle] === null) return;

debug(`[${threadId}] terminates Worker with ID ${this.threadId}`);

if (typeof callback === 'function') {
process.emitWarning(
'Passing a callback to worker.terminate() is deprecated. ' +
'It returns a Promise instead.',
'DeprecationWarning', 'DEP0132');
if (this[kHandle] === null) return Promise.resolve();
this.once('exit', (exitCode) => callback(null, exitCode));
}

if (this[kHandle] === null) return Promise.resolve();

this[kHandle].stopThread();

// Do not use events.once() here, because the 'exit' event will always be
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-worker-terminate-null-handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker } = require('worker_threads');

// Test that calling worker.terminate() if kHandler is null should return an
// empty promise that resolves to undefined, even when a callback is passed

const worker = new Worker(`
const { parentPort } = require('worker_threads');
parentPort.postMessage({ hello: 'world' });
`, { eval: true });

process.once('beforeExit', common.mustCall(() => {
console.log('beforeExit');
worker.ref();
}));

worker.on('exit', common.mustCall(() => {
console.log('exit');
worker.terminate().then((res) => assert.strictEqual(res, undefined));
worker.terminate(() => null).then(
(res) => assert.strictEqual(res, undefined)
);
}));

worker.unref();

0 comments on commit 43acce1

Please sign in to comment.