worker: set trackUnmanagedFds to true by default

This prevents accidental resource leaks when terminating or exiting
Worker that own FDs opened through `fs.open()`.

Refs: https://github.com/nodejs/node/pull/34303

PR-URL: https://github.com/nodejs/node/pull/34394
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
Anna Henningsen 2020-07-16 14:38:21 +02:00 committed by James M Snell
parent f4f191bbc2
commit 7603c7e50c
3 changed files with 19 additions and 3 deletions

View File

@ -621,6 +621,9 @@ if (isMainThread) {
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34394
description: The `trackUnmanagedFds` option was set to `true` by default.
- version:
- v14.6.0
pr-url: https://github.com/nodejs/node/pull/34303
@ -689,7 +692,7 @@ changes:
[`fs.close()`][], and close them when the Worker exits, similar to other
resources like network sockets or file descriptors managed through
the [`FileHandle`][] API. This option is automatically inherited by all
nested `Worker`s. **Default**: `false`.
nested `Worker`s. **Default**: `true`.
* `transferList` {Object[]} If one or more `MessagePort`-like objects
are passed in `workerData`, a `transferList` is required for those
items or [`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`][] will be thrown.

View File

@ -152,7 +152,7 @@ class Worker extends EventEmitter {
env === process.env ? null : env,
options.execArgv,
parseResourceLimits(options.resourceLimits),
!!options.trackUnmanagedFds);
!!(options.trackUnmanagedFds ?? true));
if (this[kHandle].invalidExecArgv) {
throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
}

View File

@ -1,10 +1,13 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { Worker } = require('worker_threads');
const { Worker, isMainThread } = require('worker_threads');
const { once } = require('events');
const fs = require('fs');
if (!isMainThread)
common.skip('test needs to be able to freely set `trackUnmanagedFds`');
// All the tests here are run sequentially, to avoid accidentally opening an fd
// which another part of the test expects to be closed.
@ -37,6 +40,16 @@ process.on('warning', (warning) => parentPort.postMessage({ warning }));
assert.throws(() => fs.fstatSync(fd), { code: 'EBADF' });
}
// The same, but trackUnmanagedFds is used only as the implied default.
{
const w = new Worker(`${preamble}
parentPort.postMessage(fs.openSync(__filename));
`, { eval: true });
const [ [ fd ] ] = await Promise.all([once(w, 'message'), once(w, 'exit')]);
assert(fd > 2);
assert.throws(() => fs.fstatSync(fd), { code: 'EBADF' });
}
// There is a warning when an fd is unexpectedly opened twice.
{
const w = new Worker(`${preamble}