Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 7603c7e

Browse filesBrowse files
addaleaxjasnell
authored andcommitted
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: #34303 PR-URL: #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>
1 parent f4f191b commit 7603c7e
Copy full SHA for 7603c7e

File tree

Expand file treeCollapse file tree

3 files changed

+19
-3
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+19
-3
lines changed
Open diff view settings
Collapse file

‎doc/api/worker_threads.md‎

Copy file name to clipboardExpand all lines: doc/api/worker_threads.md
+4-1Lines changed: 4 additions & 1 deletion
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,9 @@ if (isMainThread) {
621621
<!-- YAML
622622
added: v10.5.0
623623
changes:
624+
- version: REPLACEME
625+
pr-url: https://github.com/nodejs/node/pull/34394
626+
description: The `trackUnmanagedFds` option was set to `true` by default.
624627
- version:
625628
- v14.6.0
626629
pr-url: https://github.com/nodejs/node/pull/34303
@@ -689,7 +692,7 @@ changes:
689692
[`fs.close()`][], and close them when the Worker exits, similar to other
690693
resources like network sockets or file descriptors managed through
691694
the [`FileHandle`][] API. This option is automatically inherited by all
692-
nested `Worker`s. **Default**: `false`.
695+
nested `Worker`s. **Default**: `true`.
693696
* `transferList` {Object[]} If one or more `MessagePort`-like objects
694697
are passed in `workerData`, a `transferList` is required for those
695698
items or [`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`][] will be thrown.
Collapse file

‎lib/internal/worker.js‎

Copy file name to clipboardExpand all lines: lib/internal/worker.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class Worker extends EventEmitter {
152152
env === process.env ? null : env,
153153
options.execArgv,
154154
parseResourceLimits(options.resourceLimits),
155-
!!options.trackUnmanagedFds);
155+
!!(options.trackUnmanagedFds ?? true));
156156
if (this[kHandle].invalidExecArgv) {
157157
throw new ERR_WORKER_INVALID_EXEC_ARGV(this[kHandle].invalidExecArgv);
158158
}
Collapse file

‎test/parallel/test-worker-track-unmanaged-fds.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-worker-track-unmanaged-fds.js
+14-1Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
'use strict';
22
const common = require('../common');
33
const assert = require('assert');
4-
const { Worker } = require('worker_threads');
4+
const { Worker, isMainThread } = require('worker_threads');
55
const { once } = require('events');
66
const fs = require('fs');
77

8+
if (!isMainThread)
9+
common.skip('test needs to be able to freely set `trackUnmanagedFds`');
10+
811
// All the tests here are run sequentially, to avoid accidentally opening an fd
912
// which another part of the test expects to be closed.
1013

@@ -37,6 +40,16 @@ process.on('warning', (warning) => parentPort.postMessage({ warning }));
3740
assert.throws(() => fs.fstatSync(fd), { code: 'EBADF' });
3841
}
3942

43+
// The same, but trackUnmanagedFds is used only as the implied default.
44+
{
45+
const w = new Worker(`${preamble}
46+
parentPort.postMessage(fs.openSync(__filename));
47+
`, { eval: true });
48+
const [ [ fd ] ] = await Promise.all([once(w, 'message'), once(w, 'exit')]);
49+
assert(fd > 2);
50+
assert.throws(() => fs.fstatSync(fd), { code: 'EBADF' });
51+
}
52+
4053
// There is a warning when an fd is unexpectedly opened twice.
4154
{
4255
const w = new Worker(`${preamble}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.