From a0cfb0c9d40578be610cb9d653b6986fee43679b Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 20 May 2018 11:15:00 -0400 Subject: [PATCH 001/150] lib: add validateInteger() validator This allows validation of integers that are not int32 or uint32. PR-URL: https://github.com/nodejs/node/pull/20851 Fixes: https://github.com/nodejs/node/issues/20844 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Backport-PR-URL: https://github.com/nodejs/node/pull/21171 Co-authored-by: Shelley Vohr --- lib/internal/validators.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 556bfb2dc08f5f..aabe71ef33979a 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -13,7 +13,21 @@ function isUint32(value) { return value === (value >>> 0); } -function validateInt32(value, name) { +function validateInteger(value, name) { + let err; + + if (typeof value !== 'number') + err = new ERR_INVALID_ARG_TYPE(name, 'number', value); + else if (!Number.isSafeInteger(value)) + err = new ERR_OUT_OF_RANGE(name, 'an integer', value); + + if (err) { + Error.captureStackTrace(err, validateInteger); + throw err; + } +} + +function validateInt32(value, name, min = -2147483648, max = 2147483647) { if (!isInt32(value)) { let err; if (typeof value !== 'number') { @@ -53,6 +67,7 @@ function validateUint32(value, name, positive) { module.exports = { isInt32, isUint32, + validateInteger, validateInt32, validateUint32 }; From 469baa062ead2e44ab78405eb7b3bae75102caf7 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 20 May 2018 11:36:40 -0400 Subject: [PATCH 002/150] fs: add length validation to fs.truncate() This commit adds validation to the length parameter of fs.truncate(). Prior to this commit, passing a non-number would trigger a CHECK() in the binding layer. Backport-PR-URL: https://github.com/nodejs/node/pull/21171 PR-URL: https://github.com/nodejs/node/pull/20851 Fixes: https://github.com/nodejs/node/issues/20844 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Backport-PR-URL: https://github.com/nodejs/node/pull/21171 Co-authored-by: Shelley Vohr --- lib/fs.js | 2 ++ test/parallel/test-fs-truncate.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/fs.js b/lib/fs.js index 7b05750307c118..cc559a898dc4ea 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -85,6 +85,7 @@ const { } = require('internal/constants'); const { isUint32, + validateInteger, validateInt32, validateUint32 } = require('internal/validators'); @@ -746,6 +747,7 @@ fs.truncate = function(path, len, callback) { len = 0; } + validateInteger(len, 'len'); callback = maybeCallback(callback); fs.open(path, 'r+', function(er, fd) { if (er) return callback(er); diff --git a/test/parallel/test-fs-truncate.js b/test/parallel/test-fs-truncate.js index 2f8839583202d0..347cc0e10492d6 100644 --- a/test/parallel/test-fs-truncate.js +++ b/test/parallel/test-fs-truncate.js @@ -179,6 +179,16 @@ function testFtruncate(cb) { process.on('exit', () => fs.closeSync(fd)); ['', false, null, {}, []].forEach((input) => { + assert.throws( + () => fs.truncate(file5, input, common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError [ERR_INVALID_ARG_TYPE]', + message: 'The "len" argument must be of type number. ' + + `Received type ${typeof input}` + } + ); + assert.throws( () => fs.ftruncate(fd, input), { @@ -191,6 +201,16 @@ function testFtruncate(cb) { }); [-1.5, 1.5].forEach((input) => { + assert.throws( + () => fs.truncate(file5, input), + { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError [ERR_OUT_OF_RANGE]', + message: 'The value of "len" is out of range. It must be ' + + `an integer. Received ${input}` + } + ); + assert.throws( () => fs.ftruncate(fd, input), { From fc0b3610e22c0d6056efd3089b3fe710be54463a Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 20 May 2018 11:41:39 -0400 Subject: [PATCH 003/150] fs: don't limit ftruncate() length to 32 bits The length used by ftruncate() is 64 bits in the binding layer. This commit removes the 32 bit restriction in the JS layer. Backport-PR-URL: https://github.com/nodejs/node/pull/21171 PR-URL: https://github.com/nodejs/node/pull/20851 Fixes: https://github.com/nodejs/node/issues/20844 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Backport-PR-URL: https://github.com/nodejs/node/pull/21171 Co-authored-by: Shelley Vohr --- lib/fs.js | 9 ++------- lib/internal/fs/promises.js | 4 ++-- test/parallel/test-fs-truncate.js | 5 +---- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index cc559a898dc4ea..7168b7875498c5 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -86,7 +86,6 @@ const { const { isUint32, validateInteger, - validateInt32, validateUint32 } = require('internal/validators'); @@ -788,11 +787,7 @@ fs.ftruncate = function(fd, len = 0, callback) { len = 0; } validateUint32(fd, 'fd'); - // TODO(BridgeAR): This does not seem right. - // There does not seem to be any validation before and if there is any, it - // should work similar to validateUint32 or not have a upper cap at all. - // This applies to all usage of `validateInt32(len, 'len')`. - validateInt32(len, 'len'); + validateInteger(len, 'len'); len = Math.max(0, len); const req = new FSReqWrap(); req.oncomplete = makeCallback(callback); @@ -801,7 +796,7 @@ fs.ftruncate = function(fd, len = 0, callback) { fs.ftruncateSync = function(fd, len = 0) { validateUint32(fd, 'fd'); - validateInt32(len, 'len'); + validateInteger(len, 'len'); len = Math.max(0, len); const ctx = {}; binding.ftruncate(fd, len, undefined, ctx); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 80b30c1a8fa21a..3646a4121c7b3a 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -33,7 +33,7 @@ const { validatePath } = require('internal/fs/utils'); const { - validateInt32, + validateInteger, validateUint32 } = require('internal/validators'); const pathModule = require('path'); @@ -264,7 +264,7 @@ async function truncate(path, len = 0) { async function ftruncate(handle, len = 0) { validateFileHandle(handle); - validateInt32(len, 'len'); + validateInteger(len, 'len'); len = Math.max(0, len); return binding.ftruncate(handle.fd, len, kUsePromises); } diff --git a/test/parallel/test-fs-truncate.js b/test/parallel/test-fs-truncate.js index 347cc0e10492d6..735385f61c5249 100644 --- a/test/parallel/test-fs-truncate.js +++ b/test/parallel/test-fs-truncate.js @@ -220,17 +220,14 @@ function testFtruncate(cb) { `an integer. Received ${input}` } ); - }); - // 2 ** 31 = 2147483648 - [2147483648, -2147483649].forEach((input) => { assert.throws( () => fs.ftruncate(fd, input), { code: 'ERR_OUT_OF_RANGE', name: 'RangeError [ERR_OUT_OF_RANGE]', message: 'The value of "len" is out of range. It must be ' + - `> -2147483649 && < 2147483648. Received ${input}` + `an integer. Received ${input}` } ); }); From fc2956d37a75bd51de6392378ae59b0aeae0105e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Wed, 6 Jun 2018 15:05:20 +0200 Subject: [PATCH 004/150] process: backport process/methods file To ease future backports, create the process/methods file introduced in https://github.com/nodejs/node/pull/19973. This commit just adds the JS functions that forward calls to C++ and does not change type checking. PR-URL: https://github.com/nodejs/node/pull/21172 Reviewed-By: Joyee Cheung --- lib/internal/bootstrap/node.js | 1 + lib/internal/process/methods.js | 65 +++++++++++++++++++ node.gyp | 1 + .../{test-umask.js => test-process-umask.js} | 0 4 files changed, 67 insertions(+) create mode 100644 lib/internal/process/methods.js rename test/parallel/{test-umask.js => test-process-umask.js} (100%) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 4d95529c4b58e6..d6fd67fe5d2c85 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -40,6 +40,7 @@ NativeModule.require('internal/process/warning').setup(); NativeModule.require('internal/process/next_tick').setup(); NativeModule.require('internal/process/stdio').setup(); + NativeModule.require('internal/process/methods').setup(); const perf = process.binding('performance'); const { diff --git a/lib/internal/process/methods.js b/lib/internal/process/methods.js new file mode 100644 index 00000000000000..1a720c5cb0e5e4 --- /dev/null +++ b/lib/internal/process/methods.js @@ -0,0 +1,65 @@ +'use strict'; + +function setupProcessMethods() { + // Non-POSIX platforms like Windows don't have certain methods. + if (process.setgid !== undefined) { + setupPosixMethods(); + } + + const { chdir: _chdir, umask: _umask } = process; + + process.chdir = chdir; + process.umask = umask; + + function chdir(...args) { + return _chdir(...args); + } + + function umask(...args) { + return _umask(...args); + } +} + +function setupPosixMethods() { + const { + initgroups: _initgroups, + setegid: _setegid, + seteuid: _seteuid, + setgid: _setgid, + setuid: _setuid, + setgroups: _setgroups + } = process; + + process.initgroups = initgroups; + process.setegid = setegid; + process.seteuid = seteuid; + process.setgid = setgid; + process.setuid = setuid; + process.setgroups = setgroups; + + function initgroups(...args) { + return _initgroups(...args); + } + + function setegid(...args) { + return _setegid(...args); + } + + function seteuid(...args) { + return _seteuid(...args); + } + + function setgid(...args) { + return _setgid(...args); + } + + function setuid(...args) { + return _setuid(...args); + } + + function setgroups(...args) { + return _setgroups(...args); + } +} + +exports.setup = setupProcessMethods; diff --git a/node.gyp b/node.gyp index 19367d9bd9517d..7bcc17ce843798 100644 --- a/node.gyp +++ b/node.gyp @@ -120,6 +120,7 @@ 'lib/internal/net.js', 'lib/internal/os.js', 'lib/internal/process/esm_loader.js', + 'lib/internal/process/methods.js', 'lib/internal/process/next_tick.js', 'lib/internal/process/promises.js', 'lib/internal/process/stdio.js', diff --git a/test/parallel/test-umask.js b/test/parallel/test-process-umask.js similarity index 100% rename from test/parallel/test-umask.js rename to test/parallel/test-process-umask.js From 2fe88d2218bcfbcb19a237d322069ce72687c414 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 9 May 2018 22:44:44 +0800 Subject: [PATCH 005/150] lib: mask mode_t type of arguments with 0o777 - Introduce the `validateAndMaskMode` validator that validates `mode_t` arguments and mask them with 0o777 if they are 32-bit unsigned integer or octal string to be more consistent with POSIX APIs. - Use the validator in fs APIs and process.umask for consistency. - Add tests for 32-bit unsigned modes larger than 0o777. PR-URL: https://github.com/nodejs/node/pull/20636 Fixes: https://github.com/nodejs/node/issues/20498 Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski Backport-PR-URL: https://github.com/nodejs/node/pull/21172 --- lib/fs.js | 63 ++++++++-------- lib/internal/fs/promises.js | 19 ++--- lib/internal/fs/utils.js | 16 ---- lib/internal/validators.js | 36 +++++++++ test/parallel/test-fs-chmod-mask.js | 95 ++++++++++++++++++++++++ test/parallel/test-fs-chmod.js | 8 +- test/parallel/test-fs-fchmod.js | 38 +++------- test/parallel/test-fs-mkdir-mode-mask.js | 45 +++++++++++ test/parallel/test-fs-open-mode-mask.js | 48 ++++++++++++ test/parallel/test-fs-promises.js | 26 +------ test/parallel/test-process-umask-mask.js | 29 ++++++++ test/parallel/test-process-umask.js | 6 +- 12 files changed, 313 insertions(+), 116 deletions(-) create mode 100644 test/parallel/test-fs-chmod-mask.js create mode 100644 test/parallel/test-fs-mkdir-mode-mask.js create mode 100644 test/parallel/test-fs-open-mode-mask.js create mode 100644 test/parallel/test-process-umask-mask.js diff --git a/lib/fs.js b/lib/fs.js index 7168b7875498c5..2e4d8499ef62e3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -65,7 +65,6 @@ const internalUtil = require('internal/util'); const { copyObject, getOptions, - modeNum, nullCheck, preprocessSymlinkDestination, Stats, @@ -85,6 +84,7 @@ const { } = require('internal/constants'); const { isUint32, + validateAndMaskMode, validateInteger, validateUint32 } = require('internal/validators'); @@ -549,32 +549,36 @@ fs.closeSync = function(fd) { handleErrorFromBinding(ctx); }; -fs.open = function(path, flags, mode, callback_) { - var callback = makeCallback(arguments[arguments.length - 1]); - mode = modeNum(mode, 0o666); - +fs.open = function(path, flags, mode, callback) { path = getPathFromURL(path); validatePath(path); - validateUint32(mode, 'mode'); + const flagsNumber = stringToFlags(flags); + if (arguments.length < 4) { + callback = makeCallback(mode); + mode = 0o666; + } else { + mode = validateAndMaskMode(mode, 'mode', 0o666); + callback = makeCallback(callback); + } const req = new FSReqWrap(); req.oncomplete = callback; binding.open(pathModule.toNamespacedPath(path), - stringToFlags(flags), + flagsNumber, mode, req); }; fs.openSync = function(path, flags, mode) { - mode = modeNum(mode, 0o666); path = getPathFromURL(path); validatePath(path); - validateUint32(mode, 'mode'); + const flagsNumber = stringToFlags(flags); + mode = validateAndMaskMode(mode, 'mode', 0o666); const ctx = { path }; const result = binding.open(pathModule.toNamespacedPath(path), - stringToFlags(flags), mode, + flagsNumber, mode, undefined, ctx); handleErrorFromBinding(ctx); return result; @@ -849,12 +853,16 @@ fs.fsyncSync = function(fd) { }; fs.mkdir = function(path, mode, callback) { - if (typeof mode === 'function') callback = mode; - callback = makeCallback(callback); path = getPathFromURL(path); validatePath(path); - mode = modeNum(mode, 0o777); - validateUint32(mode, 'mode'); + + if (arguments.length < 3) { + callback = makeCallback(mode); + mode = 0o777; + } else { + callback = makeCallback(callback); + mode = validateAndMaskMode(mode, 'mode', 0o777); + } const req = new FSReqWrap(); req.oncomplete = callback; @@ -864,8 +872,7 @@ fs.mkdir = function(path, mode, callback) { fs.mkdirSync = function(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = modeNum(mode, 0o777); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode', 0o777); const ctx = { path }; binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx); handleErrorFromBinding(ctx); @@ -1047,25 +1054,18 @@ fs.unlinkSync = function(path) { }; fs.fchmod = function(fd, mode, callback) { - mode = modeNum(mode); validateUint32(fd, 'fd'); - validateUint32(mode, 'mode'); - // Values for mode < 0 are already checked via the validateUint32 function - if (mode > 0o777) - throw new ERR_OUT_OF_RANGE('mode', undefined, mode); + mode = validateAndMaskMode(mode, 'mode'); + callback = makeCallback(callback); const req = new FSReqWrap(); - req.oncomplete = makeCallback(callback); + req.oncomplete = callback; binding.fchmod(fd, mode, req); }; fs.fchmodSync = function(fd, mode) { - mode = modeNum(mode); validateUint32(fd, 'fd'); - validateUint32(mode, 'mode'); - // Values for mode < 0 are already checked via the validateUint32 function - if (mode > 0o777) - throw new ERR_OUT_OF_RANGE('mode', undefined, mode); + mode = validateAndMaskMode(mode, 'mode'); const ctx = {}; binding.fchmod(fd, mode, undefined, ctx); handleErrorFromBinding(ctx); @@ -1106,11 +1106,10 @@ if (O_SYMLINK !== undefined) { fs.chmod = function(path, mode, callback) { - callback = makeCallback(callback); path = getPathFromURL(path); validatePath(path); - mode = modeNum(mode); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode'); + callback = makeCallback(callback); const req = new FSReqWrap(); req.oncomplete = callback; @@ -1120,8 +1119,8 @@ fs.chmod = function(path, mode, callback) { fs.chmodSync = function(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = modeNum(mode); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode'); + const ctx = { path }; binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx); handleErrorFromBinding(ctx); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 3646a4121c7b3a..382407e89c4be8 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -12,8 +12,7 @@ const { Buffer, kMaxLength } = require('buffer'); const { ERR_FS_FILE_TOO_LARGE, ERR_INVALID_ARG_TYPE, - ERR_METHOD_NOT_IMPLEMENTED, - ERR_OUT_OF_RANGE + ERR_METHOD_NOT_IMPLEMENTED } = require('internal/errors').codes; const { getPathFromURL } = require('internal/url'); const { isUint8Array } = require('internal/util/types'); @@ -21,7 +20,6 @@ const { copyObject, getOptions, getStatsFromBinding, - modeNum, nullCheck, preprocessSymlinkDestination, stringToFlags, @@ -33,6 +31,7 @@ const { validatePath } = require('internal/fs/utils'); const { + validateAndMaskMode, validateInteger, validateUint32 } = require('internal/validators'); @@ -190,10 +189,9 @@ async function copyFile(src, dest, flags) { // Note that unlike fs.open() which uses numeric file descriptors, // fsPromises.open() uses the fs.FileHandle class. async function open(path, flags, mode) { - mode = modeNum(mode, 0o666); path = getPathFromURL(path); validatePath(path); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode', 0o666); return new FileHandle( await binding.openFileHandle(pathModule.toNamespacedPath(path), stringToFlags(flags), @@ -286,10 +284,9 @@ async function fsync(handle) { } async function mkdir(path, mode) { - mode = modeNum(mode, 0o777); path = getPathFromURL(path); validatePath(path); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode', 0o777); return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises); } @@ -360,19 +357,15 @@ async function unlink(path) { } async function fchmod(handle, mode) { - mode = modeNum(mode); validateFileHandle(handle); - validateUint32(mode, 'mode'); - if (mode > 0o777) - throw new ERR_OUT_OF_RANGE('mode', undefined, mode); + mode = validateAndMaskMode(mode, 'mode'); return binding.fchmod(handle.fd, mode, kUsePromises); } async function chmod(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = modeNum(mode); - validateUint32(mode, 'mode'); + mode = validateAndMaskMode(mode, 'mode'); return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises); } diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 2f7a8d8ced176e..46b3a97f741572 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -70,21 +70,6 @@ function getOptions(options, defaultOptions) { return options; } -function modeNum(m, def) { - if (typeof m === 'number') - return m; - if (typeof m === 'string') { - const parsed = parseInt(m, 8); - if (Number.isNaN(parsed)) - return m; - return parsed; - } - // TODO(BridgeAR): Only return `def` in case `m == null` - if (def !== undefined) - return def; - return m; -} - // Check if the path contains null types if it is a string nor Uint8Array, // otherwise return silently. function nullCheck(path, propName, throwError = true) { @@ -391,7 +376,6 @@ module.exports = { assertEncoding, copyObject, getOptions, - modeNum, nullCheck, preprocessSymlinkDestination, realpathCacheKey: Symbol('realpathCacheKey'), diff --git a/lib/internal/validators.js b/lib/internal/validators.js index aabe71ef33979a..991af52fee5b95 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -2,6 +2,7 @@ const { ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, ERR_OUT_OF_RANGE } = require('internal/errors').codes; @@ -13,6 +14,40 @@ function isUint32(value) { return value === (value >>> 0); } +const octalReg = /^[0-7]+$/; +const modeDesc = 'must be a 32-bit unsigned integer or an octal string'; +// Validator for mode_t (the S_* constants). Valid numbers or octal strings +// will be masked with 0o777 to be consistent with the behavior in POSIX APIs. +function validateAndMaskMode(value, name, def) { + if (isUint32(value)) { + return value & 0o777; + } + + if (typeof value === 'number') { + if (!Number.isInteger(value)) { + throw new ERR_OUT_OF_RANGE(name, 'an integer', value); + } else { + // 2 ** 32 === 4294967296 + throw new ERR_OUT_OF_RANGE(name, '>= 0 && < 4294967296', value); + } + } + + if (typeof value === 'string') { + if (!octalReg.test(value)) { + throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc); + } + const parsed = parseInt(value, 8); + return parsed & 0o777; + } + + // TODO(BridgeAR): Only return `def` in case `value == null` + if (def !== undefined) { + return def; + } + + throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc); +} + function validateInteger(value, name) { let err; @@ -67,6 +102,7 @@ function validateUint32(value, name, positive) { module.exports = { isInt32, isUint32, + validateAndMaskMode, validateInteger, validateInt32, validateUint32 diff --git a/test/parallel/test-fs-chmod-mask.js b/test/parallel/test-fs-chmod-mask.js new file mode 100644 index 00000000000000..5f3a8d5ab82864 --- /dev/null +++ b/test/parallel/test-fs-chmod-mask.js @@ -0,0 +1,95 @@ +'use strict'; + +// This tests that mode > 0o777 will be masked off with 0o777 in fs APIs. + +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); + +let mode; +// On Windows chmod is only able to manipulate write permission +if (common.isWindows) { + mode = 0o444; // read-only +} else { + mode = 0o777; +} + +const maskToIgnore = 0o10000; + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +function test(mode, asString) { + const suffix = asString ? 'str' : 'num'; + const input = asString ? + (mode | maskToIgnore).toString(8) : (mode | maskToIgnore); + + { + const file = path.join(tmpdir.path, `chmod-async-${suffix}.txt`); + fs.writeFileSync(file, 'test', 'utf-8'); + + fs.chmod(file, input, common.mustCall((err) => { + assert.ifError(err); + assert.strictEqual(fs.statSync(file).mode & 0o777, mode); + })); + } + + { + const file = path.join(tmpdir.path, `chmodSync-${suffix}.txt`); + fs.writeFileSync(file, 'test', 'utf-8'); + + fs.chmodSync(file, input); + assert.strictEqual(fs.statSync(file).mode & 0o777, mode); + } + + { + const file = path.join(tmpdir.path, `fchmod-async-${suffix}.txt`); + fs.writeFileSync(file, 'test', 'utf-8'); + fs.open(file, 'w', common.mustCall((err, fd) => { + assert.ifError(err); + + fs.fchmod(fd, input, common.mustCall((err) => { + assert.ifError(err); + assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode); + fs.close(fd, assert.ifError); + })); + })); + } + + { + const file = path.join(tmpdir.path, `fchmodSync-${suffix}.txt`); + fs.writeFileSync(file, 'test', 'utf-8'); + const fd = fs.openSync(file, 'w'); + + fs.fchmodSync(fd, input); + assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode); + + fs.close(fd, assert.ifError); + } + + if (fs.lchmod) { + const link = path.join(tmpdir.path, `lchmod-src-${suffix}`); + const file = path.join(tmpdir.path, `lchmod-dest-${suffix}`); + fs.writeFileSync(file, 'test', 'utf-8'); + fs.symlinkSync(file, link); + + fs.lchmod(link, input, common.mustCall((err) => { + assert.ifError(err); + assert.strictEqual(fs.lstatSync(link).mode & 0o777, mode); + })); + } + + if (fs.lchmodSync) { + const link = path.join(tmpdir.path, `lchmodSync-src-${suffix}`); + const file = path.join(tmpdir.path, `lchmodSync-dest-${suffix}`); + fs.writeFileSync(file, 'test', 'utf-8'); + fs.symlinkSync(file, link); + + fs.lchmodSync(link, input); + assert.strictEqual(fs.lstatSync(link).mode & 0o777, mode); + } +} + +test(mode, true); +test(mode, false); diff --git a/test/parallel/test-fs-chmod.js b/test/parallel/test-fs-chmod.js index 6727ec2144f2ce..ed5919ce887bc8 100644 --- a/test/parallel/test-fs-chmod.js +++ b/test/parallel/test-fs-chmod.js @@ -62,7 +62,7 @@ function closeSync() { } -// On Windows chmod is only able to manipulate read-only bit +// On Windows chmod is only able to manipulate write permission if (common.isWindows) { mode_async = 0o400; // read-only mode_sync = 0o600; // read-write @@ -112,10 +112,10 @@ fs.open(file2, 'w', common.mustCall((err, fd) => { common.expectsError( () => fs.fchmod(fd, {}), { - code: 'ERR_INVALID_ARG_TYPE', + code: 'ERR_INVALID_ARG_VALUE', type: TypeError, - message: 'The "mode" argument must be of type number. ' + - 'Received type object' + message: 'The argument \'mode\' must be a 32-bit unsigned integer ' + + 'or an octal string. Received {}' } ); diff --git a/test/parallel/test-fs-fchmod.js b/test/parallel/test-fs-fchmod.js index 63d780a57dfc66..df7748538a5cc4 100644 --- a/test/parallel/test-fs-fchmod.js +++ b/test/parallel/test-fs-fchmod.js @@ -1,6 +1,7 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); +const util = require('util'); const fs = require('fs'); // This test ensures that input for fchmod is valid, testing for valid @@ -16,7 +17,16 @@ const fs = require('fs'); }; assert.throws(() => fs.fchmod(input), errObj); assert.throws(() => fs.fchmodSync(input), errObj); - errObj.message = errObj.message.replace('fd', 'mode'); +}); + + +[false, null, undefined, {}, [], '', '123x'].forEach((input) => { + const errObj = { + code: 'ERR_INVALID_ARG_VALUE', + name: 'TypeError [ERR_INVALID_ARG_VALUE]', + message: 'The argument \'mode\' must be a 32-bit unsigned integer or an ' + + `octal string. Received ${util.inspect(input)}` + }; assert.throws(() => fs.fchmod(1, input), errObj); assert.throws(() => fs.fchmodSync(1, input), errObj); }); @@ -62,27 +72,3 @@ const fs = require('fs'); assert.throws(() => fs.fchmod(1, input), errObj); assert.throws(() => fs.fchmodSync(1, input), errObj); }); - -// Check for mode values range -const modeUpperBoundaryValue = 0o777; -fs.fchmod(1, modeUpperBoundaryValue, common.mustCall()); -fs.fchmodSync(1, modeUpperBoundaryValue); - -// umask of 0o777 is equal to 775 -const modeOutsideUpperBoundValue = 776; -assert.throws( - () => fs.fchmod(1, modeOutsideUpperBoundValue), - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError [ERR_OUT_OF_RANGE]', - message: 'The value of "mode" is out of range. Received 776' - } -); -assert.throws( - () => fs.fchmodSync(1, modeOutsideUpperBoundValue), - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError [ERR_OUT_OF_RANGE]', - message: 'The value of "mode" is out of range. Received 776' - } -); diff --git a/test/parallel/test-fs-mkdir-mode-mask.js b/test/parallel/test-fs-mkdir-mode-mask.js new file mode 100644 index 00000000000000..e4e8a423483376 --- /dev/null +++ b/test/parallel/test-fs-mkdir-mode-mask.js @@ -0,0 +1,45 @@ +'use strict'; + +// This tests that mode > 0o777 will be masked off with 0o777 in fs.mkdir(). + +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); + +let mode; + +if (common.isWindows) { + common.skip('mode is not supported in mkdir on Windows'); + return; +} else { + mode = 0o644; +} + +const maskToIgnore = 0o10000; + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +function test(mode, asString) { + const suffix = asString ? 'str' : 'num'; + const input = asString ? + (mode | maskToIgnore).toString(8) : (mode | maskToIgnore); + + { + const dir = path.join(tmpdir.path, `mkdirSync-${suffix}`); + fs.mkdirSync(dir, input); + assert.strictEqual(fs.statSync(dir).mode & 0o777, mode); + } + + { + const dir = path.join(tmpdir.path, `mkdir-${suffix}`); + fs.mkdir(dir, input, common.mustCall((err) => { + assert.ifError(err); + assert.strictEqual(fs.statSync(dir).mode & 0o777, mode); + })); + } +} + +test(mode, true); +test(mode, false); diff --git a/test/parallel/test-fs-open-mode-mask.js b/test/parallel/test-fs-open-mode-mask.js new file mode 100644 index 00000000000000..4db41864af099c --- /dev/null +++ b/test/parallel/test-fs-open-mode-mask.js @@ -0,0 +1,48 @@ +'use strict'; + +// This tests that mode > 0o777 will be masked off with 0o777 in fs.open(). + +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); + +let mode; + +if (common.isWindows) { + mode = 0o444; +} else { + mode = 0o644; +} + +const maskToIgnore = 0o10000; + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +function test(mode, asString) { + const suffix = asString ? 'str' : 'num'; + const input = asString ? + (mode | maskToIgnore).toString(8) : (mode | maskToIgnore); + + { + const file = path.join(tmpdir.path, `openSync-${suffix}.txt`); + const fd = fs.openSync(file, 'w+', input); + assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode); + fs.closeSync(fd); + assert.strictEqual(fs.statSync(file).mode & 0o777, mode); + } + + { + const file = path.join(tmpdir.path, `open-${suffix}.txt`); + fs.open(file, 'w+', input, common.mustCall((err, fd) => { + assert.ifError(err); + assert.strictEqual(fs.fstatSync(fd).mode & 0o777, mode); + fs.closeSync(fd); + assert.strictEqual(fs.statSync(file).mode & 0o777, mode); + })); + } +} + +test(mode, true); +test(mode, false); diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index b7e01544162e81..53219342d93af8 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -114,28 +114,10 @@ function verifyStatObject(stat) { await fchmod(handle, 0o666); await handle.chmod(0o666); - // `mode` can't be > 0o777 - assert.rejects( - async () => chmod(handle, (0o777 + 1)), - { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError [ERR_INVALID_ARG_TYPE]' - } - ); - assert.rejects( - async () => fchmod(handle, (0o777 + 1)), - { - code: 'ERR_OUT_OF_RANGE', - name: 'RangeError [ERR_OUT_OF_RANGE]' - } - ); - assert.rejects( - async () => handle.chmod(handle, (0o777 + 1)), - { - code: 'ERR_INVALID_ARG_TYPE', - name: 'TypeError [ERR_INVALID_ARG_TYPE]' - } - ); + // Mode larger than 0o777 should be masked off. + await chmod(dest, (0o777 + 1)); + await fchmod(handle, 0o777 + 1); + await handle.chmod(0o777 + 1); await utimes(dest, new Date(), new Date()); diff --git a/test/parallel/test-process-umask-mask.js b/test/parallel/test-process-umask-mask.js new file mode 100644 index 00000000000000..c312c061d2b76a --- /dev/null +++ b/test/parallel/test-process-umask-mask.js @@ -0,0 +1,29 @@ +'use strict'; + +// This tests that mask > 0o777 will be masked off with 0o777 in +// process.umask() + +const common = require('../common'); +const assert = require('assert'); + +let mask; + +if (common.isWindows) { + mask = 0o600; +} else { + mask = 0o664; +} + +const maskToIgnore = 0o10000; + +const old = process.umask(); + +function test(input, output) { + process.umask(input); + assert.strictEqual(process.umask(), output); + + process.umask(old); +} + +test(mask | maskToIgnore, mask); +test((mask | maskToIgnore).toString(8), mask); diff --git a/test/parallel/test-process-umask.js b/test/parallel/test-process-umask.js index 1ac32cb7018906..c0c00f3ba397f8 100644 --- a/test/parallel/test-process-umask.js +++ b/test/parallel/test-process-umask.js @@ -33,13 +33,13 @@ if (common.isWindows) { const old = process.umask(mask); -assert.strictEqual(parseInt(mask, 8), process.umask(old)); +assert.strictEqual(process.umask(old), parseInt(mask, 8)); // confirm reading the umask does not modify it. // 1. If the test fails, this call will succeed, but the mask will be set to 0 -assert.strictEqual(old, process.umask()); +assert.strictEqual(process.umask(), old); // 2. If the test fails, process.umask() will return 0 -assert.strictEqual(old, process.umask()); +assert.strictEqual(process.umask(), old); assert.throws(() => { process.umask({}); From a5c571424ac9009203126b8e2c9ed51ae26dbf7d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 10 May 2018 01:26:42 +0800 Subject: [PATCH 006/150] doc: document file mode caveats on Windows - On Windows only the write permission (read-only bit) can be manipulated, and there is no distinction among owner, group or others. - mkdir on Windows does not support the mode argument. PR-URL: https://github.com/nodejs/node/pull/20636 Fixes: https://github.com/nodejs/node/issues/20498 Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski Backport-PR-URL: https://github.com/nodejs/node/pull/21172 --- doc/api/fs.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index c2daa8d71a5055..af06bf91e5b5f5 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1089,6 +1089,10 @@ For example, the octal value `0o765` means: * The group may read and write the file. * Others may read and execute the file. +Caveats: on Windows only the write permission can be changed, and the +distinction among the permissions of group, owner or others is not +implemented. + ## fs.chmodSync(path, mode) * `path` {string|Buffer|URL} -* `mode` {integer} **Default:** `0o777` +* `mode` {integer} Not supported on Windows. **Default:** `0o777`. * `callback` {Function} * `err` {Error} @@ -2007,7 +2011,7 @@ changes: --> * `path` {string|Buffer|URL} -* `mode` {integer} **Default:** `0o777` +* `mode` {integer} Not supported on Windows. **Default:** `0o777`. Synchronously creates a directory. Returns `undefined`. This is the synchronous version of [`fs.mkdir()`][]. @@ -2127,7 +2131,8 @@ changes: Asynchronous file open. See open(2). `mode` sets the file mode (permission and sticky bits), but only if the file was -created. +created. Note that on Windows only the write permission can be manipulated, +see [`fs.chmod()`][]. The callback gets two arguments `(err, fd)`. From 2ffb9d6b5cc374ca128d9e8dc656c80ccc74c3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=BA=D0=BE=D0=B2=D0=BE=D1=80=D0=BE=D0=B4=D0=B0=20?= =?UTF-8?q?=D0=9D=D0=B8=D0=BA=D0=B8=D1=82=D0=B0=20=D0=90=D0=BD=D0=B4=D1=80?= =?UTF-8?q?=D0=B5=D0=B5=D0=B2=D0=B8=D1=87?= Date: Sun, 6 May 2018 12:13:56 +0300 Subject: [PATCH 007/150] fs: drop duplicate API in promises mode This drops exporting duplicate methods that accept FileHandle as the first argument (to mirror callback-based methods accepting 'fd'). Those methods were not adding actual value to the API because all of those are already present as FileHandle methods, and they would probably be confusing to the new users and making docs harder to read. Also, the API was a bit inconsistent and lacked .close(handle). Fixes: https://github.com/nodejs/node/issues/20548 PR-URL: https://github.com/nodejs/node/pull/20559 Reviewed-By: Matteo Collina Reviewed-By: Franziska Hinkelmann Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Colin Ihrig Reviewed-By: Joyee Cheung Reviewed-By: Ruben Bridgewater Backport-PR-URL: https://github.com/nodejs/node/pull/21172 --- benchmark/fs/bench-stat-promise.js | 6 +- doc/api/fs.md | 184 ----------------------------- lib/internal/fs/promises.js | 9 -- test/parallel/test-fs-promises.js | 23 +--- 4 files changed, 8 insertions(+), 214 deletions(-) diff --git a/benchmark/fs/bench-stat-promise.js b/benchmark/fs/bench-stat-promise.js index 96c7058fa6218a..99a5da5799b787 100644 --- a/benchmark/fs/bench-stat-promise.js +++ b/benchmark/fs/bench-stat-promise.js @@ -9,12 +9,12 @@ const bench = common.createBenchmark(main, { }); async function run(n, statType) { - const arg = statType === 'fstat' ? - await fsPromises.open(__filename, 'r') : __filename; + const handleMode = statType === 'fstat'; + const arg = handleMode ? await fsPromises.open(__filename, 'r') : __filename; let remaining = n; bench.start(); while (remaining-- > 0) - await fsPromises[statType](arg); + await (handleMode ? arg.stat() : fsPromises[statType](arg)); bench.end(n); if (typeof arg.close === 'function') diff --git a/doc/api/fs.md b/doc/api/fs.md index af06bf91e5b5f5..bdd5c9758cc3e3 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -3800,128 +3800,6 @@ fsPromises.copyFile('source.txt', 'destination.txt', COPYFILE_EXCL) .catch(() => console.log('The file could not be copied')); ``` -### fsPromises.fchmod(filehandle, mode) - - -* `filehandle` {FileHandle} -* `mode` {integer} -* Returns: {Promise} - -Asynchronous fchmod(2). The `Promise` is resolved with no arguments upon -success. - -### fsPromises.fchown(filehandle, uid, gid) - - -* `filehandle` {FileHandle} -* `uid` {integer} -* `gid` {integer} -* Returns: {Promise} - -Changes the ownership of the file represented by `filehandle` then resolves -the `Promise` with no arguments upon success. - -### fsPromises.fdatasync(filehandle) - - -* `filehandle` {FileHandle} -* Returns: {Promise} - -Asynchronous fdatasync(2). The `Promise` is resolved with no arguments upon -success. - -### fsPromises.fstat(filehandle) - - -* `filehandle` {FileHandle} -* Returns: {Promise} - -Retrieves the [`fs.Stats`][] for the given `filehandle`. - -### fsPromises.fsync(filehandle) - - -* `filehandle` {FileHandle} -* Returns: {Promise} - -Asynchronous fsync(2). The `Promise` is resolved with no arguments upon -success. - -### fsPromises.ftruncate(filehandle[, len]) - - -* `filehandle` {FileHandle} -* `len` {integer} **Default:** `0` -* Returns: {Promise} - -Truncates the file represented by `filehandle` then resolves the `Promise` -with no arguments upon success. - -If the file referred to by the `FileHandle` was larger than `len` bytes, only -the first `len` bytes will be retained in the file. - -For example, the following program retains only the first four bytes of the -file: - -```js -console.log(fs.readFileSync('temp.txt', 'utf8')); -// Prints: Node.js - -async function doTruncate() { - const fd = await fsPromises.open('temp.txt', 'r+'); - await fsPromises.ftruncate(fd, 4); - console.log(fs.readFileSync('temp.txt', 'utf8')); // Prints: Node -} - -doTruncate().catch(console.error); -``` - -If the file previously was shorter than `len` bytes, it is extended, and the -extended part is filled with null bytes (`'\0'`). For example, - -```js -console.log(fs.readFileSync('temp.txt', 'utf8')); -// Prints: Node.js - -async function doTruncate() { - const fd = await fsPromises.open('temp.txt', 'r+'); - await fsPromises.ftruncate(fd, 10); - console.log(fs.readFileSync('temp.txt', 'utf8')); // Prints Node.js\0\0\0 -} - -doTruncate().catch(console.error); -``` - -The last three bytes are null bytes (`'\0'`), to compensate the over-truncation. - -### fsPromises.futimes(filehandle, atime, mtime) - - -* `filehandle` {FileHandle} -* `atime` {number|string|Date} -* `mtime` {number|string|Date} -* Returns: {Promise} - -Change the file system timestamps of the object referenced by the supplied -`FileHandle` then resolves the `Promise` with no arguments upon success. - -This function does not work on AIX versions before 7.1, it will resolve the -`Promise` with an error using code `UV_ENOSYS`. - ### fsPromises.lchmod(path, mode) - -* `filehandle` {FileHandle} -* `buffer` {Buffer|Uint8Array} -* `offset` {integer} -* `length` {integer} -* `position` {integer} -* Returns: {Promise} - -Read data from the file specified by `filehandle`. - -`buffer` is the buffer that the data will be written to. - -`offset` is the offset in the buffer to start writing at. - -`length` is an integer specifying the number of bytes to read. - -`position` is an argument specifying where to begin reading from in the file. -If `position` is `null`, data will be read from the current file position, -and the file position will be updated. -If `position` is an integer, the file position will remain unchanged. - -Following successful read, the `Promise` is resolved with an object with a -`bytesRead` property specifying the number of bytes read, and a `buffer` -property that is a reference to the passed in `buffer` argument. - ### fsPromises.readdir(path[, options]) - -* `filehandle` {FileHandle} -* `buffer` {Buffer|Uint8Array} -* `offset` {integer} -* `length` {integer} -* `position` {integer} -* Returns: {Promise} - -Write `buffer` to the file specified by `filehandle`. - -The `Promise` is resolved with an object containing a `bytesWritten` property -identifying the number of bytes written, and a `buffer` property containing -a reference to the `buffer` written. - -`offset` determines the part of the buffer to be written, and `length` is -an integer specifying the number of bytes to write. - -`position` refers to the offset from the beginning of the file where this data -should be written. If `typeof position !== 'number'`, the data will be written -at the current position. See pwrite(2). - -It is unsafe to use `fsPromises.write()` multiple times on the same file -without waiting for the `Promise` to be resolved (or rejected). For this -scenario, `fs.createWriteStream` is strongly recommended. - -On Linux, positional writes do not work when the file is opened in append mode. -The kernel ignores the position argument and always appends the data to -the end of the file. - ### fsPromises.writeFile(file, data[, options]) -* `host` {string} The hostname to verify the certificate against +* `hostname` {string} The hostname to verify the certificate against * `cert` {Object} An object representing the peer's certificate. The returned object has some properties corresponding to the fields of the certificate. * Returns: {Error|undefined} -Verifies the certificate `cert` is issued to host `host`. +Verifies the certificate `cert` is issued to `hostname`. Returns {Error} object, populating it with the reason, host, and cert on failure. On success, returns {undefined}. diff --git a/lib/tls.js b/lib/tls.js index 1e444d5d8898c2..f4b72851907862 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -169,14 +169,14 @@ function check(hostParts, pattern, wildcards) { return true; } -exports.checkServerIdentity = function checkServerIdentity(host, cert) { +exports.checkServerIdentity = function checkServerIdentity(hostname, cert) { const subject = cert.subject; const altNames = cert.subjectaltname; const dnsNames = []; const uriNames = []; const ips = []; - host = '' + host; + hostname = '' + hostname; if (altNames) { for (const name of altNames.split(', ')) { @@ -194,14 +194,14 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { let valid = false; let reason = 'Unknown reason'; - if (net.isIP(host)) { - valid = ips.includes(canonicalizeIP(host)); + if (net.isIP(hostname)) { + valid = ips.includes(canonicalizeIP(hostname)); if (!valid) - reason = `IP: ${host} is not in the cert's list: ${ips.join(', ')}`; + reason = `IP: ${hostname} is not in the cert's list: ${ips.join(', ')}`; // TODO(bnoordhuis) Also check URI SANs that are IP addresses. } else if (subject) { - host = unfqdn(host); // Remove trailing dot for error messages. - const hostParts = splitHost(host); + hostname = unfqdn(hostname); // Remove trailing dot for error messages. + const hostParts = splitHost(hostname); const wildcard = (pattern) => check(hostParts, pattern, true); const noWildcard = (pattern) => check(hostParts, pattern, false); @@ -215,11 +215,12 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { valid = wildcard(cn); if (!valid) - reason = `Host: ${host}. is not cert's CN: ${cn}`; + reason = `Host: ${hostname}. is not cert's CN: ${cn}`; } else { valid = dnsNames.some(wildcard) || uriNames.some(noWildcard); if (!valid) - reason = `Host: ${host}. is not in the cert's altnames: ${altNames}`; + reason = + `Host: ${hostname}. is not in the cert's altnames: ${altNames}`; } } else { reason = 'Cert is empty'; @@ -228,7 +229,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) { if (!valid) { const err = new ERR_TLS_CERT_ALTNAME_INVALID(reason); err.reason = reason; - err.host = host; + err.host = hostname; err.cert = cert; return err; } From b4b7d368bee9f059b2786ab3f901c759ff72722b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 26 May 2018 18:51:19 +0800 Subject: [PATCH 014/150] lib: unmask mode_t values with 0o777 This commit allows permission bits higher than 0o777 to go through the API (e.g. `S_ISVTX`=`0o1000`, `S_ISGID`=`0o2000`, `S_ISUID`=`0o4000`). Also documents that these bits are not exposed through `fs.constants` and their behaviors are platform-specific, so the users need to use them on their own risk. PR-URL: https://github.com/nodejs/node/pull/20975 Reviewed-By: Luigi Pinca Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Backport-PR-URL: https://github.com/nodejs/node/pull/21172 --- doc/api/fs.md | 6 ++++++ lib/fs.js | 18 +++++++++--------- lib/internal/fs/promises.js | 10 +++++----- lib/internal/validators.js | 23 +++++++++++++++++------ test/parallel/test-fs-chmod-mask.js | 2 +- test/parallel/test-fs-mkdir-mode-mask.js | 2 +- test/parallel/test-fs-open-mode-mask.js | 2 +- test/parallel/test-fs-promises.js | 5 ++--- test/parallel/test-process-umask-mask.js | 2 +- 9 files changed, 43 insertions(+), 27 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index bdd5c9758cc3e3..e492660bae2f2a 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -1089,6 +1089,12 @@ For example, the octal value `0o765` means: * The group may read and write the file. * Others may read and execute the file. +Note: When using raw numbers where file modes are expected, +any value larger than `0o777` may result in platform-specific +behaviors that are not supported to work consistently. +Therefore constants like `S_ISVTX`, `S_ISGID` or `S_ISUID` are +not exposed in `fs.constants`. + Caveats: on Windows only the write permission can be changed, and the distinction among the permissions of group, owner or others is not implemented. diff --git a/lib/fs.js b/lib/fs.js index 49050af6677cd6..983625bc9b0af3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -77,7 +77,7 @@ const { } = require('internal/constants'); const { isUint32, - validateAndMaskMode, + validateMode, validateInteger, validateInt32, validateUint32 @@ -416,7 +416,7 @@ function open(path, flags, mode, callback) { callback = makeCallback(mode); mode = 0o666; } else { - mode = validateAndMaskMode(mode, 'mode', 0o666); + mode = validateMode(mode, 'mode', 0o666); callback = makeCallback(callback); } @@ -434,7 +434,7 @@ function openSync(path, flags, mode) { path = getPathFromURL(path); validatePath(path); const flagsNumber = stringToFlags(flags); - mode = validateAndMaskMode(mode, 'mode', 0o666); + mode = validateMode(mode, 'mode', 0o666); const ctx = { path }; const result = binding.open(pathModule.toNamespacedPath(path), @@ -721,7 +721,7 @@ function mkdir(path, mode, callback) { mode = 0o777; } else { callback = makeCallback(callback); - mode = validateAndMaskMode(mode, 'mode', 0o777); + mode = validateMode(mode, 'mode', 0o777); } const req = new FSReqWrap(); @@ -732,7 +732,7 @@ function mkdir(path, mode, callback) { function mkdirSync(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode', 0o777); + mode = validateMode(mode, 'mode', 0o777); const ctx = { path }; binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx); handleErrorFromBinding(ctx); @@ -915,7 +915,7 @@ function unlinkSync(path) { function fchmod(fd, mode, callback) { validateInt32(fd, 'fd', 0); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); callback = makeCallback(callback); const req = new FSReqWrap(); @@ -925,7 +925,7 @@ function fchmod(fd, mode, callback) { function fchmodSync(fd, mode) { validateInt32(fd, 'fd', 0); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); const ctx = {}; binding.fchmod(fd, mode, undefined, ctx); handleErrorFromBinding(ctx); @@ -966,7 +966,7 @@ function lchmodSync(path, mode) { function chmod(path, mode, callback) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); callback = makeCallback(callback); const req = new FSReqWrap(); @@ -977,7 +977,7 @@ function chmod(path, mode, callback) { function chmodSync(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); const ctx = { path }; binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx); diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index ef61880550e315..fb42ebb935a6a7 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -31,8 +31,8 @@ const { validatePath } = require('internal/fs/utils'); const { - validateAndMaskMode, validateInteger, + validateMode, validateUint32 } = require('internal/validators'); const pathModule = require('path'); @@ -191,7 +191,7 @@ async function copyFile(src, dest, flags) { async function open(path, flags, mode) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode', 0o666); + mode = validateMode(mode, 'mode', 0o666); return new FileHandle( await binding.openFileHandle(pathModule.toNamespacedPath(path), stringToFlags(flags), @@ -286,7 +286,7 @@ async function fsync(handle) { async function mkdir(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode', 0o777); + mode = validateMode(mode, 'mode', 0o777); return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises); } @@ -358,14 +358,14 @@ async function unlink(path) { async function fchmod(handle, mode) { validateFileHandle(handle); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); return binding.fchmod(handle.fd, mode, kUsePromises); } async function chmod(path, mode) { path = getPathFromURL(path); validatePath(path); - mode = validateAndMaskMode(mode, 'mode'); + mode = validateMode(mode, 'mode'); return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises); } diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 17b10dab189133..b7b21d29bfa097 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -16,11 +16,22 @@ function isUint32(value) { const octalReg = /^[0-7]+$/; const modeDesc = 'must be a 32-bit unsigned integer or an octal string'; -// Validator for mode_t (the S_* constants). Valid numbers or octal strings -// will be masked with 0o777 to be consistent with the behavior in POSIX APIs. -function validateAndMaskMode(value, name, def) { + +/** + * Validate values that will be converted into mode_t (the S_* constants). + * Only valid numbers and octal strings are allowed. They could be converted + * to 32-bit unsigned integers or non-negative signed integers in the C++ + * land, but any value higher than 0o777 will result in platform-specific + * behaviors. + * + * @param {*} value Values to be validated + * @param {string} name Name of the argument + * @param {number} def If specified, will be returned for invalid values + * @returns {number} + */ +function validateMode(value, name, def) { if (isUint32(value)) { - return value & 0o777; + return value; } if (typeof value === 'number') { @@ -37,7 +48,7 @@ function validateAndMaskMode(value, name, def) { throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc); } const parsed = parseInt(value, 8); - return parsed & 0o777; + return parsed; } // TODO(BridgeAR): Only return `def` in case `value == null` @@ -106,7 +117,7 @@ function validateUint32(value, name, positive) { module.exports = { isInt32, isUint32, - validateAndMaskMode, + validateMode, validateInteger, validateInt32, validateUint32 diff --git a/test/parallel/test-fs-chmod-mask.js b/test/parallel/test-fs-chmod-mask.js index 5f3a8d5ab82864..9564ca142bd643 100644 --- a/test/parallel/test-fs-chmod-mask.js +++ b/test/parallel/test-fs-chmod-mask.js @@ -1,6 +1,6 @@ 'use strict'; -// This tests that mode > 0o777 will be masked off with 0o777 in fs APIs. +// This tests that the lower bits of mode > 0o777 still works in fs APIs. const common = require('../common'); const assert = require('assert'); diff --git a/test/parallel/test-fs-mkdir-mode-mask.js b/test/parallel/test-fs-mkdir-mode-mask.js index e4e8a423483376..515b982054b82b 100644 --- a/test/parallel/test-fs-mkdir-mode-mask.js +++ b/test/parallel/test-fs-mkdir-mode-mask.js @@ -1,6 +1,6 @@ 'use strict'; -// This tests that mode > 0o777 will be masked off with 0o777 in fs.mkdir(). +// This tests that the lower bits of mode > 0o777 still works in fs.mkdir(). const common = require('../common'); const assert = require('assert'); diff --git a/test/parallel/test-fs-open-mode-mask.js b/test/parallel/test-fs-open-mode-mask.js index 4db41864af099c..0cd9a2c5923a71 100644 --- a/test/parallel/test-fs-open-mode-mask.js +++ b/test/parallel/test-fs-open-mode-mask.js @@ -1,6 +1,6 @@ 'use strict'; -// This tests that mode > 0o777 will be masked off with 0o777 in fs.open(). +// This tests that the lower bits of mode > 0o777 still works in fs.open(). const common = require('../common'); const assert = require('assert'); diff --git a/test/parallel/test-fs-promises.js b/test/parallel/test-fs-promises.js index 32439ce0c48cac..6871a6762b68e2 100644 --- a/test/parallel/test-fs-promises.js +++ b/test/parallel/test-fs-promises.js @@ -107,9 +107,8 @@ function verifyStatObject(stat) { await chmod(dest, 0o666); await handle.chmod(0o666); - // Mode larger than 0o777 should be masked off. - await chmod(dest, (0o777 + 1)); - await handle.chmod(0o777 + 1); + await chmod(dest, (0o10777)); + await handle.chmod(0o10777); await utimes(dest, new Date(), new Date()); diff --git a/test/parallel/test-process-umask-mask.js b/test/parallel/test-process-umask-mask.js index c312c061d2b76a..8ec8fc0074ac1b 100644 --- a/test/parallel/test-process-umask-mask.js +++ b/test/parallel/test-process-umask-mask.js @@ -1,6 +1,6 @@ 'use strict'; -// This tests that mask > 0o777 will be masked off with 0o777 in +// This tests that the lower bits of mode > 0o777 still works in // process.umask() const common = require('../common'); From c09bfd81b7e9a6f8fc0909e2eca5c1b35152811e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 27 May 2018 06:07:29 +0800 Subject: [PATCH 015/150] fs: do not crash when using a closed fs event watcher Before this commit, when the user calls methods on a closed or errored fs event watcher, they could hit a crash since the FSEventWrap in C++ land may have already been destroyed with the internal pointer set to nullptr. This commit makes sure that the user cannot hit crashes like that, instead the methods calling on a closed watcher will be noops. Also explicitly documents that the watchers should not be used in `close` and `error` event handlers. PR-URL: https://github.com/nodejs/node/pull/20985 Fixes: https://github.com/nodejs/node/issues/20738 Fixes: https://github.com/nodejs/node/issues/20297 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Ruben Bridgewater Reviewed-By: Ron Korving Reviewed-By: Sakthipriyan Vairamani Reviewed-By: James M Snell Reviewed-By: Tiancheng "Timothy" Gu Backport-PR-URL: https://github.com/nodejs/node/pull/21172 --- doc/api/fs.md | 6 ++- lib/internal/fs/watchers.js | 21 ++++++++-- .../test-fs-watch-close-when-destroyed.js | 38 +++++++++++++++++++ test/parallel/test-fs-watch.js | 19 ++++++++-- 4 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-fs-watch-close-when-destroyed.js diff --git a/doc/api/fs.md b/doc/api/fs.md index e492660bae2f2a..02bf4abdf8b13b 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -325,7 +325,8 @@ fs.watch('./tmp', { encoding: 'buffer' }, (eventType, filename) => { added: v10.0.0 --> -Emitted when the watcher stops watching for changes. +Emitted when the watcher stops watching for changes. The closed +`fs.FSWatcher` object is no longer usable in the event handler. ### Event: 'error' + +> Stability: 1 - Experimental + +## Class: MessageChannel + + +Instances of the `worker.MessageChannel` class represent an asynchronous, +two-way communications channel. +The `MessageChannel` has no methods of its own. `new MessageChannel()` +yields an object with `port1` and `port2` properties, which refer to linked +[`MessagePort`][] instances. + +```js +const { MessageChannel } = require('worker'); + +const { port1, port2 } = new MessageChannel(); +port1.on('message', (message) => console.log('received', message)); +port2.postMessage({ foo: 'bar' }); +// prints: received { foo: 'bar' } +``` + +## Class: MessagePort + + +* Extends: {EventEmitter} + +Instances of the `worker.MessagePort` class represent one end of an +asynchronous, two-way communications channel. It can be used to transfer +structured data, memory regions and other `MessagePort`s between different +[`Worker`][]s. + +With the exception of `MessagePort`s being [`EventEmitter`][]s rather +than `EventTarget`s, this implementation matches [browser `MessagePort`][]s. + +### Event: 'close' + + +The `'close'` event is emitted once either side of the channel has been +disconnected. + +### Event: 'message' + + +* `value` {any} The transmitted value + +The `'message'` event is emitted for any incoming message, containing the cloned +input of [`port.postMessage()`][]. + +Listeners on this event will receive a clone of the `value` parameter as passed +to `postMessage()` and no further arguments. + +### port.close() + + +Disables further sending of messages on either side of the connection. +This method can be called once you know that no further communication +will happen over this `MessagePort`. + +### port.postMessage(value[, transferList]) + + +* `value` {any} +* `transferList` {Object[]} + +Sends a JavaScript value to the receiving side of this channel. +`value` will be transferred in a way which is compatible with +the [HTML structured clone algorithm][]. In particular, it may contain circular +references and objects like typed arrays that the `JSON` API is not able +to stringify. + +`transferList` may be a list of `ArrayBuffer` objects. +After transferring, they will not be usable on the sending side of the channel +anymore (even if they are not contained in `value`). + +`value` may still contain `ArrayBuffer` instances that are not in +`transferList`; in that case, the underlying memory is copied rather than moved. + +For more information on the serialization and deserialization mechanisms +behind this API, see the [serialization API of the `v8` module][v8.serdes]. + +Because the object cloning uses the structured clone algorithm, +non-enumerable properties, property accessors, and object prototypes are +not preserved. In particular, [`Buffer`][] objects will be read as +plain [`Uint8Array`][]s on the receiving side. + +The message object will be cloned immediately, and can be modified after +posting without having side effects. + +### port.ref() + + +Opposite of `unref()`. Calling `ref()` on a previously `unref()`ed port will +*not* let the program exit if it's the only active handle left (the default +behavior). If the port is `ref()`ed, calling `ref()` again will have no effect. + +If listeners are attached or removed using `.on('message')`, the port will +be `ref()`ed and `unref()`ed automatically depending on whether +listeners for the event exist. + +### port.start() + + +Starts receiving messages on this `MessagePort`. When using this port +as an event emitter, this will be called automatically once `'message'` +listeners are attached. + +### port.unref() + + +Calling `unref()` on a port will allow the thread to exit if this is the only +active handle in the event system. If the port is already `unref()`ed calling +`unref()` again will have no effect. + +If listeners are attached or removed using `.on('message')`, the port will +be `ref()`ed and `unref()`ed automatically depending on whether +listeners for the event exist. + +[`Buffer`]: buffer.html +[`EventEmitter`]: events.html +[`MessagePort`]: #worker_class_messageport +[`port.postMessage()`]: #worker_port_postmessage_value_transferlist +[v8.serdes]: v8.html#v8_serialization_api +[`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array +[browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort +[HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index ff809a91291bee..417e8594e14aab 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -194,7 +194,8 @@ }; NativeModule.isInternal = function(id) { - return id.startsWith('internal/'); + return id.startsWith('internal/') || + (id === 'worker' && !process.binding('config').experimentalWorker); }; } diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 60346c5841c7df..55eaed7d376506 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -105,6 +105,11 @@ const builtinLibs = [ 'v8', 'vm', 'zlib' ]; +if (process.binding('config').experimentalWorker) { + builtinLibs.push('worker'); + builtinLibs.sort(); +} + if (typeof process.binding('inspector').open === 'function') { builtinLibs.push('inspector'); builtinLibs.sort(); diff --git a/lib/internal/worker.js b/lib/internal/worker.js new file mode 100644 index 00000000000000..73f7525aa73cc2 --- /dev/null +++ b/lib/internal/worker.js @@ -0,0 +1,105 @@ +'use strict'; + +const EventEmitter = require('events'); +const util = require('util'); + +const { internalBinding } = require('internal/bootstrap/loaders'); +const { MessagePort, MessageChannel } = internalBinding('messaging'); +const { handle_onclose } = internalBinding('symbols'); + +util.inherits(MessagePort, EventEmitter); + +const kOnMessageListener = Symbol('kOnMessageListener'); + +const debug = util.debuglog('worker'); + +// A MessagePort consists of a handle (that wraps around an +// uv_async_t) which can receive information from other threads and emits +// .onmessage events, and a function used for sending data to a MessagePort +// in some other thread. +MessagePort.prototype[kOnMessageListener] = function onmessage(payload) { + debug('received message', payload); + // Emit the deserialized object to userland. + this.emit('message', payload); +}; + +// This is for compatibility with the Web's MessagePort API. It makes sense to +// provide it as an `EventEmitter` in Node.js, but if somebody overrides +// `onmessage`, we'll switch over to the Web API model. +Object.defineProperty(MessagePort.prototype, 'onmessage', { + enumerable: true, + configurable: true, + get() { + return this[kOnMessageListener]; + }, + set(value) { + this[kOnMessageListener] = value; + if (typeof value === 'function') { + this.ref(); + this.start(); + } else { + this.unref(); + this.stop(); + } + } +}); + +// This is called from inside the `MessagePort` constructor. +function oninit() { + setupPortReferencing(this, this, 'message'); +} + +Object.defineProperty(MessagePort.prototype, 'oninit', { + enumerable: true, + writable: false, + value: oninit +}); + +// This is called after the underlying `uv_async_t` has been closed. +function onclose() { + if (typeof this.onclose === 'function') { + // Not part of the Web standard yet, but there aren't many reasonable + // alternatives in a non-EventEmitter usage setting. + // Refs: https://github.com/whatwg/html/issues/1766 + this.onclose(); + } + this.emit('close'); +} + +Object.defineProperty(MessagePort.prototype, handle_onclose, { + enumerable: false, + writable: false, + value: onclose +}); + +const originalClose = MessagePort.prototype.close; +MessagePort.prototype.close = function(cb) { + if (typeof cb === 'function') + this.once('close', cb); + originalClose.call(this); +}; + +function setupPortReferencing(port, eventEmitter, eventName) { + // Keep track of whether there are any workerMessage listeners: + // If there are some, ref() the channel so it keeps the event loop alive. + // If there are none or all are removed, unref() the channel so the worker + // can shutdown gracefully. + port.unref(); + eventEmitter.on('newListener', (name) => { + if (name === eventName && eventEmitter.listenerCount(eventName) === 0) { + port.ref(); + port.start(); + } + }); + eventEmitter.on('removeListener', (name) => { + if (name === eventName && eventEmitter.listenerCount(eventName) === 0) { + port.stop(); + port.unref(); + } + }); +} + +module.exports = { + MessagePort, + MessageChannel +}; diff --git a/lib/worker.js b/lib/worker.js new file mode 100644 index 00000000000000..d67fb4efe40a33 --- /dev/null +++ b/lib/worker.js @@ -0,0 +1,5 @@ +'use strict'; + +const { MessagePort, MessageChannel } = require('internal/worker'); + +module.exports = { MessagePort, MessageChannel }; diff --git a/node.gyp b/node.gyp index 709ce226033af0..72a5d05c43d035 100644 --- a/node.gyp +++ b/node.gyp @@ -78,6 +78,7 @@ 'lib/util.js', 'lib/v8.js', 'lib/vm.js', + 'lib/worker.js', 'lib/zlib.js', 'lib/internal/assert.js', 'lib/internal/async_hooks.js', @@ -155,6 +156,7 @@ 'lib/internal/validators.js', 'lib/internal/stream_base_commons.js', 'lib/internal/vm/module.js', + 'lib/internal/worker.js', 'lib/internal/streams/lazy_transform.js', 'lib/internal/streams/async_iterator.js', 'lib/internal/streams/buffer_list.js', @@ -333,6 +335,7 @@ 'src/node_file.cc', 'src/node_http2.cc', 'src/node_http_parser.cc', + 'src/node_messaging.cc', 'src/node_os.cc', 'src/node_platform.cc', 'src/node_perf.cc', @@ -390,6 +393,7 @@ 'src/node_http2_state.h', 'src/node_internals.h', 'src/node_javascript.h', + 'src/node_messaging.h', 'src/node_mutex.h', 'src/node_perf.h', 'src/node_perf_common.h', diff --git a/src/async_wrap.h b/src/async_wrap.h index 377702a8d6ef9c..cf269a4c1f5e1e 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -49,6 +49,7 @@ namespace node { V(HTTP2SETTINGS) \ V(HTTPPARSER) \ V(JSSTREAM) \ + V(MESSAGEPORT) \ V(PIPECONNECTWRAP) \ V(PIPESERVERWRAP) \ V(PIPEWRAP) \ diff --git a/src/env.h b/src/env.h index 7a432eaa3d4ff0..d87c39c5186bd7 100644 --- a/src/env.h +++ b/src/env.h @@ -193,6 +193,7 @@ struct PackageConfig { V(main_string, "main") \ V(max_buffer_string, "maxBuffer") \ V(message_string, "message") \ + V(message_port_constructor_string, "MessagePort") \ V(minttl_string, "minttl") \ V(modulus_string, "modulus") \ V(name_string, "name") \ @@ -212,6 +213,7 @@ struct PackageConfig { V(onhandshakedone_string, "onhandshakedone") \ V(onhandshakestart_string, "onhandshakestart") \ V(onheaders_string, "onheaders") \ + V(oninit_string, "oninit") \ V(onmessage_string, "onmessage") \ V(onnewsession_string, "onnewsession") \ V(onocspresponse_string, "onocspresponse") \ @@ -242,6 +244,8 @@ struct PackageConfig { V(pipe_target_string, "pipeTarget") \ V(pipe_source_string, "pipeSource") \ V(port_string, "port") \ + V(port1_string, "port1") \ + V(port2_string, "port2") \ V(preference_string, "preference") \ V(priority_string, "priority") \ V(promise_string, "promise") \ @@ -323,6 +327,7 @@ struct PackageConfig { V(http2stream_constructor_template, v8::ObjectTemplate) \ V(immediate_callback_function, v8::Function) \ V(inspector_console_api_object, v8::Object) \ + V(message_port_constructor_template, v8::FunctionTemplate) \ V(pbkdf2_constructor_template, v8::ObjectTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ V(performance_entry_callback, v8::Function) \ diff --git a/src/node.cc b/src/node.cc index 4973fea636afd5..281d441a26a86c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -248,6 +248,11 @@ bool config_experimental_modules = false; // that is used by lib/vm.js bool config_experimental_vm_modules = false; +// Set in node.cc by ParseArgs when --experimental-worker is used. +// Used in node_config.cc to set a constant on process.binding('config') +// that is used by lib/worker.js +bool config_experimental_worker = false; + // Set in node.cc by ParseArgs when --experimental-repl-await is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/repl.js. @@ -3104,6 +3109,7 @@ static void PrintHelp() { " --experimental-vm-modules experimental ES Module support\n" " in vm module\n" #endif // defined(NODE_HAVE_I18N_SUPPORT) + " --experimental-worker experimental threaded Worker support\n" #if HAVE_OPENSSL && NODE_FIPS_MODE " --force-fips force FIPS crypto (cannot be disabled)\n" #endif // HAVE_OPENSSL && NODE_FIPS_MODE @@ -3267,6 +3273,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env, "--experimental-modules", "--experimental-repl-await", "--experimental-vm-modules", + "--experimental-worker", "--expose-http2", // keep as a non-op through v9.x "--force-fips", "--icu-data-dir", @@ -3465,6 +3472,8 @@ static void ParseArgs(int* argc, new_v8_argc += 1; } else if (strcmp(arg, "--experimental-vm-modules") == 0) { config_experimental_vm_modules = true; + } else if (strcmp(arg, "--experimental-worker") == 0) { + config_experimental_worker = true; } else if (strcmp(arg, "--experimental-repl-await") == 0) { config_experimental_repl_await = true; } else if (strcmp(arg, "--loader") == 0) { diff --git a/src/node_config.cc b/src/node_config.cc index 603d55491a259b..dd5ee666486874 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -91,6 +91,9 @@ static void Initialize(Local target, if (config_experimental_vm_modules) READONLY_BOOLEAN_PROPERTY("experimentalVMModules"); + if (config_experimental_worker) + READONLY_BOOLEAN_PROPERTY("experimentalWorker"); + if (config_experimental_repl_await) READONLY_BOOLEAN_PROPERTY("experimentalREPLAwait"); diff --git a/src/node_errors.h b/src/node_errors.h index ab02421df8b36a..d119f4f7060853 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -19,9 +19,12 @@ namespace node { #define ERRORS_WITH_CODE(V) \ V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \ V(ERR_BUFFER_TOO_LARGE, Error) \ + V(ERR_CLOSED_MESSAGE_PORT, Error) \ + V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \ V(ERR_INDEX_OUT_OF_RANGE, RangeError) \ V(ERR_INVALID_ARG_VALUE, TypeError) \ V(ERR_INVALID_ARG_TYPE, TypeError) \ + V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \ V(ERR_MISSING_MODULE, Error) \ @@ -48,7 +51,10 @@ namespace node { // Errors with predefined static messages #define PREDEFINED_ERROR_MESSAGES(V) \ + V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort") \ + V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \ V(ERR_INDEX_OUT_OF_RANGE, "Index out of range") \ + V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \ V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") #define V(code, message) \ diff --git a/src/node_internals.h b/src/node_internals.h index 7bf4eaf1ecf86a..a5d8ed0e5d3ad7 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -114,6 +114,7 @@ struct sockaddr; V(http_parser) \ V(inspector) \ V(js_stream) \ + V(messaging) \ V(module_wrap) \ V(os) \ V(performance) \ @@ -189,6 +190,11 @@ extern bool config_experimental_modules; // that is used by lib/vm.js extern bool config_experimental_vm_modules; +// Set in node.cc by ParseArgs when --experimental-vm-modules is used. +// Used in node_config.cc to set a constant on process.binding('config') +// that is used by lib/vm.js +extern bool config_experimental_worker; + // Set in node.cc by ParseArgs when --experimental-repl-await is used. // Used in node_config.cc to set a constant on process.binding('config') // that is used by lib/repl.js. diff --git a/src/node_messaging.cc b/src/node_messaging.cc new file mode 100644 index 00000000000000..c6e701c7d94426 --- /dev/null +++ b/src/node_messaging.cc @@ -0,0 +1,548 @@ +#include "node_messaging.h" +#include "node_internals.h" +#include "node_buffer.h" +#include "node_errors.h" +#include "util.h" +#include "util-inl.h" +#include "async_wrap.h" +#include "async_wrap-inl.h" + +using v8::Array; +using v8::ArrayBuffer; +using v8::ArrayBufferCreationMode; +using v8::Context; +using v8::EscapableHandleScope; +using v8::Exception; +using v8::Function; +using v8::FunctionCallbackInfo; +using v8::FunctionTemplate; +using v8::HandleScope; +using v8::Isolate; +using v8::Just; +using v8::Local; +using v8::Maybe; +using v8::MaybeLocal; +using v8::Nothing; +using v8::Object; +using v8::String; +using v8::Value; +using v8::ValueDeserializer; +using v8::ValueSerializer; + +namespace node { +namespace worker { + +Message::Message(MallocedBuffer&& buffer) + : main_message_buf_(std::move(buffer)) {} + +namespace { + +// This is used to tell V8 how to read transferred host objects, like other +// `MessagePort`s and `SharedArrayBuffer`s, and make new JS objects out of them. +class DeserializerDelegate : public ValueDeserializer::Delegate { + public: + DeserializerDelegate(Message* m, Environment* env) + : env_(env), msg_(m) {} + + ValueDeserializer* deserializer = nullptr; + + private: + Environment* env_; + Message* msg_; +}; + +} // anonymous namespace + +MaybeLocal Message::Deserialize(Environment* env, + Local context) { + EscapableHandleScope handle_scope(env->isolate()); + Context::Scope context_scope(context); + + DeserializerDelegate delegate(this, env); + ValueDeserializer deserializer( + env->isolate(), + reinterpret_cast(main_message_buf_.data), + main_message_buf_.size, + &delegate); + delegate.deserializer = &deserializer; + + // Attach all transfered ArrayBuffers to their new Isolate. + for (uint32_t i = 0; i < array_buffer_contents_.size(); ++i) { + Local ab = + ArrayBuffer::New(env->isolate(), + array_buffer_contents_[i].release(), + array_buffer_contents_[i].size, + ArrayBufferCreationMode::kInternalized); + deserializer.TransferArrayBuffer(i, ab); + } + array_buffer_contents_.clear(); + + if (deserializer.ReadHeader(context).IsNothing()) + return MaybeLocal(); + return handle_scope.Escape( + deserializer.ReadValue(context).FromMaybe(Local())); +} + +namespace { + +// This tells V8 how to serialize objects that it does not understand +// (e.g. C++ objects) into the output buffer, in a way that our own +// DeserializerDelegate understands how to unpack. +class SerializerDelegate : public ValueSerializer::Delegate { + public: + SerializerDelegate(Environment* env, Local context, Message* m) + : env_(env), context_(context), msg_(m) {} + + void ThrowDataCloneError(Local message) override { + env_->isolate()->ThrowException(Exception::Error(message)); + } + + ValueSerializer* serializer = nullptr; + + private: + Environment* env_; + Local context_; + Message* msg_; + + friend class worker::Message; +}; + +} // anynomous namespace + +Maybe Message::Serialize(Environment* env, + Local context, + Local input, + Local transfer_list_v) { + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(context); + + // Verify that we're not silently overwriting an existing message. + CHECK(main_message_buf_.is_empty()); + + SerializerDelegate delegate(env, context, this); + ValueSerializer serializer(env->isolate(), &delegate); + delegate.serializer = &serializer; + + std::vector> array_buffers; + if (transfer_list_v->IsArray()) { + Local transfer_list = transfer_list_v.As(); + uint32_t length = transfer_list->Length(); + for (uint32_t i = 0; i < length; ++i) { + Local entry; + if (!transfer_list->Get(context, i).ToLocal(&entry)) + return Nothing(); + // Currently, we support ArrayBuffers. + if (entry->IsArrayBuffer()) { + Local ab = entry.As(); + // If we cannot render the ArrayBuffer unusable in this Isolate and + // take ownership of its memory, copying the buffer will have to do. + if (!ab->IsNeuterable() || ab->IsExternal()) + continue; + // We simply use the array index in the `array_buffers` list as the + // ID that we write into the serialized buffer. + uint32_t id = array_buffers.size(); + array_buffers.push_back(ab); + serializer.TransferArrayBuffer(id, ab); + continue; + } + + THROW_ERR_INVALID_TRANSFER_OBJECT(env); + return Nothing(); + } + } + + serializer.WriteHeader(); + if (serializer.WriteValue(context, input).IsNothing()) { + return Nothing(); + } + + for (Local ab : array_buffers) { + // If serialization succeeded, we want to take ownership of + // (a.k.a. externalize) the underlying memory region and render + // it inaccessible in this Isolate. + ArrayBuffer::Contents contents = ab->Externalize(); + ab->Neuter(); + array_buffer_contents_.push_back( + MallocedBuffer { static_cast(contents.Data()), + contents.ByteLength() }); + } + + // The serializer gave us a buffer allocated using `malloc()`. + std::pair data = serializer.Release(); + main_message_buf_ = + MallocedBuffer(reinterpret_cast(data.first), data.second); + return Just(true); +} + +MessagePortData::MessagePortData(MessagePort* owner) : owner_(owner) { } + +MessagePortData::~MessagePortData() { + CHECK_EQ(owner_, nullptr); + Disentangle(); +} + +void MessagePortData::AddToIncomingQueue(Message&& message) { + // This function will be called by other threads. + Mutex::ScopedLock lock(mutex_); + incoming_messages_.emplace_back(std::move(message)); + + if (owner_ != nullptr) + owner_->TriggerAsync(); +} + +bool MessagePortData::IsSiblingClosed() const { + Mutex::ScopedLock lock(*sibling_mutex_); + return sibling_ == nullptr; +} + +void MessagePortData::Entangle(MessagePortData* a, MessagePortData* b) { + CHECK_EQ(a->sibling_, nullptr); + CHECK_EQ(b->sibling_, nullptr); + a->sibling_ = b; + b->sibling_ = a; + a->sibling_mutex_ = b->sibling_mutex_; +} + +void MessagePortData::PingOwnerAfterDisentanglement() { + Mutex::ScopedLock lock(mutex_); + if (owner_ != nullptr) + owner_->TriggerAsync(); +} + +void MessagePortData::Disentangle() { + // Grab a copy of the sibling mutex, then replace it so that each sibling + // has its own sibling_mutex_ now. + std::shared_ptr sibling_mutex = sibling_mutex_; + Mutex::ScopedLock sibling_lock(*sibling_mutex); + sibling_mutex_ = std::make_shared(); + + MessagePortData* sibling = sibling_; + if (sibling_ != nullptr) { + sibling_->sibling_ = nullptr; + sibling_ = nullptr; + } + + // We close MessagePorts after disentanglement, so we trigger the + // corresponding uv_async_t to let them know that this happened. + PingOwnerAfterDisentanglement(); + if (sibling != nullptr) { + sibling->PingOwnerAfterDisentanglement(); + } +} + +MessagePort::~MessagePort() { + if (data_) + data_->owner_ = nullptr; +} + +MessagePort::MessagePort(Environment* env, + Local context, + Local wrap) + : HandleWrap(env, + wrap, + reinterpret_cast(new uv_async_t()), + AsyncWrap::PROVIDER_MESSAGEPORT), + data_(new MessagePortData(this)) { + auto onmessage = [](uv_async_t* handle) { + // Called when data has been put into the queue. + MessagePort* channel = static_cast(handle->data); + channel->OnMessage(); + }; + CHECK_EQ(uv_async_init(env->event_loop(), + async(), + onmessage), 0); + async()->data = static_cast(this); + + Local fn; + if (!wrap->Get(context, env->oninit_string()).ToLocal(&fn)) + return; + + if (fn->IsFunction()) { + Local init = fn.As(); + USE(init->Call(context, wrap, 0, nullptr)); + } +} + +void MessagePort::AddToIncomingQueue(Message&& message) { + data_->AddToIncomingQueue(std::move(message)); +} + +uv_async_t* MessagePort::async() { + return reinterpret_cast(GetHandle()); +} + +void MessagePort::TriggerAsync() { + CHECK_EQ(uv_async_send(async()), 0); +} + +void MessagePort::New(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!args.IsConstructCall()) { + THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); + return; + } + + Local context = args.This()->CreationContext(); + Context::Scope context_scope(context); + + new MessagePort(env, context, args.This()); +} + +MessagePort* MessagePort::New( + Environment* env, + Local context, + std::unique_ptr data) { + Context::Scope context_scope(context); + Local ctor; + if (!GetMessagePortConstructor(env, context).ToLocal(&ctor)) + return nullptr; + MessagePort* port = nullptr; + + // Construct a new instance, then assign the listener instance and possibly + // the MessagePortData to it. + Local instance; + if (!ctor->NewInstance(context).ToLocal(&instance)) + return nullptr; + ASSIGN_OR_RETURN_UNWRAP(&port, instance, nullptr); + if (data) { + port->Detach(); + port->data_ = std::move(data); + port->data_->owner_ = port; + // If the existing MessagePortData object had pending messages, this is + // the easiest way to run that queue. + port->TriggerAsync(); + } + return port; +} + +void MessagePort::OnMessage() { + HandleScope handle_scope(env()->isolate()); + Local context = object()->CreationContext(); + + // data_ can only ever be modified by the owner thread, so no need to lock. + // However, the message port may be transferred while it is processing + // messages, so we need to check that this handle still owns its `data_` field + // on every iteration. + while (data_) { + Message received; + { + // Get the head of the message queue. + Mutex::ScopedLock lock(data_->mutex_); + if (!data_->receiving_messages_) + break; + if (data_->incoming_messages_.empty()) + break; + received = std::move(data_->incoming_messages_.front()); + data_->incoming_messages_.pop_front(); + } + + if (!env()->can_call_into_js()) { + // In this case there is nothing to do but to drain the current queue. + continue; + } + + { + // Call the JS .onmessage() callback. + HandleScope handle_scope(env()->isolate()); + Context::Scope context_scope(context); + Local args[] = { + received.Deserialize(env(), context).FromMaybe(Local()) + }; + + if (args[0].IsEmpty() || + !object()->Has(context, env()->onmessage_string()).FromMaybe(false) || + MakeCallback(env()->onmessage_string(), 1, args).IsEmpty()) { + // Re-schedule OnMessage() execution in case of failure. + if (data_) + TriggerAsync(); + return; + } + } + } + + if (data_ && data_->IsSiblingClosed()) { + Close(); + } +} + +bool MessagePort::IsSiblingClosed() const { + CHECK(data_); + return data_->IsSiblingClosed(); +} + +void MessagePort::OnClose() { + if (data_) { + data_->owner_ = nullptr; + data_->Disentangle(); + } + data_.reset(); + delete async(); +} + +std::unique_ptr MessagePort::Detach() { + Mutex::ScopedLock lock(data_->mutex_); + data_->owner_ = nullptr; + return std::move(data_); +} + + +void MessagePort::Send(Message&& message) { + Mutex::ScopedLock lock(*data_->sibling_mutex_); + if (data_->sibling_ == nullptr) + return; + data_->sibling_->AddToIncomingQueue(std::move(message)); +} + +void MessagePort::Send(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Message msg; + if (msg.Serialize(env, object()->CreationContext(), args[0], args[1]) + .IsNothing()) { + return; + } + Send(std::move(msg)); +} + +void MessagePort::PostMessage(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + MessagePort* port; + ASSIGN_OR_RETURN_UNWRAP(&port, args.This()); + if (!port->data_) { + return THROW_ERR_CLOSED_MESSAGE_PORT(env); + } + if (args.Length() == 0) { + return THROW_ERR_MISSING_ARGS(env, "Not enough arguments to " + "MessagePort.postMessage"); + } + port->Send(args); +} + +void MessagePort::Start() { + Mutex::ScopedLock lock(data_->mutex_); + data_->receiving_messages_ = true; + if (!data_->incoming_messages_.empty()) + TriggerAsync(); +} + +void MessagePort::Stop() { + Mutex::ScopedLock lock(data_->mutex_); + data_->receiving_messages_ = false; +} + +void MessagePort::Start(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + MessagePort* port; + ASSIGN_OR_RETURN_UNWRAP(&port, args.This()); + if (!port->data_) { + THROW_ERR_CLOSED_MESSAGE_PORT(env); + return; + } + port->Start(); +} + +void MessagePort::Stop(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + MessagePort* port; + ASSIGN_OR_RETURN_UNWRAP(&port, args.This()); + if (!port->data_) { + THROW_ERR_CLOSED_MESSAGE_PORT(env); + return; + } + port->Stop(); +} + +size_t MessagePort::self_size() const { + Mutex::ScopedLock lock(data_->mutex_); + size_t sz = sizeof(*this) + sizeof(*data_); + for (const Message& msg : data_->incoming_messages_) + sz += sizeof(msg) + msg.main_message_buf_.size; + return sz; +} + +void MessagePort::Entangle(MessagePort* a, MessagePort* b) { + Entangle(a, b->data_.get()); +} + +void MessagePort::Entangle(MessagePort* a, MessagePortData* b) { + MessagePortData::Entangle(a->data_.get(), b); +} + +MaybeLocal GetMessagePortConstructor( + Environment* env, Local context) { + // Factor generating the MessagePort JS constructor into its own piece + // of code, because it is needed early on in the child environment setup. + Local templ = env->message_port_constructor_template(); + if (!templ.IsEmpty()) + return templ->GetFunction(context); + + { + Local m = env->NewFunctionTemplate(MessagePort::New); + m->SetClassName(env->message_port_constructor_string()); + m->InstanceTemplate()->SetInternalFieldCount(1); + + AsyncWrap::AddWrapMethods(env, m); + + env->SetProtoMethod(m, "postMessage", MessagePort::PostMessage); + env->SetProtoMethod(m, "start", MessagePort::Start); + env->SetProtoMethod(m, "stop", MessagePort::Stop); + env->SetProtoMethod(m, "close", HandleWrap::Close); + env->SetProtoMethod(m, "unref", HandleWrap::Unref); + env->SetProtoMethod(m, "ref", HandleWrap::Ref); + env->SetProtoMethod(m, "hasRef", HandleWrap::HasRef); + + env->set_message_port_constructor_template(m); + } + + return GetMessagePortConstructor(env, context); +} + +namespace { + +static void MessageChannel(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!args.IsConstructCall()) { + THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); + return; + } + + Local context = args.This()->CreationContext(); + Context::Scope context_scope(context); + + MessagePort* port1 = MessagePort::New(env, context); + MessagePort* port2 = MessagePort::New(env, context); + MessagePort::Entangle(port1, port2); + + args.This()->Set(env->context(), env->port1_string(), port1->object()) + .FromJust(); + args.This()->Set(env->context(), env->port2_string(), port2->object()) + .FromJust(); +} + +static void InitMessaging(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + + { + Local message_channel_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "MessageChannel"); + Local templ = env->NewFunctionTemplate(MessageChannel); + templ->SetClassName(message_channel_string); + target->Set(env->context(), + message_channel_string, + templ->GetFunction(context).ToLocalChecked()).FromJust(); + } + + target->Set(context, + env->message_port_constructor_string(), + GetMessagePortConstructor(env, context).ToLocalChecked()) + .FromJust(); +} + +} // anonymous namespace + +} // namespace worker +} // namespace node + +NODE_MODULE_CONTEXT_AWARE_INTERNAL(messaging, node::worker::InitMessaging) diff --git a/src/node_messaging.h b/src/node_messaging.h new file mode 100644 index 00000000000000..7bd60163ea167c --- /dev/null +++ b/src/node_messaging.h @@ -0,0 +1,167 @@ +#ifndef SRC_NODE_MESSAGING_H_ +#define SRC_NODE_MESSAGING_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "env.h" +#include "node_mutex.h" +#include +#include + +namespace node { +namespace worker { + +class MessagePortData; +class MessagePort; + +// Represents a single communication message. +class Message { + public: + explicit Message(MallocedBuffer&& payload = MallocedBuffer()); + + Message(Message&& other) = default; + Message& operator=(Message&& other) = default; + Message& operator=(const Message&) = delete; + Message(const Message&) = delete; + + // Deserialize the contained JS value. May only be called once, and only + // after Serialize() has been called (e.g. by another thread). + v8::MaybeLocal Deserialize(Environment* env, + v8::Local context); + + // Serialize a JS value, and optionally transfer objects, into this message. + // The Message object retains ownership of all transferred objects until + // deserialization. + v8::Maybe Serialize(Environment* env, + v8::Local context, + v8::Local input, + v8::Local transfer_list); + + private: + MallocedBuffer main_message_buf_; + std::vector> array_buffer_contents_; + + friend class MessagePort; +}; + +// This contains all data for a `MessagePort` instance that is not tied to +// a specific Environment/Isolate/event loop, for easier transfer between those. +class MessagePortData { + public: + explicit MessagePortData(MessagePort* owner); + ~MessagePortData(); + + MessagePortData(MessagePortData&& other) = delete; + MessagePortData& operator=(MessagePortData&& other) = delete; + MessagePortData(const MessagePortData& other) = delete; + MessagePortData& operator=(const MessagePortData& other) = delete; + + // Add a message to the incoming queue and notify the receiver. + // This may be called from any thread. + void AddToIncomingQueue(Message&& message); + + // Returns true if and only this MessagePort is currently not entangled + // with another message port. + bool IsSiblingClosed() const; + + // Turns `a` and `b` into siblings, i.e. connects the sending side of one + // to the receiving side of the other. This is not thread-safe. + static void Entangle(MessagePortData* a, MessagePortData* b); + + // Removes any possible sibling. This is thread-safe (it acquires both + // `sibling_mutex_` and `mutex_`), and has to be because it is called once + // the corresponding JS handle handle wants to close + // which can happen on either side of a worker. + void Disentangle(); + + private: + // After disentangling this message port, the owner handle (if any) + // is asynchronously triggered, so that it can close down naturally. + void PingOwnerAfterDisentanglement(); + + // This mutex protects all fields below it, with the exception of + // sibling_. + mutable Mutex mutex_; + bool receiving_messages_ = false; + std::list incoming_messages_; + MessagePort* owner_ = nullptr; + // This mutex protects the sibling_ field and is shared between two entangled + // MessagePorts. If both mutexes are acquired, this one needs to be + // acquired first. + std::shared_ptr sibling_mutex_ = std::make_shared(); + MessagePortData* sibling_ = nullptr; + + friend class MessagePort; +}; + +// A message port that receives messages from other threads, including +// the uv_async_t handle that is used to notify the current event loop of +// new incoming messages. +class MessagePort : public HandleWrap { + public: + // Create a new MessagePort. The `context` argument specifies the Context + // instance that is used for creating the values emitted from this port. + MessagePort(Environment* env, + v8::Local context, + v8::Local wrap); + ~MessagePort(); + + // Create a new message port instance, optionally over an existing + // `MessagePortData` object. + static MessagePort* New(Environment* env, + v8::Local context, + std::unique_ptr data = nullptr); + + // Send a message, i.e. deliver it into the sibling's incoming queue. + // If there is no sibling, i.e. this port is closed, + // this message is silently discarded. + void Send(Message&& message); + void Send(const v8::FunctionCallbackInfo& args); + // Deliver a single message into this port's incoming queue. + void AddToIncomingQueue(Message&& message); + + // Start processing messages on this port as a receiving end. + void Start(); + // Stop processing messages on this port as a receiving end. + void Stop(); + + static void New(const v8::FunctionCallbackInfo& args); + static void PostMessage(const v8::FunctionCallbackInfo& args); + static void Start(const v8::FunctionCallbackInfo& args); + static void Stop(const v8::FunctionCallbackInfo& args); + + // Turns `a` and `b` into siblings, i.e. connects the sending side of one + // to the receiving side of the other. This is not thread-safe. + static void Entangle(MessagePort* a, MessagePort* b); + static void Entangle(MessagePort* a, MessagePortData* b); + + // Detach this port's data for transferring. After this, the MessagePortData + // is no longer associated with this handle, although it can still receive + // messages. + std::unique_ptr Detach(); + + bool IsSiblingClosed() const; + + size_t self_size() const override; + + private: + void OnClose() override; + void OnMessage(); + void TriggerAsync(); + inline uv_async_t* async(); + + std::unique_ptr data_ = nullptr; + + friend class MessagePortData; +}; + +v8::MaybeLocal GetMessagePortConstructor( + Environment* env, v8::Local context); + +} // namespace worker +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + + +#endif // SRC_NODE_MESSAGING_H_ diff --git a/src/util.h b/src/util.h index e272286d3e4b96..fade27458f3e16 100644 --- a/src/util.h +++ b/src/util.h @@ -436,8 +436,11 @@ struct MallocedBuffer { return ret; } + inline bool is_empty() const { return data == nullptr; } + MallocedBuffer() : data(nullptr) {} explicit MallocedBuffer(size_t size) : data(Malloc(size)), size(size) {} + MallocedBuffer(char* data, size_t size) : data(data), size(size) {} MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) { other.data = nullptr; } diff --git a/test/parallel/test-message-channel.js b/test/parallel/test-message-channel.js new file mode 100644 index 00000000000000..0facaa1d835ea8 --- /dev/null +++ b/test/parallel/test-message-channel.js @@ -0,0 +1,26 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { MessageChannel } = require('worker'); + +{ + const channel = new MessageChannel(); + + channel.port1.on('message', common.mustCall(({ typedArray }) => { + assert.deepStrictEqual(typedArray, new Uint8Array([0, 1, 2, 3, 4])); + })); + + const typedArray = new Uint8Array([0, 1, 2, 3, 4]); + channel.port2.postMessage({ typedArray }, [ typedArray.buffer ]); + assert.strictEqual(typedArray.buffer.byteLength, 0); + channel.port2.close(); +} + +{ + const channel = new MessageChannel(); + + channel.port1.on('close', common.mustCall()); + channel.port2.on('close', common.mustCall()); + channel.port2.close(); +} diff --git a/test/parallel/test-message-port-arraybuffer.js b/test/parallel/test-message-port-arraybuffer.js new file mode 100644 index 00000000000000..4abeb585b4fb15 --- /dev/null +++ b/test/parallel/test-message-port-arraybuffer.js @@ -0,0 +1,20 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { MessageChannel } = require('worker'); + +{ + const { port1, port2 } = new MessageChannel(); + + const arrayBuffer = new ArrayBuffer(40); + const typedArray = new Uint32Array(arrayBuffer); + typedArray[0] = 0x12345678; + + port1.postMessage(typedArray, [ arrayBuffer ]); + port2.on('message', common.mustCall((received) => { + assert.strictEqual(received[0], 0x12345678); + port2.close(common.mustCall()); + })); +} diff --git a/test/parallel/test-message-port.js b/test/parallel/test-message-port.js new file mode 100644 index 00000000000000..8a7f3805200fa3 --- /dev/null +++ b/test/parallel/test-message-port.js @@ -0,0 +1,56 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { MessageChannel, MessagePort } = require('worker'); + +{ + const { port1, port2 } = new MessageChannel(); + assert(port1 instanceof MessagePort); + assert(port2 instanceof MessagePort); + + const input = { a: 1 }; + port1.postMessage(input); + port2.on('message', common.mustCall((received) => { + assert.deepStrictEqual(received, input); + port2.close(common.mustCall()); + })); +} + +{ + const { port1, port2 } = new MessageChannel(); + + const input = { a: 1 }; + port1.postMessage(input); + // Check that the message still gets delivered if `port2` has its + // `on('message')` handler attached at a later point in time. + setImmediate(() => { + port2.on('message', common.mustCall((received) => { + assert.deepStrictEqual(received, input); + port2.close(common.mustCall()); + })); + }); +} + +{ + const { port1, port2 } = new MessageChannel(); + + const input = { a: 1 }; + + const dummy = common.mustNotCall(); + // Check that the message still gets delivered if `port2` has its + // `on('message')` handler attached at a later point in time, even if a + // listener was removed previously. + port2.addListener('message', dummy); + setImmediate(() => { + port2.removeListener('message', dummy); + port1.postMessage(input); + setImmediate(() => { + port2.on('message', common.mustCall((received) => { + assert.deepStrictEqual(received, input); + port2.close(common.mustCall()); + })); + }); + }); +} diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 971296915ceecb..84a3e3b1f4dc05 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -35,7 +35,9 @@ common.crashOnUnhandledRejection(); delete providers.HTTP2STREAM; delete providers.HTTP2PING; delete providers.HTTP2SETTINGS; + // TODO(addaleax): Test for these delete providers.STREAMPIPE; + delete providers.MESSAGEPORT; const objKeys = Object.keys(providers); if (objKeys.length > 0) diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js index e7c7aa69da5e95..be72893832373a 100644 --- a/tools/doc/type-parser.js +++ b/tools/doc/type-parser.js @@ -117,7 +117,9 @@ const customTypesMap = { 'Tracing': 'tracing.html#tracing_tracing_object', 'URL': 'url.html#url_the_whatwg_url_api', - 'URLSearchParams': 'url.html#url_class_urlsearchparams' + 'URLSearchParams': 'url.html#url_class_urlsearchparams', + + 'MessagePort': 'worker.html#worker_class_messageport' }; const arrayPart = /(?:\[])+$/; From f447acd87b67dc04b6081d212b79f444e001232f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 7 Oct 2017 14:39:02 -0700 Subject: [PATCH 023/150] worker: support MessagePort passing in messages Support passing `MessagePort` instances through other `MessagePort`s, as expected by the `MessagePort` spec. Thanks to Stephen Belanger for reviewing this change in its original PR. Refs: https://github.com/ayojs/ayo/pull/106 PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- doc/api/errors.md | 12 +++ doc/api/worker.md | 2 +- src/node_errors.h | 7 +- src/node_messaging.cc | 80 ++++++++++++++++++- src/node_messaging.h | 5 ++ ...-message-port-message-port-transferring.js | 23 ++++++ 6 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-message-port-message-port-transferring.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 161e5eb9564614..be67744f28799f 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -629,6 +629,12 @@ An operation outside the bounds of a `Buffer` was attempted. An attempt has been made to create a `Buffer` larger than the maximum allowed size. + +### ERR_CANNOT_TRANSFER_OBJECT + +The value passed to `postMessage()` contained an object that is not supported +for transferring. + ### ERR_CANNOT_WATCH_SIGINT @@ -1304,6 +1310,12 @@ strict compliance with the API specification (which in some cases may accept `func(undefined)` and `func()` are treated identically, and the [`ERR_INVALID_ARG_TYPE`][] error code may be used instead. + +### ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST + +A `MessagePort` was found in the object passed to a `postMessage()` call, +but not provided in the `transferList` for that call. + ### ERR_MISSING_MODULE diff --git a/doc/api/worker.md b/doc/api/worker.md index 4724714cd62f26..6a391c5a9e3b19 100644 --- a/doc/api/worker.md +++ b/doc/api/worker.md @@ -83,7 +83,7 @@ the [HTML structured clone algorithm][]. In particular, it may contain circular references and objects like typed arrays that the `JSON` API is not able to stringify. -`transferList` may be a list of `ArrayBuffer` objects. +`transferList` may be a list of `ArrayBuffer` and `MessagePort` objects. After transferring, they will not be usable on the sending side of the channel anymore (even if they are not contained in `value`). diff --git a/src/node_errors.h b/src/node_errors.h index d119f4f7060853..0c4964312e91d3 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -19,6 +19,7 @@ namespace node { #define ERRORS_WITH_CODE(V) \ V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \ V(ERR_BUFFER_TOO_LARGE, Error) \ + V(ERR_CANNOT_TRANSFER_OBJECT, TypeError) \ V(ERR_CLOSED_MESSAGE_PORT, Error) \ V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \ V(ERR_INDEX_OUT_OF_RANGE, RangeError) \ @@ -27,6 +28,7 @@ namespace node { V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MISSING_ARGS, TypeError) \ + V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \ V(ERR_MISSING_MODULE, Error) \ V(ERR_STRING_TOO_LONG, Error) \ @@ -51,11 +53,14 @@ namespace node { // Errors with predefined static messages #define PREDEFINED_ERROR_MESSAGES(V) \ + V(ERR_CANNOT_TRANSFER_OBJECT, "Cannot transfer object of unsupported type")\ V(ERR_CLOSED_MESSAGE_PORT, "Cannot send data on closed MessagePort") \ V(ERR_CONSTRUCT_CALL_REQUIRED, "Cannot call constructor without `new`") \ V(ERR_INDEX_OUT_OF_RANGE, "Index out of range") \ V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList") \ - V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") + V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \ + V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, \ + "MessagePort was found in message but not listed in transferList") #define V(code, message) \ inline v8::Local code(v8::Isolate* isolate) { \ diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c6e701c7d94426..1c6551e0969f3e 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -41,14 +41,27 @@ namespace { // `MessagePort`s and `SharedArrayBuffer`s, and make new JS objects out of them. class DeserializerDelegate : public ValueDeserializer::Delegate { public: - DeserializerDelegate(Message* m, Environment* env) - : env_(env), msg_(m) {} + DeserializerDelegate(Message* m, + Environment* env, + const std::vector& message_ports) + : env_(env), msg_(m), message_ports_(message_ports) {} + + MaybeLocal ReadHostObject(Isolate* isolate) override { + // Currently, only MessagePort hosts objects are supported, so identifying + // by the index in the message's MessagePort array is sufficient. + uint32_t id; + if (!deserializer->ReadUint32(&id)) + return MaybeLocal(); + CHECK_LE(id, message_ports_.size()); + return message_ports_[id]->object(); + }; ValueDeserializer* deserializer = nullptr; private: Environment* env_; Message* msg_; + const std::vector& message_ports_; }; } // anonymous namespace @@ -58,7 +71,23 @@ MaybeLocal Message::Deserialize(Environment* env, EscapableHandleScope handle_scope(env->isolate()); Context::Scope context_scope(context); - DeserializerDelegate delegate(this, env); + // Create all necessary MessagePort handles. + std::vector ports(message_ports_.size()); + for (uint32_t i = 0; i < message_ports_.size(); ++i) { + ports[i] = MessagePort::New(env, + context, + std::move(message_ports_[i])); + if (ports[i] == nullptr) { + for (MessagePort* port : ports) { + // This will eventually release the MessagePort object itself. + port->Close(); + } + return MaybeLocal(); + } + } + message_ports_.clear(); + + DeserializerDelegate delegate(this, env, ports); ValueDeserializer deserializer( env->isolate(), reinterpret_cast(main_message_buf_.data), @@ -83,6 +112,10 @@ MaybeLocal Message::Deserialize(Environment* env, deserializer.ReadValue(context).FromMaybe(Local())); } +void Message::AddMessagePort(std::unique_ptr&& data) { + message_ports_.emplace_back(std::move(data)); +} + namespace { // This tells V8 how to serialize objects that it does not understand @@ -97,12 +130,43 @@ class SerializerDelegate : public ValueSerializer::Delegate { env_->isolate()->ThrowException(Exception::Error(message)); } + Maybe WriteHostObject(Isolate* isolate, Local object) override { + if (env_->message_port_constructor_template()->HasInstance(object)) { + return WriteMessagePort(Unwrap(object)); + } + + THROW_ERR_CANNOT_TRANSFER_OBJECT(env_); + return Nothing(); + } + + void Finish() { + // Only close the MessagePort handles and actually transfer them + // once we know that serialization succeeded. + for (MessagePort* port : ports_) { + port->Close(); + msg_->AddMessagePort(port->Detach()); + } + } + ValueSerializer* serializer = nullptr; private: + Maybe WriteMessagePort(MessagePort* port) { + for (uint32_t i = 0; i < ports_.size(); i++) { + if (ports_[i] == port) { + serializer->WriteUint32(i); + return Just(true); + } + } + + THROW_ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST(env_); + return Nothing(); + } + Environment* env_; Local context_; Message* msg_; + std::vector ports_; friend class worker::Message; }; @@ -131,7 +195,7 @@ Maybe Message::Serialize(Environment* env, Local entry; if (!transfer_list->Get(context, i).ToLocal(&entry)) return Nothing(); - // Currently, we support ArrayBuffers. + // Currently, we support ArrayBuffers and MessagePorts. if (entry->IsArrayBuffer()) { Local ab = entry.As(); // If we cannot render the ArrayBuffer unusable in this Isolate and @@ -144,6 +208,12 @@ Maybe Message::Serialize(Environment* env, array_buffers.push_back(ab); serializer.TransferArrayBuffer(id, ab); continue; + } else if (env->message_port_constructor_template() + ->HasInstance(entry)) { + MessagePort* port = Unwrap(entry.As()); + CHECK_NE(port, nullptr); + delegate.ports_.push_back(port); + continue; } THROW_ERR_INVALID_TRANSFER_OBJECT(env); @@ -167,6 +237,8 @@ Maybe Message::Serialize(Environment* env, contents.ByteLength() }); } + delegate.Finish(); + // The serializer gave us a buffer allocated using `malloc()`. std::pair data = serializer.Release(); main_message_buf_ = diff --git a/src/node_messaging.h b/src/node_messaging.h index 7bd60163ea167c..074267bb67c2dd 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -37,9 +37,14 @@ class Message { v8::Local input, v8::Local transfer_list); + // Internal method of Message that is called once serialization finishes + // and that transfers ownership of `data` to this message. + void AddMessagePort(std::unique_ptr&& data); + private: MallocedBuffer main_message_buf_; std::vector> array_buffer_contents_; + std::vector> message_ports_; friend class MessagePort; }; diff --git a/test/parallel/test-message-port-message-port-transferring.js b/test/parallel/test-message-port-message-port-transferring.js new file mode 100644 index 00000000000000..a7490b3678ac74 --- /dev/null +++ b/test/parallel/test-message-port-message-port-transferring.js @@ -0,0 +1,23 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { MessageChannel } = require('worker'); + +{ + const { port1: basePort1, port2: basePort2 } = new MessageChannel(); + const { + port1: transferredPort1, port2: transferredPort2 + } = new MessageChannel(); + + basePort1.postMessage({ transferredPort1 }, [ transferredPort1 ]); + basePort2.on('message', common.mustCall(({ transferredPort1 }) => { + transferredPort1.postMessage('foobar'); + transferredPort2.on('message', common.mustCall((msg) => { + assert.strictEqual(msg, 'foobar'); + transferredPort1.close(common.mustCall()); + basePort1.close(common.mustCall()); + })); + })); +} From d1f372f0522558dac6e8dec9e3201a4c3325d3bc Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 13 May 2018 19:39:32 +0200 Subject: [PATCH 024/150] worker: add `SharedArrayBuffer` sharing Logic is added to the `MessagePort` mechanism that attaches hidden objects to those instances when they are transferred that track their lifetime and maintain a reference count, to make sure that memory is freed at the appropriate times. Thanks to Stephen Belanger for reviewing this change in its original PR. Refs: https://github.com/ayojs/ayo/pull/106 PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- doc/api/worker.md | 15 +++- node.gyp | 2 + src/env.h | 2 + src/node_errors.h | 5 +- src/node_messaging.cc | 57 ++++++++++++- src/node_messaging.h | 6 +- src/sharedarraybuffer_metadata.cc | 129 ++++++++++++++++++++++++++++++ src/sharedarraybuffer_metadata.h | 67 ++++++++++++++++ 8 files changed, 274 insertions(+), 9 deletions(-) create mode 100644 src/sharedarraybuffer_metadata.cc create mode 100644 src/sharedarraybuffer_metadata.h diff --git a/doc/api/worker.md b/doc/api/worker.md index 6a391c5a9e3b19..974ff2e46710db 100644 --- a/doc/api/worker.md +++ b/doc/api/worker.md @@ -85,14 +85,16 @@ to stringify. `transferList` may be a list of `ArrayBuffer` and `MessagePort` objects. After transferring, they will not be usable on the sending side of the channel -anymore (even if they are not contained in `value`). +anymore (even if they are not contained in `value`). Unlike with +[child processes][], transferring handles such as network sockets is currently +not supported. + +If `value` contains [`SharedArrayBuffer`][] instances, those will be accessible +from either thread. They cannot be listed in `transferList`. `value` may still contain `ArrayBuffer` instances that are not in `transferList`; in that case, the underlying memory is copied rather than moved. -For more information on the serialization and deserialization mechanisms -behind this API, see the [serialization API of the `v8` module][v8.serdes]. - Because the object cloning uses the structured clone algorithm, non-enumerable properties, property accessors, and object prototypes are not preserved. In particular, [`Buffer`][] objects will be read as @@ -101,6 +103,9 @@ plain [`Uint8Array`][]s on the receiving side. The message object will be cloned immediately, and can be modified after posting without having side effects. +For more information on the serialization and deserialization mechanisms +behind this API, see the [serialization API of the `v8` module][v8.serdes]. + ### port.ref() + +* {boolean} + +Is `true` if this code is not running inside of a [`Worker`][] thread. + +## worker.parentPort + + +* {null|MessagePort} + +If this thread was spawned as a [`Worker`][], this will be a [`MessagePort`][] +allowing communication with the parent thread. Messages sent using +`parentPort.postMessage()` will be available in the parent thread +using `worker.on('message')`, and messages sent from the parent thread +using `worker.postMessage()` will be available in this thread using +`parentPort.on('message')`. + +## worker.threadId + + +* {integer} + +An integer identifier for the current thread. On the corresponding worker object +(if there is any), it is available as [`worker.threadId`][]. + +## worker.workerData + + +An arbitrary JavaScript value that contains a clone of the data passed +to this thread’s `Worker` constructor. + ## Class: MessageChannel + +The `Worker` class represents an independent JavaScript execution thread. +Most Node.js APIs are available inside of it. + +Notable differences inside a Worker environment are: + +- The [`process.stdin`][], [`process.stdout`][] and [`process.stderr`][] + properties are set to `null`. +- The [`require('worker').isMainThread`][] property is set to `false`. +- The [`require('worker').parentPort`][] message port is available, +- [`process.exit()`][] does not stop the whole program, just the single thread, + and [`process.abort()`][] is not available. +- [`process.chdir()`][] and `process` methods that set group or user ids + are not available. +- [`process.env`][] is a read-only reference to the environment variables. +- [`process.title`][] cannot be modified. +- Signals will not be delivered through [`process.on('...')`][Signals events]. +- Execution may stop at any point as a result of [`worker.terminate()`][] + being invoked. +- IPC channels from parent processes are not accessible. + +Currently, the following differences also exist until they are addressed: + +- The [`inspector`][] module is not available yet. +- Native addons are not supported yet. + +Creating `Worker` instances inside of other `Worker`s is possible. + +Like [Web Workers][] and the [`cluster` module][], two-way communication can be +achieved through inter-thread message passing. Internally, a `Worker` has a +built-in pair of [`MessagePort`][]s that are already associated with each other +when the `Worker` is created. While the `MessagePort` object on the parent side +is not directly exposed, its functionalities are exposed through +[`worker.postMessage()`][] and the [`worker.on('message')`][] event +on the `Worker` object for the parent thread. + +To create custom messaging channels (which is encouraged over using the default +global channel because it facilitates separation of concerns), users can create +a `MessageChannel` object on either thread and pass one of the +`MessagePort`s on that `MessageChannel` to the other thread through a +pre-existing channel, such as the global one. + +See [`port.postMessage()`][] for more information on how messages are passed, +and what kind of JavaScript values can be successfully transported through +the thread barrier. + +For example: + +```js +const assert = require('assert'); +const { Worker, MessageChannel, MessagePort, isMainThread } = require('worker'); +if (isMainThread) { + const worker = new Worker(__filename); + const subChannel = new MessageChannel(); + worker.postMessage({ hereIsYourPort: subChannel.port1 }, [subChannel.port1]); + subChannel.port2.on('message', (value) => { + console.log('received:', value); + }); +} else { + require('worker').once('workerMessage', (value) => { + assert(value.hereIsYourPort instanceof MessagePort); + value.hereIsYourPort.postMessage('the worker is sending this'); + value.hereIsYourPort.close(); + }); +} +``` + +### new Worker(filename, options) + +* `filename` {string} The absolute path to the Worker’s main script. + If `options.eval` is true, this is a string containing JavaScript code rather + than a path. +* `options` {Object} + * `eval` {boolean} If true, interpret the first argument to the constructor + as a script that is executed once the worker is online. + * `data` {any} Any JavaScript value that will be cloned and made + available as [`require('worker').workerData`][]. The cloning will occur as + described in the [HTML structured clone algorithm][], and an error will be + thrown if the object cannot be cloned (e.g. because it contains + `function`s). + +### Event: 'error' + + +* `err` {Error} + +The `'error'` event is emitted if the worker thread throws an uncaught +exception. In that case, the worker will be terminated. + +### Event: 'exit' + + +* `exitCode` {integer} + +The `'exit'` event is emitted once the worker has stopped. If the worker +exited by calling [`process.exit()`][], the `exitCode` parameter will be the +passed exit code. If the worker was terminated, the `exitCode` parameter will +be `1`. + +### Event: 'message' + + +* `value` {any} The transmitted value + +The `'message'` event is emitted when the worker thread has invoked +[`require('worker').postMessage()`][]. See the [`port.on('message')`][] event +for more details. + +### Event: 'online' + + +The `'online'` event is emitted when the worker thread has started executing +JavaScript code. + +### worker.postMessage(value[, transferList]) + + +* `value` {any} +* `transferList` {Object[]} + +Send a message to the worker that will be received via +[`require('worker').on('workerMessage')`][]. See [`port.postMessage()`][] for +more details. + +### worker.ref() + + +Opposite of `unref()`, calling `ref()` on a previously `unref()`ed worker will +*not* let the program exit if it's the only active handle left (the default +behavior). If the worker is `ref()`ed, calling `ref()` again will have +no effect. + +### worker.terminate([callback]) + + +* `callback` {Function} + +Stop all JavaScript execution in the worker thread as soon as possible. +`callback` is an optional function that is invoked once this operation is known +to have completed. + +**Warning**: Currently, not all code in the internals of Node.js is prepared to +expect termination at arbitrary points in time and may crash if it encounters +that condition. Consequently, you should currently only call `.terminate()` if +it is known that the Worker thread is not accessing Node.js core modules other +than what is exposed in the `worker` module. + +### worker.threadId + + +* {integer} + +An integer identifier for the referenced thread. Inside the worker thread, +it is available as [`require('worker').threadId`][]. + +### worker.unref() + + +Calling `unref()` on a worker will allow the thread to exit if this is the only +active handle in the event system. If the worker is already `unref()`ed calling +`unref()` again will have no effect. + [`Buffer`]: buffer.html -[child processes]: child_process.html [`EventEmitter`]: events.html [`MessagePort`]: #worker_class_messageport [`port.postMessage()`]: #worker_port_postmessage_value_transferlist +[`Worker`]: #worker_class_worker +[`worker.terminate()`]: #worker_worker_terminate_callback +[`worker.postMessage()`]: #worker_worker_postmessage_value_transferlist_1 +[`worker.on('message')`]: #worker_event_message_1 +[`worker.threadId`]: #worker_worker_threadid_1 +[`port.on('message')`]: #worker_event_message +[`process.exit()`]: process.html#process_process_exit_code +[`process.abort()`]: process.html#process_process_abort +[`process.chdir()`]: process.html#process_process_chdir_directory +[`process.env`]: process.html#process_process_env +[`process.stdin`]: process.html#process_process_stdin +[`process.stderr`]: process.html#process_process_stderr +[`process.stdout`]: process.html#process_process_stdout +[`process.title`]: process.html#process_process_title +[`require('worker').workerData`]: #worker_worker_workerdata +[`require('worker').on('workerMessage')`]: #worker_event_workermessage +[`require('worker').postMessage()`]: #worker_worker_postmessage_value_transferlist +[`require('worker').isMainThread`]: #worker_worker_ismainthread +[`require('worker').threadId`]: #worker_worker_threadid +[`cluster` module]: cluster.html +[`inspector`]: inspector.html [v8.serdes]: v8.html#v8_serialization_api [`SharedArrayBuffer`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer +[Signals events]: process.html#process_signal_events [`Uint8Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array [browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort +[child processes]: child_process.html [HTML structured clone algorithm]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm +[Web Workers]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API diff --git a/lib/inspector.js b/lib/inspector.js index 3285c1040a7132..f4ec71fd6c2105 100644 --- a/lib/inspector.js +++ b/lib/inspector.js @@ -12,7 +12,7 @@ const { const util = require('util'); const { Connection, open, url } = process.binding('inspector'); -if (!Connection) +if (!Connection || !require('internal/worker').isMainThread) throw new ERR_INSPECTOR_NOT_AVAILABLE(); const connectionSymbol = Symbol('connectionProperty'); diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 6477c2d8282f43..4817ec110a99e5 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -24,6 +24,7 @@ _shouldAbortOnUncaughtToggle }, { internalBinding, NativeModule }) { const exceptionHandlerState = { captureFn: null }; + const isMainThread = internalBinding('worker').threadId === 0; function startup() { const EventEmitter = NativeModule.require('events'); @@ -100,7 +101,9 @@ NativeModule.require('internal/inspector_async_hook').setup(); } - _process.setupChannel(); + if (isMainThread) + _process.setupChannel(); + _process.setupRawDebug(_rawDebug); const browserGlobals = !process._noBrowserGlobals; @@ -175,8 +178,11 @@ // are running from a script and running the REPL - but there are a few // others like the debugger or running --eval arguments. Here we decide // which mode we run in. - - if (NativeModule.exists('_third_party_main')) { + if (internalBinding('worker').getEnvMessagePort() !== undefined) { + // This means we are in a Worker context, and any script execution + // will be directed by the worker module. + NativeModule.require('internal/worker').setupChild(evalScript); + } else if (NativeModule.exists('_third_party_main')) { // To allow people to extend Node in different ways, this hook allows // one to drop a file lib/_third_party_main.js into the build // directory which will be executed instead of Node's normal loading. @@ -542,7 +548,7 @@ return `process.binding('inspector').callAndPauseOnStart(${fn}, {})`; } - function evalScript(name) { + function evalScript(name, body = wrapForBreakOnFirstLine(process._eval)) { const CJSModule = NativeModule.require('internal/modules/cjs/loader'); const path = NativeModule.require('path'); const cwd = tryGetCwd(path); @@ -550,7 +556,6 @@ const module = new CJSModule(name); module.filename = path.join(cwd, name); module.paths = CJSModule._nodeModulePaths(cwd); - const body = wrapForBreakOnFirstLine(process._eval); const script = `global.__filename = ${JSON.stringify(name)};\n` + 'global.exports = exports;\n' + 'global.module = module;\n' + diff --git a/lib/internal/errors.js b/lib/internal/errors.js index b9db40abd3b4c9..1fb90b37f88642 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -851,4 +851,9 @@ E('ERR_VM_MODULE_NOT_LINKED', E('ERR_VM_MODULE_NOT_MODULE', 'Provided module is not an instance of Module', Error); E('ERR_VM_MODULE_STATUS', 'Module status %s', Error); +E('ERR_WORKER_NEED_ABSOLUTE_PATH', + 'The worker script filename must be an absolute path. Received "%s"', + TypeError); +E('ERR_WORKER_UNSERIALIZABLE_ERROR', + 'Serializing an uncaught exception failed', Error); E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error); diff --git a/lib/internal/process.js b/lib/internal/process.js index bcefca316a236e..45d9c77b32007f 100644 --- a/lib/internal/process.js +++ b/lib/internal/process.js @@ -16,6 +16,7 @@ const util = require('util'); const constants = process.binding('constants').os.signals; const assert = require('assert').strict; const { deprecate } = require('internal/util'); +const { isMainThread } = require('internal/worker'); process.assert = deprecate( function(x, msg) { @@ -186,6 +187,11 @@ function setupKillAndExit() { function setupSignalHandlers() { + if (!isMainThread) { + // Worker threads don't receive signals. + return; + } + const signalWraps = Object.create(null); let Signal; diff --git a/lib/internal/process/methods.js b/lib/internal/process/methods.js index 209702ed7b9b76..77ceeafef41122 100644 --- a/lib/internal/process/methods.js +++ b/lib/internal/process/methods.js @@ -1,9 +1,17 @@ 'use strict'; +const { + isMainThread +} = require('internal/worker'); + function setupProcessMethods(_chdir, _cpuUsage, _hrtime, _memoryUsage, _rawDebug, _umask, _initgroups, _setegid, _seteuid, _setgid, _setuid, _setgroups) { // Non-POSIX platforms like Windows don't have certain methods. + // Workers also lack these methods since they change process-global state. + if (!isMainThread) + return; + if (_setgid !== undefined) { setupPosixMethods(_initgroups, _setegid, _seteuid, _setgid, _setuid, _setgroups); diff --git a/lib/internal/process/stdio.js b/lib/internal/process/stdio.js index eaba4dfca13a47..76e6ab85140535 100644 --- a/lib/internal/process/stdio.js +++ b/lib/internal/process/stdio.js @@ -6,6 +6,7 @@ const { ERR_UNKNOWN_STDIN_TYPE, ERR_UNKNOWN_STREAM_TYPE } = require('internal/errors').codes; +const { isMainThread } = require('internal/worker'); exports.setup = setupStdio; @@ -16,6 +17,8 @@ function setupStdio() { function getStdout() { if (stdout) return stdout; + if (!isMainThread) + return new (require('stream').Writable)({ write(b, e, cb) { cb(); } }); stdout = createWritableStdioStream(1); stdout.destroySoon = stdout.destroy; stdout._destroy = function(er, cb) { @@ -31,6 +34,8 @@ function setupStdio() { function getStderr() { if (stderr) return stderr; + if (!isMainThread) + return new (require('stream').Writable)({ write(b, e, cb) { cb(); } }); stderr = createWritableStdioStream(2); stderr.destroySoon = stderr.destroy; stderr._destroy = function(er, cb) { @@ -46,6 +51,8 @@ function setupStdio() { function getStdin() { if (stdin) return stdin; + if (!isMainThread) + return new (require('stream').Readable)({ read() { this.push(null); } }); const tty_wrap = process.binding('tty_wrap'); const fd = 0; diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 634d3302333584..3dd73415ded862 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -1,6 +1,8 @@ 'use strict'; -const hasInspector = process.config.variables.v8_enable_inspector === 1; +// TODO(addaleax): Figure out how to integrate the inspector with workers. +const hasInspector = process.config.variables.v8_enable_inspector === 1 && + require('internal/worker').isMainThread; const inspector = hasInspector ? require('inspector') : undefined; let session; diff --git a/lib/internal/worker.js b/lib/internal/worker.js index 73f7525aa73cc2..c982478b9334e8 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -1,24 +1,49 @@ 'use strict'; +const Buffer = require('buffer').Buffer; const EventEmitter = require('events'); +const assert = require('assert'); +const path = require('path'); const util = require('util'); +const { + ERR_INVALID_ARG_TYPE, + ERR_WORKER_NEED_ABSOLUTE_PATH, + ERR_WORKER_UNSERIALIZABLE_ERROR +} = require('internal/errors').codes; const { internalBinding } = require('internal/bootstrap/loaders'); const { MessagePort, MessageChannel } = internalBinding('messaging'); const { handle_onclose } = internalBinding('symbols'); +const { clearAsyncIdStack } = require('internal/async_hooks'); util.inherits(MessagePort, EventEmitter); +const { + Worker: WorkerImpl, + getEnvMessagePort, + threadId +} = internalBinding('worker'); + +const isMainThread = threadId === 0; + const kOnMessageListener = Symbol('kOnMessageListener'); +const kHandle = Symbol('kHandle'); +const kPort = Symbol('kPort'); +const kPublicPort = Symbol('kPublicPort'); +const kDispose = Symbol('kDispose'); +const kOnExit = Symbol('kOnExit'); +const kOnMessage = Symbol('kOnMessage'); +const kOnCouldNotSerializeErr = Symbol('kOnCouldNotSerializeErr'); +const kOnErrorMessage = Symbol('kOnErrorMessage'); const debug = util.debuglog('worker'); -// A MessagePort consists of a handle (that wraps around an +// A communication channel consisting of a handle (that wraps around an // uv_async_t) which can receive information from other threads and emits // .onmessage events, and a function used for sending data to a MessagePort // in some other thread. MessagePort.prototype[kOnMessageListener] = function onmessage(payload) { - debug('received message', payload); + debug(`[${threadId}] received message`, payload); // Emit the deserialized object to userland. this.emit('message', payload); }; @@ -79,6 +104,9 @@ MessagePort.prototype.close = function(cb) { originalClose.call(this); }; +const drainMessagePort = MessagePort.prototype.drain; +delete MessagePort.prototype.drain; + function setupPortReferencing(port, eventEmitter, eventName) { // Keep track of whether there are any workerMessage listeners: // If there are some, ref() the channel so it keeps the event loop alive. @@ -99,7 +127,194 @@ function setupPortReferencing(port, eventEmitter, eventName) { }); } + +class Worker extends EventEmitter { + constructor(filename, options = {}) { + super(); + debug(`[${threadId}] create new worker`, filename, options); + if (typeof filename !== 'string') { + throw new ERR_INVALID_ARG_TYPE('filename', 'string', filename); + } + + if (!options.eval && !path.isAbsolute(filename)) { + throw new ERR_WORKER_NEED_ABSOLUTE_PATH(filename); + } + + // Set up the C++ handle for the worker, as well as some internal wiring. + this[kHandle] = new WorkerImpl(); + this[kHandle].onexit = (code) => this[kOnExit](code); + this[kPort] = this[kHandle].messagePort; + this[kPort].on('message', (data) => this[kOnMessage](data)); + this[kPort].start(); + this[kPort].unref(); + debug(`[${threadId}] created Worker with ID ${this.threadId}`); + + const { port1, port2 } = new MessageChannel(); + this[kPublicPort] = port1; + this[kPublicPort].on('message', (message) => this.emit('message', message)); + setupPortReferencing(this[kPublicPort], this, 'message'); + this[kPort].postMessage({ + type: 'loadScript', + filename, + doEval: !!options.eval, + workerData: options.workerData, + publicPort: port2 + }, [port2]); + // Actually start the new thread now that everything is in place. + this[kHandle].startThread(); + } + + [kOnExit](code) { + debug(`[${threadId}] hears end event for Worker ${this.threadId}`); + drainMessagePort.call(this[kPublicPort]); + this[kDispose](); + this.emit('exit', code); + this.removeAllListeners(); + } + + [kOnCouldNotSerializeErr]() { + this.emit('error', new ERR_WORKER_UNSERIALIZABLE_ERROR()); + } + + [kOnErrorMessage](serialized) { + // This is what is called for uncaught exceptions. + const error = deserializeError(serialized); + this.emit('error', error); + } + + [kOnMessage](message) { + switch (message.type) { + case 'upAndRunning': + return this.emit('online'); + case 'couldNotSerializeError': + return this[kOnCouldNotSerializeErr](); + case 'errorMessage': + return this[kOnErrorMessage](message.error); + } + + assert.fail(`Unknown worker message type ${message.type}`); + } + + [kDispose]() { + this[kHandle].onexit = null; + this[kHandle] = null; + this[kPort] = null; + this[kPublicPort] = null; + } + + postMessage(...args) { + this[kPublicPort].postMessage(...args); + } + + terminate(callback) { + if (this[kHandle] === null) return; + + debug(`[${threadId}] terminates Worker with ID ${this.threadId}`); + + if (typeof callback !== 'undefined') + this.once('exit', (exitCode) => callback(null, exitCode)); + + this[kHandle].stopThread(); + } + + ref() { + if (this[kHandle] === null) return; + + this[kHandle].ref(); + this[kPublicPort].ref(); + } + + unref() { + if (this[kHandle] === null) return; + + this[kHandle].unref(); + this[kPublicPort].unref(); + } + + get threadId() { + if (this[kHandle] === null) return -1; + + return this[kHandle].threadId; + } +} + +let originalFatalException; + +function setupChild(evalScript) { + // Called during bootstrap to set up worker script execution. + debug(`[${threadId}] is setting up worker child environment`); + const port = getEnvMessagePort(); + + const publicWorker = require('worker'); + + port.on('message', (message) => { + if (message.type === 'loadScript') { + const { filename, doEval, workerData, publicPort } = message; + publicWorker.parentPort = publicPort; + setupPortReferencing(publicPort, publicPort, 'message'); + publicWorker.workerData = workerData; + debug(`[${threadId}] starts worker script ${filename} ` + + `(eval = ${eval}) at cwd = ${process.cwd()}`); + port.unref(); + port.postMessage({ type: 'upAndRunning' }); + if (doEval) { + evalScript('[worker eval]', filename); + } else { + process.argv[1] = filename; // script filename + require('module').runMain(); + } + return; + } + + assert.fail(`Unknown worker message type ${message.type}`); + }); + + port.start(); + + originalFatalException = process._fatalException; + process._fatalException = fatalException; + + function fatalException(error) { + debug(`[${threadId}] gets fatal exception`); + let caught = false; + try { + caught = originalFatalException.call(this, error); + } catch (e) { + error = e; + } + debug(`[${threadId}] fatal exception caught = ${caught}`); + + if (!caught) { + let serialized; + try { + serialized = serializeError(error); + } catch {} + debug(`[${threadId}] fatal exception serialized = ${!!serialized}`); + if (serialized) + port.postMessage({ type: 'errorMessage', error: serialized }); + else + port.postMessage({ type: 'couldNotSerializeError' }); + clearAsyncIdStack(); + } + } +} + +// TODO(addaleax): These can be improved a lot. +function serializeError(error) { + return Buffer.from(util.inspect(error), 'utf8'); +} + +function deserializeError(error) { + return Buffer.from(error.buffer, + error.byteOffset, + error.byteLength).toString('utf8'); +} + module.exports = { MessagePort, - MessageChannel + MessageChannel, + threadId, + Worker, + setupChild, + isMainThread }; diff --git a/lib/worker.js b/lib/worker.js index d67fb4efe40a33..0609650cd5731d 100644 --- a/lib/worker.js +++ b/lib/worker.js @@ -1,5 +1,18 @@ 'use strict'; -const { MessagePort, MessageChannel } = require('internal/worker'); +const { + isMainThread, + MessagePort, + MessageChannel, + threadId, + Worker +} = require('internal/worker'); -module.exports = { MessagePort, MessageChannel }; +module.exports = { + isMainThread, + MessagePort, + MessageChannel, + threadId, + Worker, + parentPort: null +}; diff --git a/node.gyp b/node.gyp index 823dda45b03280..c7ba2432827132 100644 --- a/node.gyp +++ b/node.gyp @@ -348,6 +348,7 @@ 'src/node_v8.cc', 'src/node_stat_watcher.cc', 'src/node_watchdog.cc', + 'src/node_worker.cc', 'src/node_zlib.cc', 'src/node_i18n.cc', 'src/pipe_wrap.cc', @@ -406,6 +407,7 @@ 'src/node_wrap.h', 'src/node_revert.h', 'src/node_i18n.h', + 'src/node_worker.h', 'src/pipe_wrap.h', 'src/tty_wrap.h', 'src/tcp_wrap.h', diff --git a/src/async_wrap.h b/src/async_wrap.h index cf269a4c1f5e1e..b2f96477b490e0 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -67,6 +67,7 @@ namespace node { V(TTYWRAP) \ V(UDPSENDWRAP) \ V(UDPWRAP) \ + V(WORKER) \ V(WRITEWRAP) \ V(ZLIB) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 3bd854639b2c6d..06a29223973c5d 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -65,6 +65,14 @@ v8::Local BaseObject::object() { return PersistentToLocal(env_->isolate(), persistent_handle_); } +v8::Local BaseObject::object(v8::Isolate* isolate) { + v8::Local handle = object(); +#ifdef DEBUG + CHECK_EQ(handle->CreationContext()->GetIsolate(), isolate); + CHECK_EQ(env_->isolate(), isolate); +#endif + return handle; +} Environment* BaseObject::env() const { return env_; diff --git a/src/base_object.h b/src/base_object.h index e0b60843401681..38291d598feb1c 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -43,6 +43,10 @@ class BaseObject { // persistent.IsEmpty() is true. inline v8::Local object(); + // Same as the above, except it additionally verifies that this object + // is associated with the passed Isolate in debug mode. + inline v8::Local object(v8::Isolate* isolate); + inline Persistent& persistent(); inline Environment* env() const; diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 35c7c4dc696ebd..f9db02562d9c8a 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -114,12 +114,14 @@ void SetupBootstrapObject(Environment* env, BOOTSTRAP_METHOD(_umask, Umask); #if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__) - BOOTSTRAP_METHOD(_initgroups, InitGroups); - BOOTSTRAP_METHOD(_setegid, SetEGid); - BOOTSTRAP_METHOD(_seteuid, SetEUid); - BOOTSTRAP_METHOD(_setgid, SetGid); - BOOTSTRAP_METHOD(_setuid, SetUid); - BOOTSTRAP_METHOD(_setgroups, SetGroups); + if (env->is_main_thread()) { + BOOTSTRAP_METHOD(_initgroups, InitGroups); + BOOTSTRAP_METHOD(_setegid, SetEGid); + BOOTSTRAP_METHOD(_seteuid, SetEUid); + BOOTSTRAP_METHOD(_setgid, SetGid); + BOOTSTRAP_METHOD(_setuid, SetUid); + BOOTSTRAP_METHOD(_setgroups, SetGroups); + } #endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__) Local should_abort_on_uncaught_toggle = diff --git a/src/callback_scope.cc b/src/callback_scope.cc index 9eac7beb038a26..23e6d5b0632f2c 100644 --- a/src/callback_scope.cc +++ b/src/callback_scope.cc @@ -79,6 +79,11 @@ void InternalCallbackScope::Close() { closed_ = true; HandleScope handle_scope(env_->isolate()); + if (!env_->can_call_into_js()) return; + if (failed_ && !env_->is_main_thread() && env_->is_stopping_worker()) { + env_->async_hooks()->clear_async_id_stack(); + } + if (pushed_ids_) env_->async_hooks()->pop_async_id(async_context_.async_id); diff --git a/src/env-inl.h b/src/env-inl.h index 50328bd77c1a89..eeb419b4a0fad2 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -582,13 +582,42 @@ void Environment::SetUnrefImmediate(native_immediate_callback cb, } inline bool Environment::can_call_into_js() const { - return can_call_into_js_; + return can_call_into_js_ && (is_main_thread() || !is_stopping_worker()); } inline void Environment::set_can_call_into_js(bool can_call_into_js) { can_call_into_js_ = can_call_into_js; } +inline bool Environment::is_main_thread() const { + return thread_id_ == 0; +} + +inline double Environment::thread_id() const { + return thread_id_; +} + +inline void Environment::set_thread_id(double id) { + thread_id_ = id; +} + +inline worker::Worker* Environment::worker_context() const { + return worker_context_; +} + +inline void Environment::set_worker_context(worker::Worker* context) { + CHECK_EQ(worker_context_, nullptr); // Should be set only once. + worker_context_ = context; +} + +inline void Environment::add_sub_worker_context(worker::Worker* context) { + sub_worker_contexts_.insert(context); +} + +inline void Environment::remove_sub_worker_context(worker::Worker* context) { + sub_worker_contexts_.erase(context); +} + inline performance::performance_state* Environment::performance_state() { return performance_state_.get(); } diff --git a/src/env.cc b/src/env.cc index 090b43968bf665..8df59d1546dbdd 100644 --- a/src/env.cc +++ b/src/env.cc @@ -4,6 +4,7 @@ #include "node_buffer.h" #include "node_platform.h" #include "node_file.h" +#include "node_worker.h" #include "tracing/agent.h" #include @@ -25,6 +26,7 @@ using v8::StackTrace; using v8::String; using v8::Symbol; using v8::Value; +using worker::Worker; IsolateData::IsolateData(Isolate* isolate, uv_loop_t* event_loop, @@ -444,7 +446,9 @@ void Environment::RunAndClearNativeImmediates() { if (it->refed_) ref_count++; if (UNLIKELY(try_catch.HasCaught())) { - FatalException(isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(isolate(), try_catch); + // Bail out, remove the already executed callbacks from list // and set up a new TryCatch for the other pending callbacks. std::move_backward(it, list.end(), list.begin() + (list.end() - it)); @@ -632,4 +636,25 @@ void Environment::AsyncHooks::grow_async_ids_stack() { uv_key_t Environment::thread_local_env = {}; +void Environment::Exit(int exit_code) { + if (is_main_thread()) + exit(exit_code); + else + worker_context_->Exit(exit_code); +} + +void Environment::stop_sub_worker_contexts() { + while (!sub_worker_contexts_.empty()) { + Worker* w = *sub_worker_contexts_.begin(); + remove_sub_worker_context(w); + w->Exit(1); + w->JoinThread(); + } +} + +bool Environment::is_stopping_worker() const { + CHECK(!is_main_thread()); + return worker_context_->is_stopped(); +} + } // namespace node diff --git a/src/env.h b/src/env.h index cdb592732a4264..cf6873e5fe7c6a 100644 --- a/src/env.h +++ b/src/env.h @@ -55,6 +55,10 @@ namespace performance { class performance_state; } +namespace worker { +class Worker; +} + namespace loader { class ModuleWrap; @@ -193,7 +197,10 @@ struct PackageConfig { V(mac_string, "mac") \ V(main_string, "main") \ V(max_buffer_string, "maxBuffer") \ + V(max_semi_space_size_string, "maxSemiSpaceSize") \ + V(max_old_space_size_string, "maxOldSpaceSize") \ V(message_string, "message") \ + V(message_port_string, "messagePort") \ V(message_port_constructor_string, "MessagePort") \ V(minttl_string, "minttl") \ V(modulus_string, "modulus") \ @@ -280,6 +287,7 @@ struct PackageConfig { V(subject_string, "subject") \ V(subjectaltname_string, "subjectaltname") \ V(syscall_string, "syscall") \ + V(thread_id_string, "threadId") \ V(ticketkeycallback_string, "onticketkeycallback") \ V(timeout_string, "timeout") \ V(tls_ticket_string, "tlsTicket") \ @@ -328,6 +336,7 @@ struct PackageConfig { V(http2stream_constructor_template, v8::ObjectTemplate) \ V(immediate_callback_function, v8::Function) \ V(inspector_console_api_object, v8::Object) \ + V(message_port, v8::Object) \ V(message_port_constructor_template, v8::FunctionTemplate) \ V(pbkdf2_constructor_template, v8::ObjectTemplate) \ V(pipe_constructor_template, v8::FunctionTemplate) \ @@ -601,6 +610,7 @@ class Environment { void RegisterHandleCleanups(); void CleanupHandles(); + void Exit(int code); // Register clean-up cb to be called on environment destruction. inline void RegisterHandleCleanup(uv_handle_t* handle, @@ -714,6 +724,18 @@ class Environment { inline bool can_call_into_js() const; inline void set_can_call_into_js(bool can_call_into_js); + // TODO(addaleax): This should be inline. + bool is_stopping_worker() const; + + inline bool is_main_thread() const; + inline double thread_id() const; + inline void set_thread_id(double id); + inline worker::Worker* worker_context() const; + inline void set_worker_context(worker::Worker* context); + inline void add_sub_worker_context(worker::Worker* context); + inline void remove_sub_worker_context(worker::Worker* context); + void stop_sub_worker_contexts(); + inline void ThrowError(const char* errmsg); inline void ThrowTypeError(const char* errmsg); inline void ThrowRangeError(const char* errmsg); @@ -855,12 +877,15 @@ class Environment { std::vector destroy_async_id_list_; AliasedBuffer should_abort_on_uncaught_toggle_; - int should_not_abort_scope_counter_ = 0; std::unique_ptr performance_state_; std::unordered_map performance_marks_; + bool can_call_into_js_ = true; + double thread_id_ = 0; + std::unordered_set sub_worker_contexts_; + #if HAVE_INSPECTOR std::unique_ptr inspector_agent_; @@ -893,6 +918,8 @@ class Environment { std::vector> file_handle_read_wrap_freelist_; + worker::Worker* worker_context_ = nullptr; + struct ExitCallback { void (*cb_)(void* arg); void* arg_; diff --git a/src/js_stream.cc b/src/js_stream.cc index c766c322e3017a..e562a62f3d1bb2 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -44,7 +44,8 @@ bool JSStream::IsClosing() { TryCatch try_catch(env()->isolate()); Local value; if (!MakeCallback(env()->isclosing_string(), 0, nullptr).ToLocal(&value)) { - FatalException(env()->isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(env()->isolate(), try_catch); return true; } return value->IsTrue(); @@ -59,7 +60,8 @@ int JSStream::ReadStart() { int value_int = UV_EPROTO; if (!MakeCallback(env()->onreadstart_string(), 0, nullptr).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { - FatalException(env()->isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(env()->isolate(), try_catch); } return value_int; } @@ -73,7 +75,8 @@ int JSStream::ReadStop() { int value_int = UV_EPROTO; if (!MakeCallback(env()->onreadstop_string(), 0, nullptr).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { - FatalException(env()->isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(env()->isolate(), try_catch); } return value_int; } @@ -94,7 +97,8 @@ int JSStream::DoShutdown(ShutdownWrap* req_wrap) { arraysize(argv), argv).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { - FatalException(env()->isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(env()->isolate(), try_catch); } return value_int; } @@ -128,7 +132,8 @@ int JSStream::DoWrite(WriteWrap* w, arraysize(argv), argv).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { - FatalException(env()->isolate(), try_catch); + if (!try_catch.HasTerminated()) + FatalException(env()->isolate(), try_catch); } return value_int; } diff --git a/src/node.cc b/src/node.cc index 281d441a26a86c..f257fde148925e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1016,9 +1016,9 @@ void AppendExceptionLine(Environment* env, } -static void ReportException(Environment* env, - Local er, - Local message) { +void ReportException(Environment* env, + Local er, + Local message) { CHECK(!er.IsEmpty()); HandleScope scope(env->isolate()); @@ -1105,9 +1105,9 @@ static void ReportException(Environment* env, const TryCatch& try_catch) { // Executes a str within the current v8 context. -static Local ExecuteString(Environment* env, - Local source, - Local filename) { +static MaybeLocal ExecuteString(Environment* env, + Local source, + Local filename) { EscapableHandleScope scope(env->isolate()); TryCatch try_catch(env->isolate()); @@ -1120,13 +1120,19 @@ static Local ExecuteString(Environment* env, v8::Script::Compile(env->context(), source, &origin); if (script.IsEmpty()) { ReportException(env, try_catch); - exit(3); + env->Exit(3); + return MaybeLocal(); } MaybeLocal result = script.ToLocalChecked()->Run(env->context()); if (result.IsEmpty()) { + if (try_catch.HasTerminated()) { + env->isolate()->CancelTerminateExecution(); + return MaybeLocal(); + } ReportException(env, try_catch); - exit(4); + env->Exit(4); + return MaybeLocal(); } return scope.Escape(result.ToLocalChecked()); @@ -1225,6 +1231,7 @@ static void Abort(const FunctionCallbackInfo& args) { void Chdir(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + CHECK(env->is_main_thread()); if (args.Length() != 1 || !args[0]->IsString()) { return env->ThrowTypeError("Bad argument."); @@ -1424,6 +1431,7 @@ static void GetEGid(const FunctionCallbackInfo& args) { void SetGid(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + CHECK(env->is_main_thread()); if (!args[0]->IsUint32() && !args[0]->IsString()) { return env->ThrowTypeError("setgid argument must be a number or a string"); @@ -1443,6 +1451,7 @@ void SetGid(const FunctionCallbackInfo& args) { void SetEGid(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + CHECK(env->is_main_thread()); if (!args[0]->IsUint32() && !args[0]->IsString()) { return env->ThrowTypeError("setegid argument must be a number or string"); @@ -1462,6 +1471,7 @@ void SetEGid(const FunctionCallbackInfo& args) { void SetUid(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + CHECK(env->is_main_thread()); if (!args[0]->IsUint32() && !args[0]->IsString()) { return env->ThrowTypeError("setuid argument must be a number or a string"); @@ -1481,6 +1491,7 @@ void SetUid(const FunctionCallbackInfo& args) { void SetEUid(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + CHECK(env->is_main_thread()); if (!args[0]->IsUint32() && !args[0]->IsString()) { return env->ThrowTypeError("seteuid argument must be a number or string"); @@ -1639,9 +1650,10 @@ static void WaitForInspectorDisconnect(Environment* env) { static void Exit(const FunctionCallbackInfo& args) { - WaitForInspectorDisconnect(Environment::GetCurrent(args)); + Environment* env = Environment::GetCurrent(args); + WaitForInspectorDisconnect(env); v8_platform.StopTracingAgent(); - exit(args[0]->Int32Value()); + env->Exit(args[0]->Int32Value()); } @@ -2050,6 +2062,9 @@ void FatalException(Isolate* isolate, Local caught = fatal_exception_function->Call(process_object, 1, &error); + if (fatal_try_catch.HasTerminated()) + return; + if (fatal_try_catch.HasCaught()) { // The fatal exception function threw, so we must exit ReportException(env, fatal_try_catch); @@ -2063,6 +2078,12 @@ void FatalException(Isolate* isolate, void FatalException(Isolate* isolate, const TryCatch& try_catch) { + // If we try to print out a termination exception, we'd just get 'null', + // so just crashing here with that information seems like a better idea, + // and in particular it seems like we should handle terminations at the call + // site for this function rather than by printing them out somewhere. + CHECK(!try_catch.HasTerminated()); + HandleScope scope(isolate); if (!try_catch.IsVerbose()) { FatalException(isolate, try_catch.Exception(), try_catch.Message()); @@ -2584,11 +2605,12 @@ void SetupProcessObject(Environment* env, Local process = env->process_object(); auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title"); - CHECK(process->SetAccessor(env->context(), - title_string, - ProcessTitleGetter, - ProcessTitleSetter, - env->as_external()).FromJust()); + CHECK(process->SetAccessor( + env->context(), + title_string, + ProcessTitleGetter, + env->is_main_thread() ? ProcessTitleSetter : nullptr, + env->as_external()).FromJust()); // process.version READONLY_PROPERTY(process, @@ -2872,25 +2894,27 @@ void SetupProcessObject(Environment* env, CHECK(process->SetAccessor(env->context(), debug_port_string, DebugPortGetter, - DebugPortSetter, + env->is_main_thread() ? DebugPortSetter : nullptr, env->as_external()).FromJust()); // define various internal methods - env->SetMethod(process, - "_startProfilerIdleNotifier", - StartProfilerIdleNotifier); - env->SetMethod(process, - "_stopProfilerIdleNotifier", - StopProfilerIdleNotifier); + if (env->is_main_thread()) { + env->SetMethod(process, + "_startProfilerIdleNotifier", + StartProfilerIdleNotifier); + env->SetMethod(process, + "_stopProfilerIdleNotifier", + StopProfilerIdleNotifier); + env->SetMethod(process, "abort", Abort); + env->SetMethod(process, "chdir", Chdir); + env->SetMethod(process, "umask", Umask); + } + env->SetMethod(process, "_getActiveRequests", GetActiveRequests); env->SetMethod(process, "_getActiveHandles", GetActiveHandles); env->SetMethod(process, "reallyExit", Exit); - env->SetMethod(process, "abort", Abort); - env->SetMethod(process, "chdir", Chdir); env->SetMethod(process, "cwd", Cwd); - env->SetMethod(process, "umask", Umask); - #if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__) env->SetMethod(process, "getuid", GetUid); env->SetMethod(process, "geteuid", GetEUid); @@ -2900,16 +2924,17 @@ void SetupProcessObject(Environment* env, #endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__) env->SetMethod(process, "_kill", Kill); + env->SetMethod(process, "dlopen", DLOpen); - env->SetMethod(process, "_debugProcess", DebugProcess); - env->SetMethod(process, "_debugEnd", DebugEnd); + if (env->is_main_thread()) { + env->SetMethod(process, "_debugProcess", DebugProcess); + env->SetMethod(process, "_debugEnd", DebugEnd); + } env->SetMethod(process, "hrtime", Hrtime); env->SetMethod(process, "cpuUsage", CPUUsage); - env->SetMethod(process, "dlopen", DLOpen); - env->SetMethod(process, "uptime", Uptime); env->SetMethod(process, "memoryUsage", MemoryUsage); } @@ -2945,8 +2970,10 @@ void RawDebug(const FunctionCallbackInfo& args) { } -static Local GetBootstrapper(Environment* env, Local source, - Local script_name) { +static MaybeLocal GetBootstrapper( + Environment* env, + Local source, + Local script_name) { EscapableHandleScope scope(env->isolate()); TryCatch try_catch(env->isolate()); @@ -2957,16 +2984,17 @@ static Local GetBootstrapper(Environment* env, Local source, try_catch.SetVerbose(false); // Execute the bootstrapper javascript file - Local bootstrapper_v = ExecuteString(env, source, script_name); + MaybeLocal bootstrapper_v = ExecuteString(env, source, script_name); + if (bootstrapper_v.IsEmpty()) // This happens when execution was interrupted. + return MaybeLocal(); + if (try_catch.HasCaught()) { ReportException(env, try_catch); exit(10); } - CHECK(bootstrapper_v->IsFunction()); - Local bootstrapper = Local::Cast(bootstrapper_v); - - return scope.Escape(bootstrapper); + CHECK(bootstrapper_v.ToLocalChecked()->IsFunction()); + return scope.Escape(bootstrapper_v.ToLocalChecked().As()); } static bool ExecuteBootstrapper(Environment* env, Local bootstrapper, @@ -3005,13 +3033,18 @@ void LoadEnvironment(Environment* env) { // node_js2c. Local loaders_name = FIXED_ONE_BYTE_STRING(env->isolate(), "internal/bootstrap/loaders.js"); - Local loaders_bootstrapper = + MaybeLocal loaders_bootstrapper = GetBootstrapper(env, LoadersBootstrapperSource(env), loaders_name); Local node_name = FIXED_ONE_BYTE_STRING(env->isolate(), "internal/bootstrap/node.js"); - Local node_bootstrapper = + MaybeLocal node_bootstrapper = GetBootstrapper(env, NodeBootstrapperSource(env), node_name); + if (loaders_bootstrapper.IsEmpty() || node_bootstrapper.IsEmpty()) { + // Execution was interrupted. + return; + } + // Add a reference to the global object Local global = env->context()->Global(); @@ -3059,7 +3092,7 @@ void LoadEnvironment(Environment* env) { // Bootstrap internal loaders Local bootstrapped_loaders; - if (!ExecuteBootstrapper(env, loaders_bootstrapper, + if (!ExecuteBootstrapper(env, loaders_bootstrapper.ToLocalChecked(), arraysize(loaders_bootstrapper_args), loaders_bootstrapper_args, &bootstrapped_loaders)) { @@ -3075,7 +3108,7 @@ void LoadEnvironment(Environment* env) { bootstrapper, bootstrapped_loaders }; - if (!ExecuteBootstrapper(env, node_bootstrapper, + if (!ExecuteBootstrapper(env, node_bootstrapper.ToLocalChecked(), arraysize(node_bootstrapper_args), node_bootstrapper_args, &bootstrapped_node)) { @@ -4209,6 +4242,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, WaitForInspectorDisconnect(&env); env.set_can_call_into_js(false); + env.stop_sub_worker_contexts(); uv_tty_reset_mode(); env.RunCleanup(); RunAtExit(&env); diff --git a/src/node_errors.h b/src/node_errors.h index c9b8e064d49999..8dddfe5a33a4ee 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -30,6 +30,7 @@ namespace node { V(ERR_MISSING_ARGS, TypeError) \ V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \ V(ERR_MISSING_MODULE, Error) \ + V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \ V(ERR_STRING_TOO_LONG, Error) \ V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, TypeError) \ @@ -62,6 +63,9 @@ namespace node { V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \ V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, \ "MessagePort was found in message but not listed in transferList") \ + V(ERR_MISSING_PLATFORM_FOR_WORKER, \ + "The V8 platform used by this instance of Node does not support " \ + "creating Workers") \ V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, \ "Cannot serialize externalized SharedArrayBuffer") \ diff --git a/src/node_internals.h b/src/node_internals.h index a5d8ed0e5d3ad7..7760eb26c6c15c 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -137,6 +137,7 @@ struct sockaddr; V(util) \ V(uv) \ V(v8) \ + V(worker) \ V(zlib) #define NODE_BUILTIN_MODULES(V) \ @@ -314,6 +315,10 @@ class FatalTryCatch : public v8::TryCatch { Environment* env_; }; +void ReportException(Environment* env, + v8::Local er, + v8::Local message); + v8::Maybe ProcessEmitWarning(Environment* env, const char* fmt, ...); v8::Maybe ProcessEmitDeprecationWarning(Environment* env, const char* warning, diff --git a/src/node_messaging.cc b/src/node_messaging.cc index b56cef2d7767cf..352749ea48f483 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -57,7 +57,7 @@ class DeserializerDelegate : public ValueDeserializer::Delegate { if (!deserializer->ReadUint32(&id)) return MaybeLocal(); CHECK_LE(id, message_ports_.size()); - return message_ports_[id]->object(); + return message_ports_[id]->object(isolate); }; MaybeLocal GetSharedArrayBufferFromId( @@ -436,7 +436,7 @@ MessagePort* MessagePort::New( void MessagePort::OnMessage() { HandleScope handle_scope(env()->isolate()); - Local context = object()->CreationContext(); + Local context = object(env()->isolate())->CreationContext(); // data_ can only ever be modified by the owner thread, so no need to lock. // However, the message port may be transferred while it is processing @@ -447,6 +447,13 @@ void MessagePort::OnMessage() { { // Get the head of the message queue. Mutex::ScopedLock lock(data_->mutex_); + + if (stop_event_loop_) { + CHECK(!data_->receiving_messages_); + uv_stop(env()->event_loop()); + break; + } + if (!data_->receiving_messages_) break; if (data_->incoming_messages_.empty()) @@ -514,8 +521,9 @@ void MessagePort::Send(Message&& message) { void MessagePort::Send(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + Local context = object(env->isolate())->CreationContext(); Message msg; - if (msg.Serialize(env, object()->CreationContext(), args[0], args[1]) + if (msg.Serialize(env, context, args[0], args[1]) .IsNothing()) { return; } @@ -548,6 +556,14 @@ void MessagePort::Stop() { data_->receiving_messages_ = false; } +void MessagePort::StopEventLoop() { + Mutex::ScopedLock lock(data_->mutex_); + data_->receiving_messages_ = false; + stop_event_loop_ = true; + + TriggerAsync(); +} + void MessagePort::Start(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); MessagePort* port; @@ -570,6 +586,12 @@ void MessagePort::Stop(const FunctionCallbackInfo& args) { port->Stop(); } +void MessagePort::Drain(const FunctionCallbackInfo& args) { + MessagePort* port; + ASSIGN_OR_RETURN_UNWRAP(&port, args.This()); + port->OnMessage(); +} + size_t MessagePort::self_size() const { Mutex::ScopedLock lock(data_->mutex_); size_t sz = sizeof(*this) + sizeof(*data_); @@ -604,6 +626,7 @@ MaybeLocal GetMessagePortConstructor( env->SetProtoMethod(m, "postMessage", MessagePort::PostMessage); env->SetProtoMethod(m, "start", MessagePort::Start); env->SetProtoMethod(m, "stop", MessagePort::Stop); + env->SetProtoMethod(m, "drain", MessagePort::Drain); env->SetProtoMethod(m, "close", HandleWrap::Close); env->SetProtoMethod(m, "unref", HandleWrap::Unref); env->SetProtoMethod(m, "ref", HandleWrap::Ref); diff --git a/src/node_messaging.h b/src/node_messaging.h index ff8fcc72439e9f..9a13437d19a331 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -133,11 +133,15 @@ class MessagePort : public HandleWrap { void Start(); // Stop processing messages on this port as a receiving end. void Stop(); + // Stop processing messages on this port as a receiving end, + // and stop the event loop that this port is associated with. + void StopEventLoop(); static void New(const v8::FunctionCallbackInfo& args); static void PostMessage(const v8::FunctionCallbackInfo& args); static void Start(const v8::FunctionCallbackInfo& args); static void Stop(const v8::FunctionCallbackInfo& args); + static void Drain(const v8::FunctionCallbackInfo& args); // Turns `a` and `b` into siblings, i.e. connects the sending side of one // to the receiving side of the other. This is not thread-safe. @@ -160,6 +164,7 @@ class MessagePort : public HandleWrap { inline uv_async_t* async(); std::unique_ptr data_ = nullptr; + bool stop_event_loop_ = false; friend class MessagePortData; }; diff --git a/src/node_worker.cc b/src/node_worker.cc new file mode 100644 index 00000000000000..366dca353d345c --- /dev/null +++ b/src/node_worker.cc @@ -0,0 +1,428 @@ +#include "node_worker.h" +#include "node_errors.h" +#include "node_internals.h" +#include "node_buffer.h" +#include "node_perf.h" +#include "util.h" +#include "util-inl.h" +#include "async_wrap.h" +#include "async_wrap-inl.h" + +using v8::ArrayBuffer; +using v8::Context; +using v8::Function; +using v8::FunctionCallbackInfo; +using v8::FunctionTemplate; +using v8::HandleScope; +using v8::Integer; +using v8::Isolate; +using v8::Local; +using v8::Locker; +using v8::Number; +using v8::Object; +using v8::SealHandleScope; +using v8::String; +using v8::Value; + +namespace node { +namespace worker { + +namespace { + +double next_thread_id = 1; +Mutex next_thread_id_mutex; + +} // anonymous namespace + +Worker::Worker(Environment* env, Local wrap) + : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER) { + // Generate a new thread id. + { + Mutex::ScopedLock next_thread_id_lock(next_thread_id_mutex); + thread_id_ = next_thread_id++; + } + wrap->Set(env->context(), + env->thread_id_string(), + Number::New(env->isolate(), thread_id_)).FromJust(); + + // Set up everything that needs to be set up in the parent environment. + parent_port_ = MessagePort::New(env, env->context()); + if (parent_port_ == nullptr) { + // This can happen e.g. because execution is terminating. + return; + } + + child_port_data_.reset(new MessagePortData(nullptr)); + MessagePort::Entangle(parent_port_, child_port_data_.get()); + + object()->Set(env->context(), + env->message_port_string(), + parent_port_->object()).FromJust(); + + array_buffer_allocator_.reset(CreateArrayBufferAllocator()); + + isolate_ = NewIsolate(array_buffer_allocator_.get()); + CHECK_NE(isolate_, nullptr); + CHECK_EQ(uv_loop_init(&loop_), 0); + + thread_exit_async_.reset(new uv_async_t); + thread_exit_async_->data = this; + CHECK_EQ(uv_async_init(env->event_loop(), + thread_exit_async_.get(), + [](uv_async_t* handle) { + static_cast(handle->data)->OnThreadStopped(); + }), 0); + + { + // Enter an environment capable of executing code in the child Isolate + // (and only in it). + Locker locker(isolate_); + Isolate::Scope isolate_scope(isolate_); + HandleScope handle_scope(isolate_); + + isolate_data_.reset(CreateIsolateData(isolate_, + &loop_, + env->isolate_data()->platform(), + array_buffer_allocator_.get())); + CHECK(isolate_data_); + + Local context = NewContext(isolate_); + Context::Scope context_scope(context); + + // TODO(addaleax): Use CreateEnvironment(), or generally another public API. + env_.reset(new Environment(isolate_data_.get(), + context, + nullptr)); + CHECK_NE(env_, nullptr); + env_->set_abort_on_uncaught_exception(false); + env_->set_worker_context(this); + env_->set_thread_id(thread_id_); + + env_->Start(0, nullptr, 0, nullptr, env->profiler_idle_notifier_started()); + } + + // The new isolate won't be bothered on this thread again. + isolate_->DiscardThreadSpecificMetadata(); +} + +bool Worker::is_stopped() const { + Mutex::ScopedLock stopped_lock(stopped_mutex_); + return stopped_; +} + +void Worker::Run() { + MultiIsolatePlatform* platform = isolate_data_->platform(); + CHECK_NE(platform, nullptr); + + { + Locker locker(isolate_); + Isolate::Scope isolate_scope(isolate_); + SealHandleScope outer_seal(isolate_); + + { + Context::Scope context_scope(env_->context()); + HandleScope handle_scope(isolate_); + + { + HandleScope handle_scope(isolate_); + Mutex::ScopedLock lock(mutex_); + // Set up the message channel for receiving messages in the child. + child_port_ = MessagePort::New(env_.get(), + env_->context(), + std::move(child_port_data_)); + // MessagePort::New() may return nullptr if execution is terminated + // within it. + if (child_port_ != nullptr) + env_->set_message_port(child_port_->object(isolate_)); + } + + if (!is_stopped()) { + HandleScope handle_scope(isolate_); + Environment::AsyncCallbackScope callback_scope(env_.get()); + env_->async_hooks()->push_async_ids(1, 0); + // This loads the Node bootstrapping code. + LoadEnvironment(env_.get()); + env_->async_hooks()->pop_async_id(1); + } + + { + SealHandleScope seal(isolate_); + bool more; + env_->performance_state()->Mark( + node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_START); + do { + if (is_stopped()) break; + uv_run(&loop_, UV_RUN_DEFAULT); + if (is_stopped()) break; + + platform->DrainBackgroundTasks(isolate_); + + more = uv_loop_alive(&loop_); + if (more && !is_stopped()) + continue; + + EmitBeforeExit(env_.get()); + + // Emit `beforeExit` if the loop became alive either after emitting + // event, or after running some callbacks. + more = uv_loop_alive(&loop_); + } while (more == true); + env_->performance_state()->Mark( + node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT); + } + } + + { + int exit_code; + bool stopped = is_stopped(); + if (!stopped) + exit_code = EmitExit(env_.get()); + Mutex::ScopedLock lock(mutex_); + if (exit_code_ == 0 && !stopped) + exit_code_ = exit_code; + } + + env_->set_can_call_into_js(false); + Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_, + Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); + + // Grab the parent-to-child channel and render is unusable. + MessagePort* child_port; + { + Mutex::ScopedLock lock(mutex_); + child_port = child_port_; + child_port_ = nullptr; + } + + { + Context::Scope context_scope(env_->context()); + child_port->Close(); + env_->stop_sub_worker_contexts(); + env_->RunCleanup(); + RunAtExit(env_.get()); + + { + Mutex::ScopedLock stopped_lock(stopped_mutex_); + stopped_ = true; + } + + env_->RunCleanup(); + + // This call needs to be made while the `Environment` is still alive + // because we assume that it is available for async tracking in the + // NodePlatform implementation. + platform->DrainBackgroundTasks(isolate_); + } + + env_.reset(); + } + + DisposeIsolate(); + + // Need to run the loop one more time to close the platform's uv_async_t + uv_run(&loop_, UV_RUN_ONCE); + + { + Mutex::ScopedLock lock(mutex_); + CHECK(thread_exit_async_); + scheduled_on_thread_stopped_ = true; + uv_async_send(thread_exit_async_.get()); + } +} + +void Worker::DisposeIsolate() { + if (isolate_ == nullptr) + return; + + CHECK(isolate_data_); + MultiIsolatePlatform* platform = isolate_data_->platform(); + platform->CancelPendingDelayedTasks(isolate_); + + isolate_data_.reset(); + + isolate_->Dispose(); + isolate_ = nullptr; +} + +void Worker::JoinThread() { + if (thread_joined_) + return; + CHECK_EQ(uv_thread_join(&tid_), 0); + thread_joined_ = true; + + env()->remove_sub_worker_context(this); + + if (thread_exit_async_) { + env()->CloseHandle(thread_exit_async_.release(), [](uv_async_t* async) { + delete async; + }); + + if (scheduled_on_thread_stopped_) + OnThreadStopped(); + } +} + +void Worker::OnThreadStopped() { + Mutex::ScopedLock lock(mutex_); + scheduled_on_thread_stopped_ = false; + + { + Mutex::ScopedLock stopped_lock(stopped_mutex_); + CHECK(stopped_); + } + + CHECK_EQ(child_port_, nullptr); + parent_port_ = nullptr; + + // It's okay to join the thread while holding the mutex because + // OnThreadStopped means it's no longer doing any work that might grab it + // and really just silently exiting. + JoinThread(); + + { + HandleScope handle_scope(env()->isolate()); + Context::Scope context_scope(env()->context()); + + // Reset the parent port as we're closing it now anyway. + object()->Set(env()->context(), + env()->message_port_string(), + Undefined(env()->isolate())).FromJust(); + + Local code = Integer::New(env()->isolate(), exit_code_); + MakeCallback(env()->onexit_string(), 1, &code); + } + + // JoinThread() cleared all libuv handles bound to this Worker, + // the C++ object is no longer needed for anything now. + MakeWeak(); +} + +Worker::~Worker() { + Mutex::ScopedLock lock(mutex_); + JoinThread(); + + CHECK(stopped_); + CHECK(thread_joined_); + CHECK_EQ(child_port_, nullptr); + CHECK_EQ(uv_loop_close(&loop_), 0); + + // This has most likely already happened within the worker thread -- this + // is just in case Worker creation failed early. + DisposeIsolate(); +} + +void Worker::New(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + CHECK(args.IsConstructCall()); + + if (env->isolate_data()->platform() == nullptr) { + THROW_ERR_MISSING_PLATFORM_FOR_WORKER(env); + return; + } + + new Worker(env, args.This()); +} + +void Worker::StartThread(const FunctionCallbackInfo& args) { + Worker* w; + ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); + Mutex::ScopedLock lock(w->mutex_); + + w->env()->add_sub_worker_context(w); + w->stopped_ = false; + CHECK_EQ(uv_thread_create(&w->tid_, [](void* arg) { + static_cast(arg)->Run(); + }, static_cast(w)), 0); + w->thread_joined_ = false; +} + +void Worker::StopThread(const FunctionCallbackInfo& args) { + Worker* w; + ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); + + w->Exit(1); + w->JoinThread(); +} + +void Worker::Ref(const FunctionCallbackInfo& args) { + Worker* w; + ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); + if (w->thread_exit_async_) + uv_ref(reinterpret_cast(w->thread_exit_async_.get())); +} + +void Worker::Unref(const FunctionCallbackInfo& args) { + Worker* w; + ASSIGN_OR_RETURN_UNWRAP(&w, args.This()); + if (w->thread_exit_async_) + uv_unref(reinterpret_cast(w->thread_exit_async_.get())); +} + +void Worker::Exit(int code) { + Mutex::ScopedLock lock(mutex_); + Mutex::ScopedLock stopped_lock(stopped_mutex_); + if (!stopped_) { + CHECK_NE(env_, nullptr); + stopped_ = true; + exit_code_ = code; + if (child_port_ != nullptr) + child_port_->StopEventLoop(); + isolate_->TerminateExecution(); + } +} + +size_t Worker::self_size() const { + return sizeof(*this); +} + +namespace { + +// Return the MessagePort that is global for this Environment and communicates +// with the internal [kPort] port of the JS Worker class in the parent thread. +void GetEnvMessagePort(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Local port = env->message_port(); + if (!port.IsEmpty()) { + CHECK_EQ(port->CreationContext()->GetIsolate(), args.GetIsolate()); + args.GetReturnValue().Set(port); + } +} + +void InitWorker(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + + { + Local w = env->NewFunctionTemplate(Worker::New); + + w->InstanceTemplate()->SetInternalFieldCount(1); + + AsyncWrap::AddWrapMethods(env, w); + env->SetProtoMethod(w, "startThread", Worker::StartThread); + env->SetProtoMethod(w, "stopThread", Worker::StopThread); + env->SetProtoMethod(w, "ref", Worker::Ref); + env->SetProtoMethod(w, "unref", Worker::Unref); + + Local workerString = + FIXED_ONE_BYTE_STRING(env->isolate(), "Worker"); + w->SetClassName(workerString); + target->Set(workerString, w->GetFunction()); + } + + env->SetMethod(target, "getEnvMessagePort", GetEnvMessagePort); + + auto thread_id_string = FIXED_ONE_BYTE_STRING(env->isolate(), "threadId"); + target->Set(env->context(), + thread_id_string, + Number::New(env->isolate(), env->thread_id())).FromJust(); +} + +} // anonymous namespace + +} // namespace worker +} // namespace node + +NODE_MODULE_CONTEXT_AWARE_INTERNAL(worker, node::worker::InitWorker) diff --git a/src/node_worker.h b/src/node_worker.h new file mode 100644 index 00000000000000..0a98d2f11ef00f --- /dev/null +++ b/src/node_worker.h @@ -0,0 +1,83 @@ +#ifndef SRC_NODE_WORKER_H_ +#define SRC_NODE_WORKER_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include "node_messaging.h" +#include + +namespace node { +namespace worker { + +// A worker thread, as represented in its parent thread. +class Worker : public AsyncWrap { + public: + Worker(Environment* env, v8::Local wrap); + ~Worker(); + + // Run the worker. This is only called from the worker thread. + void Run(); + + // Forcibly exit the thread with a specified exit code. This may be called + // from any thread. + void Exit(int code); + + // Wait for the worker thread to stop (in a blocking manner). + void JoinThread(); + + size_t self_size() const override; + bool is_stopped() const; + + static void New(const v8::FunctionCallbackInfo& args); + static void StartThread(const v8::FunctionCallbackInfo& args); + static void StopThread(const v8::FunctionCallbackInfo& args); + static void GetMessagePort(const v8::FunctionCallbackInfo& args); + static void Ref(const v8::FunctionCallbackInfo& args); + static void Unref(const v8::FunctionCallbackInfo& args); + + private: + void OnThreadStopped(); + void DisposeIsolate(); + + uv_loop_t loop_; + DeleteFnPtr isolate_data_; + DeleteFnPtr env_; + v8::Isolate* isolate_ = nullptr; + DeleteFnPtr + array_buffer_allocator_; + uv_thread_t tid_; + + // This mutex protects access to all variables listed below it. + mutable Mutex mutex_; + + // Currently only used for telling the parent thread that the child + // thread exited. + std::unique_ptr thread_exit_async_; + bool scheduled_on_thread_stopped_ = false; + + // This mutex only protects stopped_. If both locks are acquired, this needs + // to be the latter one. + mutable Mutex stopped_mutex_; + bool stopped_ = true; + + bool thread_joined_ = true; + int exit_code_ = 0; + double thread_id_ = -1; + + std::unique_ptr child_port_data_; + + // The child port is always kept alive by the child Environment's persistent + // handle to it. + MessagePort* child_port_ = nullptr; + // This is always kept alive because the JS object associated with the Worker + // instance refers to it via its [kPort] property. + MessagePort* parent_port_ = nullptr; +}; + +} // namespace worker +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + + +#endif // SRC_NODE_WORKER_H_ diff --git a/test/fixtures/worker-script.mjs b/test/fixtures/worker-script.mjs new file mode 100644 index 00000000000000..b712248b2788e8 --- /dev/null +++ b/test/fixtures/worker-script.mjs @@ -0,0 +1,3 @@ +import worker from 'worker'; + +worker.parentPort.postMessage('Hello, world!'); diff --git a/test/parallel/test-message-channel-sharedarraybuffer.js b/test/parallel/test-message-channel-sharedarraybuffer.js new file mode 100644 index 00000000000000..7ae922adbc4e40 --- /dev/null +++ b/test/parallel/test-message-channel-sharedarraybuffer.js @@ -0,0 +1,28 @@ +// Flags: --expose-gc --experimental-worker +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker'); + +{ + const sharedArrayBuffer = new SharedArrayBuffer(12); + const local = Buffer.from(sharedArrayBuffer); + + const w = new Worker(` + const { parentPort } = require('worker'); + parentPort.on('message', ({ sharedArrayBuffer }) => { + const local = Buffer.from(sharedArrayBuffer); + local.write('world!', 6); + parentPort.postMessage('written!'); + }); + `, { eval: true }); + w.on('message', common.mustCall(() => { + assert.strictEqual(local.toString(), 'Hello world!'); + global.gc(); + w.terminate(); + })); + w.postMessage({ sharedArrayBuffer }); + // This would be a race condition if the memory regions were overlapping + local.write('Hello '); +} diff --git a/test/parallel/test-message-channel.js b/test/parallel/test-message-channel.js index 0facaa1d835ea8..eb13fa57c6aa0f 100644 --- a/test/parallel/test-message-channel.js +++ b/test/parallel/test-message-channel.js @@ -2,7 +2,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const { MessageChannel } = require('worker'); +const { MessageChannel, MessagePort, Worker } = require('worker'); { const channel = new MessageChannel(); @@ -24,3 +24,23 @@ const { MessageChannel } = require('worker'); channel.port2.on('close', common.mustCall()); channel.port2.close(); } + +{ + const channel = new MessageChannel(); + + const w = new Worker(` + const { MessagePort } = require('worker'); + const assert = require('assert'); + require('worker').parentPort.on('message', ({ port }) => { + assert(port instanceof MessagePort); + port.postMessage('works'); + }); + `, { eval: true }); + w.postMessage({ port: channel.port2 }, [ channel.port2 ]); + assert(channel.port1 instanceof MessagePort); + assert(channel.port2 instanceof MessagePort); + channel.port1.on('message', common.mustCall((message) => { + assert.strictEqual(message, 'works'); + w.terminate(); + })); +} diff --git a/test/parallel/test-worker-cleanup-handles.js b/test/parallel/test-worker-cleanup-handles.js new file mode 100644 index 00000000000000..ba4f6aa51a9d41 --- /dev/null +++ b/test/parallel/test-worker-cleanup-handles.js @@ -0,0 +1,30 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread, parentPort } = require('worker'); +const { Server } = require('net'); +const fs = require('fs'); + +if (isMainThread) { + const w = new Worker(__filename); + let fd = null; + w.on('message', common.mustCall((fd_) => { + assert.strictEqual(typeof fd_, 'number'); + fd = fd_; + })); + w.on('exit', common.mustCall((code) => { + if (fd === -1) { + // This happens when server sockets don’t have file descriptors, + // i.e. on Windows. + return; + } + common.expectsError(() => fs.fstatSync(fd), + { code: 'EBADF' }); + })); +} else { + const server = new Server(); + server.listen(0); + parentPort.postMessage(server._handle.fd); + server.unref(); +} diff --git a/test/parallel/test-worker-dns-terminate.js b/test/parallel/test-worker-dns-terminate.js new file mode 100644 index 00000000000000..079a29d52e09a3 --- /dev/null +++ b/test/parallel/test-worker-dns-terminate.js @@ -0,0 +1,15 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const { Worker } = require('worker'); + +const w = new Worker(` +const dns = require('dns'); +dns.lookup('nonexistent.org', () => {}); +require('worker').parentPort.postMessage('0'); +`, { eval: true }); + +w.on('message', common.mustCall(() => { + // This should not crash the worker during a DNS request. + w.terminate(common.mustCall()); +})); diff --git a/test/parallel/test-worker-esmodule.js b/test/parallel/test-worker-esmodule.js new file mode 100644 index 00000000000000..4189eeca3f8908 --- /dev/null +++ b/test/parallel/test-worker-esmodule.js @@ -0,0 +1,11 @@ +// Flags: --experimental-worker --experimental-modules +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { Worker } = require('worker'); + +const w = new Worker(fixtures.path('worker-script.mjs')); +w.on('message', common.mustCall((message) => { + assert.strictEqual(message, 'Hello, world!'); +})); diff --git a/test/parallel/test-worker-memory.js b/test/parallel/test-worker-memory.js new file mode 100644 index 00000000000000..34b1e0acaf2f2f --- /dev/null +++ b/test/parallel/test-worker-memory.js @@ -0,0 +1,41 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const util = require('util'); +const { Worker } = require('worker'); + +const numWorkers = +process.env.JOBS || require('os').cpus().length; + +// Verify that a Worker's memory isn't kept in memory after the thread finishes. + +function run(n, done) { + if (n <= 0) + return done(); + const worker = new Worker( + 'require(\'worker\').parentPort.postMessage(2 + 2)', + { eval: true }); + worker.on('message', common.mustCall((value) => { + assert.strictEqual(value, 4); + })); + worker.on('exit', common.mustCall(() => { + run(n - 1, done); + })); +} + +const startStats = process.memoryUsage(); +let finished = 0; +for (let i = 0; i < numWorkers; ++i) { + run(60 / numWorkers, () => { + if (++finished === numWorkers) { + const finishStats = process.memoryUsage(); + // A typical value for this ratio would be ~1.15. + // 5 as a upper limit is generous, but the main point is that we + // don't have the memory of 50 Isolates/Node.js environments just lying + // around somewhere. + assert.ok(finishStats.rss / startStats.rss < 5, + 'Unexpected memory overhead: ' + + util.inspect([startStats, finishStats])); + } + }); +} diff --git a/test/parallel/test-worker-nexttick-terminate.js b/test/parallel/test-worker-nexttick-terminate.js new file mode 100644 index 00000000000000..b010a7dbe5727f --- /dev/null +++ b/test/parallel/test-worker-nexttick-terminate.js @@ -0,0 +1,20 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const { Worker } = require('worker'); + +// Checks that terminating in the middle of `process.nextTick()` does not +// Crash the process. + +const w = new Worker(` +require('worker').parentPort.postMessage('0'); +process.nextTick(() => { + while(1); +}); +`, { eval: true }); + +w.on('message', common.mustCall(() => { + setTimeout(() => { + w.terminate(common.mustCall()); + }, 1); +})); diff --git a/test/parallel/test-worker-syntax-error-file.js b/test/parallel/test-worker-syntax-error-file.js new file mode 100644 index 00000000000000..37798f334387d8 --- /dev/null +++ b/test/parallel/test-worker-syntax-error-file.js @@ -0,0 +1,18 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { Worker } = require('worker'); + +// 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(fixtures.path('syntax', 'bad_syntax.js')); + w.on('message', common.mustNotCall()); + w.on('error', common.mustCall((err) => { + assert(/SyntaxError/.test(err)); + })); +} else { + throw new Error('foo'); +} diff --git a/test/parallel/test-worker-syntax-error.js b/test/parallel/test-worker-syntax-error.js new file mode 100644 index 00000000000000..8f9812a721132b --- /dev/null +++ b/test/parallel/test-worker-syntax-error.js @@ -0,0 +1,17 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker'); + +// 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('abc)', { eval: true }); + w.on('message', common.mustNotCall()); + w.on('error', common.mustCall((err) => { + assert(/SyntaxError/.test(err)); + })); +} else { + throw new Error('foo'); +} diff --git a/test/parallel/test-worker-uncaught-exception-async.js b/test/parallel/test-worker-uncaught-exception-async.js new file mode 100644 index 00000000000000..c1d2a5f4fcab16 --- /dev/null +++ b/test/parallel/test-worker-uncaught-exception-async.js @@ -0,0 +1,20 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker'); + +// 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); + w.on('message', common.mustNotCall()); + w.on('error', common.mustCall((err) => { + // TODO(addaleax): be more specific here + assert(/foo/.test(err)); + })); +} else { + setImmediate(() => { + throw new Error('foo'); + }); +} diff --git a/test/parallel/test-worker-uncaught-exception.js b/test/parallel/test-worker-uncaught-exception.js new file mode 100644 index 00000000000000..b0e3ad11fae839 --- /dev/null +++ b/test/parallel/test-worker-uncaught-exception.js @@ -0,0 +1,18 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker'); + +// 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); + w.on('message', common.mustNotCall()); + w.on('error', common.mustCall((err) => { + // TODO(addaleax): be more specific here + assert(/foo/.test(err)); + })); +} else { + throw new Error('foo'); +} diff --git a/test/parallel/test-worker.js b/test/parallel/test-worker.js new file mode 100644 index 00000000000000..3fa6e67a347b37 --- /dev/null +++ b/test/parallel/test-worker.js @@ -0,0 +1,18 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread, parentPort } = require('worker'); + +if (isMainThread) { + const w = new Worker(__filename); + w.on('message', common.mustCall((message) => { + assert.strictEqual(message, 'Hello, world!'); + })); +} else { + setImmediate(() => { + process.nextTick(() => { + parentPort.postMessage('Hello, world!'); + }); + }); +} diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 84a3e3b1f4dc05..af08d7b6567018 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -38,6 +38,7 @@ common.crashOnUnhandledRejection(); // TODO(addaleax): Test for these delete providers.STREAMPIPE; delete providers.MESSAGEPORT; + delete providers.WORKER; const objKeys = Object.keys(providers); if (objKeys.length > 0) From 93ce63c89fd05b720d426ba5082fe4d1a7ceeb3e Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 25 Sep 2017 16:30:09 -0700 Subject: [PATCH 028/150] test: add test against unsupported worker features Refs: https://github.com/ayojs/ayo/pull/113 Reviewed-By: Anna Henningsen PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- .../test-worker-unsupported-things.js | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/parallel/test-worker-unsupported-things.js diff --git a/test/parallel/test-worker-unsupported-things.js b/test/parallel/test-worker-unsupported-things.js new file mode 100644 index 00000000000000..9f407a6806a7c9 --- /dev/null +++ b/test/parallel/test-worker-unsupported-things.js @@ -0,0 +1,41 @@ +// Flags: --experimental-worker +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker, isMainThread, parentPort } = require('worker'); + +if (isMainThread) { + const w = new Worker(__filename); + w.on('message', common.mustCall((message) => { + assert.strictEqual(message, true); + })); +} else { + { + const before = process.title; + process.title += ' in worker'; + assert.strictEqual(process.title, before); + } + + { + const before = process.debugPort; + process.debugPort++; + assert.strictEqual(process.debugPort, before); + } + + assert.strictEqual('abort' in process, false); + assert.strictEqual('chdir' in process, false); + assert.strictEqual('setuid' in process, false); + assert.strictEqual('seteuid' in process, false); + assert.strictEqual('setgid' in process, false); + assert.strictEqual('setegid' in process, false); + assert.strictEqual('setgroups' in process, false); + assert.strictEqual('initgroups' in process, false); + + assert.strictEqual('_startProfilerIdleNotifier' in process, false); + assert.strictEqual('_stopProfilerIdleNotifier' in process, false); + assert.strictEqual('_debugProcess' in process, false); + assert.strictEqual('_debugPause' in process, false); + assert.strictEqual('_debugEnd' in process, false); + + parentPort.postMessage(true); +} From c97fb91e5510bb1ec194280c628d6fe7c67955d5 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Wed, 20 Sep 2017 14:23:31 -0700 Subject: [PATCH 029/150] worker: restrict supported extensions Only allow `.js` and `.mjs` extensions to provide future-proofing for file type detection. Refs: https://github.com/ayojs/ayo/pull/117 Reviewed-By: Stephen Belanger Reviewed-By: Olivia Hugger Reviewed-By: Anna Henningsen PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- lib/internal/errors.js | 3 +++ lib/internal/worker.js | 13 ++++++--- test/parallel/test-worker-unsupported-path.js | 27 +++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-worker-unsupported-path.js diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 1fb90b37f88642..59838f2e6adbf8 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -856,4 +856,7 @@ E('ERR_WORKER_NEED_ABSOLUTE_PATH', TypeError); E('ERR_WORKER_UNSERIALIZABLE_ERROR', 'Serializing an uncaught exception failed', Error); +E('ERR_WORKER_UNSUPPORTED_EXTENSION', + 'The worker script extension must be ".js" or ".mjs". Received "%s"', + TypeError); E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error); diff --git a/lib/internal/worker.js b/lib/internal/worker.js index c982478b9334e8..edd954d8a3a2be 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -8,7 +8,8 @@ const util = require('util'); const { ERR_INVALID_ARG_TYPE, ERR_WORKER_NEED_ABSOLUTE_PATH, - ERR_WORKER_UNSERIALIZABLE_ERROR + ERR_WORKER_UNSERIALIZABLE_ERROR, + ERR_WORKER_UNSUPPORTED_EXTENSION, } = require('internal/errors').codes; const { internalBinding } = require('internal/bootstrap/loaders'); @@ -136,8 +137,14 @@ class Worker extends EventEmitter { throw new ERR_INVALID_ARG_TYPE('filename', 'string', filename); } - if (!options.eval && !path.isAbsolute(filename)) { - throw new ERR_WORKER_NEED_ABSOLUTE_PATH(filename); + if (!options.eval) { + if (!path.isAbsolute(filename)) { + throw new ERR_WORKER_NEED_ABSOLUTE_PATH(filename); + } + const ext = path.extname(filename); + if (ext !== '.js' && ext !== '.mjs') { + throw new ERR_WORKER_UNSUPPORTED_EXTENSION(ext); + } } // Set up the C++ handle for the worker, as well as some internal wiring. diff --git a/test/parallel/test-worker-unsupported-path.js b/test/parallel/test-worker-unsupported-path.js new file mode 100644 index 00000000000000..3716377ec2fb1f --- /dev/null +++ b/test/parallel/test-worker-unsupported-path.js @@ -0,0 +1,27 @@ +// Flags: --experimental-worker +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker'); + +{ + const expectedErr = common.expectsError({ + code: 'ERR_WORKER_NEED_ABSOLUTE_PATH', + type: TypeError + }, 4); + assert.throws(() => { new Worker('a.js'); }, expectedErr); + assert.throws(() => { new Worker('b'); }, expectedErr); + assert.throws(() => { new Worker('c/d.js'); }, expectedErr); + assert.throws(() => { new Worker('a.mjs'); }, expectedErr); +} + +{ + const expectedErr = common.expectsError({ + code: 'ERR_WORKER_UNSUPPORTED_EXTENSION', + type: TypeError + }, 3); + assert.throws(() => { new Worker('/b'); }, expectedErr); + assert.throws(() => { new Worker('/c.wasm'); }, expectedErr); + assert.throws(() => { new Worker('/d.txt'); }, expectedErr); +} From 6b1a887aa2fc7197fa787934188d24cd0dd574e7 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 13 May 2018 23:25:14 +0200 Subject: [PATCH 030/150] worker: enable stdio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provide `stdin`, `stdout` and `stderr` options for the `Worker` constructor, and make these available to the worker thread under their usual names. The default for `stdin` is an empty stream, the default for `stdout` and `stderr` is redirecting to the parent thread’s corresponding stdio streams. PR-URL: https://github.com/nodejs/node/pull/20876 Reviewed-By: Gireesh Punathil Reviewed-By: Benjamin Gruenbaum Reviewed-By: Shingo Inoue Reviewed-By: Matteo Collina Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: John-David Dalton Reviewed-By: Gus Caplan --- doc/api/worker.md | 44 +++++++- lib/internal/process/stdio.js | 14 +-- lib/internal/worker.js | 166 ++++++++++++++++++++++++++++- test/parallel/test-worker-stdio.js | 43 ++++++++ 4 files changed, 256 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-worker-stdio.js diff --git a/doc/api/worker.md b/doc/api/worker.md index 3517a4c86ac767..2fa55cfa2ddadf 100644 --- a/doc/api/worker.md +++ b/doc/api/worker.md @@ -240,7 +240,7 @@ Most Node.js APIs are available inside of it. Notable differences inside a Worker environment are: - The [`process.stdin`][], [`process.stdout`][] and [`process.stderr`][] - properties are set to `null`. + may be redirected by the parent thread. - The [`require('worker').isMainThread`][] property is set to `false`. - The [`require('worker').parentPort`][] message port is available, - [`process.exit()`][] does not stop the whole program, just the single thread, @@ -313,6 +313,13 @@ if (isMainThread) { described in the [HTML structured clone algorithm][], and an error will be thrown if the object cannot be cloned (e.g. because it contains `function`s). + * stdin {boolean} If this is set to `true`, then `worker.stdin` will + provide a writable stream whose contents will appear as `process.stdin` + inside the Worker. By default, no data is provided. + * stdout {boolean} If this is set to `true`, then `worker.stdout` will + not automatically be piped through to `process.stdout` in the parent. + * stderr {boolean} If this is set to `true`, then `worker.stderr` will + not automatically be piped through to `process.stderr` in the parent. ### Event: 'error' + +* {stream.Readable} + +This is a readable stream which contains data written to [`process.stderr`][] +inside the worker thread. If `stderr: true` was not passed to the +[`Worker`][] constructor, then data will be piped to the parent thread's +[`process.stderr`][] stream. + +### worker.stdin + + +* {null|stream.Writable} + +If `stdin: true` was passed to the [`Worker`][] constructor, this is a +writable stream. The data written to this stream will be made available in +the worker thread as [`process.stdin`][]. + +### worker.stdout + + +* {stream.Readable} + +This is a readable stream which contains data written to [`process.stdout`][] +inside the worker thread. If `stdout: true` was not passed to the +[`Worker`][] constructor, then data will be piped to the parent thread's +[`process.stdout`][] stream. + ### worker.terminate([callback]) @@ -9,7 +9,7 @@ on independent threads, and to create message channels between them. It can be accessed using: ```js -const worker = require('worker'); +const worker = require('worker_threads'); ``` Workers are useful for performing CPU-intensive JavaScript operations; do not @@ -23,7 +23,9 @@ share memory efficiently by transferring `ArrayBuffer` instances or sharing ## Example ```js -const { Worker, isMainThread, parentPort, workerData } = require('worker'); +const { + Worker, isMainThread, parentPort, workerData +} = require('worker_threads'); if (isMainThread) { module.exports = async function parseJSAsync(script) { @@ -104,7 +106,7 @@ yields an object with `port1` and `port2` properties, which refer to linked [`MessagePort`][] instances. ```js -const { MessageChannel } = require('worker'); +const { MessageChannel } = require('worker_threads'); const { port1, port2 } = new MessageChannel(); port1.on('message', (message) => console.log('received', message)); @@ -241,8 +243,8 @@ Notable differences inside a Worker environment are: - The [`process.stdin`][], [`process.stdout`][] and [`process.stderr`][] may be redirected by the parent thread. -- The [`require('worker').isMainThread`][] property is set to `false`. -- The [`require('worker').parentPort`][] message port is available, +- The [`require('worker_threads').isMainThread`][] property is set to `false`. +- The [`require('worker_threads').parentPort`][] message port is available, - [`process.exit()`][] does not stop the whole program, just the single thread, and [`process.abort()`][] is not available. - [`process.chdir()`][] and `process` methods that set group or user ids @@ -283,7 +285,9 @@ For example: ```js const assert = require('assert'); -const { Worker, MessageChannel, MessagePort, isMainThread } = require('worker'); +const { + Worker, MessageChannel, MessagePort, isMainThread +} = require('worker_threads'); if (isMainThread) { const worker = new Worker(__filename); const subChannel = new MessageChannel(); @@ -292,7 +296,7 @@ if (isMainThread) { console.log('received:', value); }); } else { - require('worker').once('workerMessage', (value) => { + require('worker_threads').once('workerMessage', (value) => { assert(value.hereIsYourPort instanceof MessagePort); value.hereIsYourPort.postMessage('the worker is sending this'); value.hereIsYourPort.close(); @@ -309,9 +313,9 @@ if (isMainThread) { * `eval` {boolean} If true, interpret the first argument to the constructor as a script that is executed once the worker is online. * `data` {any} Any JavaScript value that will be cloned and made - available as [`require('worker').workerData`][]. The cloning will occur as - described in the [HTML structured clone algorithm][], and an error will be - thrown if the object cannot be cloned (e.g. because it contains + available as [`require('worker_threads').workerData`][]. The cloning will + occur as described in the [HTML structured clone algorithm][], and an error + will be thrown if the object cannot be cloned (e.g. because it contains `function`s). * stdin {boolean} If this is set to `true`, then `worker.stdin` will provide a writable stream whose contents will appear as `process.stdin` @@ -351,8 +355,8 @@ added: REPLACEME * `value` {any} The transmitted value The `'message'` event is emitted when the worker thread has invoked -[`require('worker').postMessage()`][]. See the [`port.on('message')`][] event -for more details. +[`require('worker_threads').postMessage()`][]. See the [`port.on('message')`][] +event for more details. ### Event: 'online' * `options` {Object} - * `pfx` {string|string[]|Buffer|Buffer[]|Object[]} Optional PFX or PKCS12 - encoded private key and certificate chain. `pfx` is an alternative to - providing `key` and `cert` individually. PFX is usually encrypted, if it is, - `passphrase` will be used to decrypt it. Multiple PFX can be provided either - as an array of unencrypted PFX buffers, or an array of objects in the form - `{buf: [, passphrase: ]}`. The object form can only - occur in an array. `object.passphrase` is optional. Encrypted PFX will be - decrypted with `object.passphrase` if provided, or `options.passphrase` if - it is not. - * `key` {string|string[]|Buffer|Buffer[]|Object[]} Optional private keys in - PEM format. PEM allows the option of private keys being encrypted. Encrypted - keys will be decrypted with `options.passphrase`. Multiple keys using - different algorithms can be provided either as an array of unencrypted key - strings or buffers, or an array of objects in the form `{pem: - [, passphrase: ]}`. The object form can only occur in - an array. `object.passphrase` is optional. Encrypted keys will be decrypted - with `object.passphrase` if provided, or `options.passphrase` if it is not. - * `passphrase` {string} Optional shared passphrase used for a single private - key and/or a PFX. - * `cert` {string|string[]|Buffer|Buffer[]} Optional cert chains in PEM format. - One cert chain should be provided per private key. Each cert chain should - consist of the PEM formatted certificate for a provided private `key`, - followed by the PEM formatted intermediate certificates (if any), in order, - and not including the root CA (the root CA must be pre-known to the peer, - see `ca`). When providing multiple cert chains, they do not have to be in - the same order as their private keys in `key`. If the intermediate - certificates are not provided, the peer will not be able to validate the - certificate, and the handshake will fail. * `ca` {string|string[]|Buffer|Buffer[]} Optionally override the trusted CA certificates. Default is to trust the well-known CAs curated by Mozilla. Mozilla's CAs are completely replaced when CAs are explicitly specified @@ -1067,19 +1039,17 @@ changes: certificate can match or chain to. For self-signed certificates, the certificate is its own CA, and must be provided. + * `cert` {string|string[]|Buffer|Buffer[]} Optional cert chains in PEM format. + One cert chain should be provided per private key. Each cert chain should + consist of the PEM formatted certificate for a provided private `key`, + followed by the PEM formatted intermediate certificates (if any), in order, + and not including the root CA (the root CA must be pre-known to the peer, + see `ca`). When providing multiple cert chains, they do not have to be in + the same order as their private keys in `key`. If the intermediate + certificates are not provided, the peer will not be able to validate the + certificate, and the handshake will fail. * `ciphers` {string} Optional cipher suite specification, replacing the default. For more information, see [modifying the default cipher suite][]. - * `honorCipherOrder` {boolean} Attempt to use the server's cipher suite - preferences instead of the client's. When `true`, causes - `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see - [OpenSSL Options][] for more information. - * `ecdhCurve` {string} A string describing a named curve or a colon separated - list of curve NIDs or names, for example `P-521:P-384:P-256`, to use for - ECDH key agreement, or `false` to disable ECDH. Set to `auto` to select the - curve automatically. Use [`crypto.getCurves()`][] to obtain a list of - available curve names. On recent releases, `openssl ecparam -list_curves` - will also display the name and description of each available elliptic curve. - **Default:** [`tls.DEFAULT_ECDH_CURVE`]. * `clientCertEngine` {string} Optional name of an OpenSSL engine which can provide the client certificate. * `crl` {string|string[]|Buffer|Buffer[]} Optional PEM formatted @@ -1090,6 +1060,36 @@ changes: error will be thrown. It is strongly recommended to use 2048 bits or larger for stronger security. If omitted or invalid, the parameters are silently discarded and DHE ciphers will not be available. + * `ecdhCurve` {string} A string describing a named curve or a colon separated + list of curve NIDs or names, for example `P-521:P-384:P-256`, to use for + ECDH key agreement, or `false` to disable ECDH. Set to `auto` to select the + curve automatically. Use [`crypto.getCurves()`][] to obtain a list of + available curve names. On recent releases, `openssl ecparam -list_curves` + will also display the name and description of each available elliptic curve. + **Default:** [`tls.DEFAULT_ECDH_CURVE`]. + * `honorCipherOrder` {boolean} Attempt to use the server's cipher suite + preferences instead of the client's. When `true`, causes + `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see + [OpenSSL Options][] for more information. + * `key` {string|string[]|Buffer|Buffer[]|Object[]} Optional private keys in + PEM format. PEM allows the option of private keys being encrypted. Encrypted + keys will be decrypted with `options.passphrase`. Multiple keys using + different algorithms can be provided either as an array of unencrypted key + strings or buffers, or an array of objects in the form `{pem: + [, passphrase: ]}`. The object form can only occur in + an array. `object.passphrase` is optional. Encrypted keys will be decrypted + with `object.passphrase` if provided, or `options.passphrase` if it is not. + * `passphrase` {string} Optional shared passphrase used for a single private + key and/or a PFX. + * `pfx` {string|string[]|Buffer|Buffer[]|Object[]} Optional PFX or PKCS12 + encoded private key and certificate chain. `pfx` is an alternative to + providing `key` and `cert` individually. PFX is usually encrypted, if it is, + `passphrase` will be used to decrypt it. Multiple PFX can be provided either + as an array of unencrypted PFX buffers, or an array of objects in the form + `{buf: [, passphrase: ]}`. The object form can only + occur in an array. `object.passphrase` is optional. Encrypted PFX will be + decrypted with `object.passphrase` if provided, or `options.passphrase` if + it is not. * `secureOptions` {number} Optionally affect the OpenSSL protocol behavior, which is not usually necessary. This should be used carefully if at all! Value is a numeric bitmask of the `SSL_OP_*` options from @@ -1133,38 +1133,38 @@ changes: --> * `options` {Object} + * `ALPNProtocols`: {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array} + An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or + `Uint8Array` containing the supported ALPN protocols. `Buffer`s should have + the format `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the + first byte is the length of the next protocol name. Passing an array is + usually much simpler, e.g. `['hello', 'world']`. + (Protocols should be ordered by their priority.) * `clientCertEngine` {string} Optional name of an OpenSSL engine which can provide the client certificate. * `handshakeTimeout` {number} Abort the connection if the SSL/TLS handshake does not finish in the specified number of milliseconds. A `'tlsClientError'` is emitted on the `tls.Server` object whenever a handshake times out. **Default:** `120000` (120 seconds). - * `requestCert` {boolean} If `true` the server will request a certificate from - clients that connect and attempt to verify that certificate. **Default:** - `false`. * `rejectUnauthorized` {boolean} If not `false` the server will reject any connection which is not authorized with the list of supplied CAs. This option only has an effect if `requestCert` is `true`. **Default:** `true`. - * `ALPNProtocols`: {string[]|Buffer[]|Uint8Array[]|Buffer|Uint8Array} - An array of strings, `Buffer`s or `Uint8Array`s, or a single `Buffer` or - `Uint8Array` containing the supported ALPN protocols. `Buffer`s should have - the format `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the - first byte is the length of the next protocol name. Passing an array is - usually much simpler, e.g. `['hello', 'world']`. - (Protocols should be ordered by their priority.) + * `requestCert` {boolean} If `true` the server will request a certificate from + clients that connect and attempt to verify that certificate. **Default:** + `false`. + * `sessionTimeout` {number} An integer specifying the number of seconds after + which the TLS session identifiers and TLS session tickets created by the + server will time out. See [`SSL_CTX_set_timeout`] for more details. * `SNICallback(servername, cb)` {Function} A function that will be called if the client supports SNI TLS extension. Two arguments will be passed when called: `servername` and `cb`. `SNICallback` should invoke `cb(null, ctx)`, where `ctx` is a `SecureContext` instance. (`tls.createSecureContext(...)` can be used to get a proper `SecureContext`.) If `SNICallback` wasn't provided the default callback with high-level API will be used (see below). - * `sessionTimeout` {number} An integer specifying the number of seconds after - which the TLS session identifiers and TLS session tickets created by the - server will time out. See [`SSL_CTX_set_timeout`] for more details. * `ticketKeys`: A 48-byte `Buffer` instance consisting of a 16-byte prefix, a 16-byte HMAC key, and a 16-byte AES key. This can be used to accept TLS session tickets on multiple instances of the TLS server. - * ...: Any [`tls.createSecureContext()`][] options can be provided. For + * ...: Any [`tls.createSecureContext()`][] option can be provided. For servers, the identity options (`pfx` or `key`/`cert`) are usually required. * `secureConnectionListener` {Function} From 2c4f80ffba95aee1aff3dfc5dabd539c857a584e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 4 Jun 2018 12:32:54 -0700 Subject: [PATCH 048/150] doc: remove spaces around slashes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove spaces around slash characters in documentation. This change sometimes rewords the content where the slash construction may not be what is called for. PR-URL: https://github.com/nodejs/node/pull/21140 Reviewed-By: Vse Mozhet Byt Reviewed-By: Michaël Zasso Reviewed-By: Trivikram Kamat Reviewed-By: Colin Ihrig --- doc/api/assert.md | 2 +- doc/api/crypto.md | 6 +++--- doc/api/dgram.md | 2 +- doc/api/dns.md | 4 ++-- doc/api/errors.md | 12 ++++++------ doc/api/intl.md | 2 +- doc/api/net.md | 2 +- doc/api/stream.md | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/doc/api/assert.md b/doc/api/assert.md index 45132eec85cfb5..57fcc14de2ba90 100644 --- a/doc/api/assert.md +++ b/doc/api/assert.md @@ -1083,7 +1083,7 @@ validating against a string property. See below for examples. If specified, `message` will be the message provided by the `AssertionError` if the block fails to throw. -Custom validation object / error instance: +Custom validation object/error instance: ```js const err = new TypeError('Wrong value'); diff --git a/doc/api/crypto.md b/doc/api/crypto.md index 9b8e21469a8e65..70f910e11b7975 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -2263,7 +2263,7 @@ mode must adhere to certain restrictions when using the cipher API: bytes (`7 ≤ N ≤ 13`). - The length of the plaintext is limited to `2 ** (8 * (15 - N))` bytes. - When decrypting, the authentication tag must be set via `setAuthTag()` before - specifying additional authenticated data and / or calling `update()`. + specifying additional authenticated data or calling `update()`. Otherwise, decryption will fail and `final()` will throw an error in compliance with section 2.6 of [RFC 3610][]. - Using stream methods such as `write(data)`, `end(data)` or `pipe()` in CCM @@ -2273,8 +2273,8 @@ mode must adhere to certain restrictions when using the cipher API: option. This is not necessary if no AAD is used. - As CCM processes the whole message at once, `update()` can only be called once. -- Even though calling `update()` is sufficient to encrypt / decrypt the message, - applications *must* call `final()` to compute and / or verify the +- Even though calling `update()` is sufficient to encrypt/decrypt the message, + applications *must* call `final()` to compute or verify the authentication tag. ```js diff --git a/doc/api/dgram.md b/doc/api/dgram.md index cd3e1de15adfde..f4db86d5ea8b71 100644 --- a/doc/api/dgram.md +++ b/doc/api/dgram.md @@ -1,4 +1,4 @@ -# UDP / Datagram Sockets +# UDP/Datagram Sockets diff --git a/doc/api/dns.md b/doc/api/dns.md index 93f91a6e351c65..8d47556d7c94c1 100644 --- a/doc/api/dns.md +++ b/doc/api/dns.md @@ -499,8 +499,8 @@ will be present on the object: | Type | Properties | |------|------------| -| `'A'` | `address` / `ttl` | -| `'AAAA'` | `address` / `ttl` | +| `'A'` | `address`/`ttl` | +| `'AAAA'` | `address`/`ttl` | | `'CNAME'` | `value` | | `'MX'` | Refer to [`dns.resolveMx()`][] | | `'NAPTR'` | Refer to [`dns.resolveNaptr()`][] | diff --git a/doc/api/errors.md b/doc/api/errors.md index 6fa1e8a7797138..ac489accdb1c51 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -31,7 +31,7 @@ called. All JavaScript errors are handled as exceptions that *immediately* generate and throw an error using the standard JavaScript `throw` mechanism. These -are handled using the [`try / catch` construct][try-catch] provided by the +are handled using the [`try…catch` construct][try-catch] provided by the JavaScript language. ```js @@ -45,7 +45,7 @@ try { ``` Any use of the JavaScript `throw` mechanism will raise an exception that -*must* be handled using `try / catch` or the Node.js process will exit +*must* be handled using `try…catch` or the Node.js process will exit immediately. With few exceptions, _Synchronous_ APIs (any blocking method that does not @@ -90,7 +90,7 @@ Errors that occur within _Asynchronous APIs_ may be reported in multiple ways: - A handful of typically asynchronous methods in the Node.js API may still use the `throw` mechanism to raise exceptions that must be handled using - `try / catch`. There is no comprehensive list of such methods; please + `try…catch`. There is no comprehensive list of such methods; please refer to the documentation of each method to determine the appropriate error handling mechanism required. @@ -116,7 +116,7 @@ setImmediate(() => { }); ``` -Errors generated in this way *cannot* be intercepted using `try / catch` as +Errors generated in this way *cannot* be intercepted using `try…catch` as they are thrown *after* the calling code has already exited. Developers must refer to the documentation for each method to determine @@ -149,7 +149,7 @@ fs.readFile('/some/file/that/does-not-exist', errorFirstCallback); fs.readFile('/some/file/that/does-exist', errorFirstCallback); ``` -The JavaScript `try / catch` mechanism **cannot** be used to intercept errors +The JavaScript `try…catch` mechanism **cannot** be used to intercept errors generated by asynchronous APIs. A common mistake for beginners is to try to use `throw` inside an error-first callback: @@ -648,7 +648,7 @@ Used when a child process is being forked without specifying an IPC channel. ### ERR_CHILD_PROCESS_STDIO_MAXBUFFER Used when the main process is trying to read data from the child process's -STDERR / STDOUT, and the data's length is longer than the `maxBuffer` option. +STDERR/STDOUT, and the data's length is longer than the `maxBuffer` option. ### ERR_CLOSED_MESSAGE_PORT diff --git a/doc/api/intl.md b/doc/api/intl.md index fe5c4ef9d055f5..fc10bf9dda2e82 100644 --- a/doc/api/intl.md +++ b/doc/api/intl.md @@ -36,7 +36,7 @@ To control how ICU is used in Node.js, four `configure` options are available during compilation. Additional details on how to compile Node.js are documented in [BUILDING.md][]. -- `--with-intl=none` / `--without-intl` +- `--with-intl=none`/`--without-intl` - `--with-intl=system-icu` - `--with-intl=small-icu` (default) - `--with-intl=full-icu` diff --git a/doc/api/net.md b/doc/api/net.md index e48bdc1eaaf3b7..f1e41b46c47db7 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -213,7 +213,7 @@ called. Otherwise, an `ERR_SERVER_ALREADY_LISTEN` error will be thrown. One of the most common errors raised when listening is `EADDRINUSE`. This happens when another server is already listening on the requested -`port` / `path` / `handle`. One way to handle this would be to retry +`port`/`path`/`handle`. One way to handle this would be to retry after a certain amount of time: ```js diff --git a/doc/api/stream.md b/doc/api/stream.md index 483aec1b700fb6..014b2f68b3359b 100644 --- a/doc/api/stream.md +++ b/doc/api/stream.md @@ -367,7 +367,7 @@ added: v8.0.0 Destroy the stream, and emit the passed `'error'` and a `'close'` event. After this call, the writable stream has ended and subsequent calls -to `write()` / `end()` will give an `ERR_STREAM_DESTROYED` error. +to `write()` or `end()` will result in an `ERR_STREAM_DESTROYED` error. Implementors should not override this method, but instead implement [`writable._destroy()`][writable-_destroy]. From 917960e0a123681e098fa244d292a7a6f82aa68e Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 28 Mar 2018 21:16:47 +0200 Subject: [PATCH 049/150] win, build: add documentation support to vcbuild Adds `doc` option to vcbuild.bat which will install node-doc-generator dependencies and build the documentation in the %config%\doc folder. Adds `lint-md-build` option which will download markdown linter. Adds `lint-md` option, included by default in `lint` and `test` options which will run linter on the markdown files in the doc folder. PR-URL: https://github.com/nodejs/node/pull/19663 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Refael Ackermann --- vcbuild.bat | 102 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 91 insertions(+), 11 deletions(-) diff --git a/vcbuild.bat b/vcbuild.bat index c6921ab1915028..49ff50f60e4a36 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -28,6 +28,8 @@ set upload= set licensertf= set lint_js= set lint_cpp= +set lint_md= +set lint_md_build= set build_testgc_addon= set noetw= set noetw_msi_arg= @@ -53,6 +55,7 @@ set nghttp2_debug= set link_module= set no_cctest= set openssl_no_asm= +set doc= :next-arg if "%1"=="" goto args-done @@ -71,7 +74,7 @@ if /i "%1"=="nosnapshot" set nosnapshot=1&goto arg-ok if /i "%1"=="noetw" set noetw=1&goto arg-ok if /i "%1"=="noperfctr" set noperfctr=1&goto arg-ok if /i "%1"=="licensertf" set licensertf=1&goto arg-ok -if /i "%1"=="test" set test_args=%test_args% -J %common_test_suites%&set lint_cpp=1&set lint_js=1&goto arg-ok +if /i "%1"=="test" set test_args=%test_args% -J %common_test_suites%&set lint_cpp=1&set lint_js=1&set lint_md=1&goto arg-ok if /i "%1"=="test-ci" set test_args=%test_args% %test_ci_args% -p tap --logfile test.tap %common_test_suites%&set cctest_args=%cctest_args% --gtest_output=tap:cctest.tap&goto arg-ok if /i "%1"=="build-addons" set build_addons=1&goto arg-ok if /i "%1"=="build-addons-napi" set build_addons_napi=1&goto arg-ok @@ -98,7 +101,9 @@ if /i "%1"=="lint-js" set lint_js=1&goto arg-ok if /i "%1"=="jslint" set lint_js=1&echo Please use lint-js instead of jslint&goto arg-ok if /i "%1"=="lint-js-ci" set lint_js_ci=1&goto arg-ok if /i "%1"=="jslint-ci" set lint_js_ci=1&echo Please use lint-js-ci instead of jslint-ci&goto arg-ok -if /i "%1"=="lint" set lint_cpp=1&set lint_js=1&goto arg-ok +if /i "%1"=="lint-md" set lint_md=1&goto arg-ok +if /i "%1"=="lint-md-build" set lint_md_build=1&goto arg-ok +if /i "%1"=="lint" set lint_cpp=1&set lint_js=1&set lint_md=1&goto arg-ok if /i "%1"=="lint-ci" set lint_cpp=1&set lint_js_ci=1&goto arg-ok if /i "%1"=="package" set package=1&goto arg-ok if /i "%1"=="msi" set msi=1&set licensertf=1&set download_arg="--download=all"&set i18n_arg=small-icu&goto arg-ok @@ -118,6 +123,7 @@ if /i "%1"=="debug-nghttp2" set debug_nghttp2=1&goto arg-ok if /i "%1"=="link-module" set "link_module= --link-module=%2%link_module%"&goto arg-ok-2 if /i "%1"=="no-cctest" set no_cctest=1&goto arg-ok if /i "%1"=="openssl-no-asm" set openssl_no_asm=1&goto arg-ok +if /i "%1"=="doc" set doc=1&goto arg-ok echo Error: invalid command line option `%1`. exit /b 1 @@ -146,6 +152,7 @@ if defined build_release ( :: assign path to node_exe set "node_exe=%config%\node.exe" set "node_gyp_exe="%node_exe%" deps\npm\node_modules\node-gyp\bin\node-gyp" +set "npm_exe="%~dp0%node_exe%" %~dp0deps\npm\bin\npm-cli.js" if "%target_env%"=="vs2017" set "node_gyp_exe=%node_gyp_exe% --msvs_version=2017" if "%config%"=="Debug" set configure_flags=%configure_flags% --debug @@ -179,7 +186,11 @@ call :getnodeversion || exit /b 1 if defined TAG set configure_flags=%configure_flags% --tag=%TAG% -if "%target%"=="Clean" rmdir /Q /S "%~dp0%config%\node-v%FULLVERSION%-win-%target_arch%" > nul 2> nul +if not "%target%"=="Clean" goto skip-clean +rmdir /Q /S "%~dp0%config%\node-v%FULLVERSION%-win-%target_arch%" > nul 2> nul +rmdir /Q /S "%~dp0tools\remark-cli\node_modules" +rmdir /Q /S "%~dp0tools\remark-preset-lint-node\node_modules" +:skip-clean if defined noprojgen if defined nobuild if not defined sign if not defined msi goto licensertf @@ -235,7 +246,7 @@ goto exit :wix-not-found echo Build skipped. To generate installer, you need to install Wix. -goto run +goto build-doc :msbuild-found @@ -342,7 +353,7 @@ exit /b 1 :msi @rem Skip msi generation if not requested -if not defined msi goto run +if not defined msi goto build-doc :msibuild echo Building node-v%FULLVERSION%-%target_arch%.msi @@ -357,7 +368,7 @@ if errorlevel 1 echo Failed to sign msi&goto exit :upload @rem Skip upload if not requested -if not defined upload goto run +if not defined upload goto build-doc if not defined SSHCONFIG ( echo SSHCONFIG is not set for upload @@ -385,6 +396,30 @@ ssh -F %SSHCONFIG% %STAGINGSERVER% "touch nodejs/%DISTTYPEDIR%/v%FULLVERSION%/no if errorlevel 1 goto exit +:build-doc +@rem Build documentation if requested +if not defined doc goto run +if not exist %node_exe% ( + echo Failed to find node.exe + goto run +) +mkdir %config%\doc +robocopy /e doc\api %config%\doc\api +robocopy /e doc\api_assets %config%\doc\api\assets + +if exist "tools\doc\node_modules\js-yaml\package.json" goto doc-skip-js-yaml +SETLOCAL +cd tools\doc +%npm_exe% install +cd ..\.. +if errorlevel 1 goto exit +ENDLOCAL +:doc-skip-js-yaml +for %%F in (%config%\doc\api\*.md) do ( + %node_exe% tools\doc\generate.js --format=json %%F > %%~dF%%~pF%%~nF.json + %node_exe% tools\doc\generate.js --node-version=v%FULLVERSION% --format=html --analytics=%DOCS_ANALYTICS% %%F > %%~dF%%~pF%%~nF.html +) + :run @rem Run tests if requested. @@ -528,28 +563,73 @@ goto exit :lint-js if defined lint_js_ci goto lint-js-ci -if not defined lint_js goto exit +if not defined lint_js goto lint-md-build if not exist tools\node_modules\eslint goto no-lint echo running lint-js %config%\node tools\node_modules\eslint\bin\eslint.js --cache --rule "linebreak-style: 0" --ext=.js,.mjs,.md .eslintrc.js benchmark doc lib test tools -goto exit +goto lint-md-build :lint-js-ci echo running lint-js-ci %config%\node tools\lint-js.js -J -f tap -o test-eslint.tap benchmark doc lib test tools -goto exit +goto lint-md-build :no-lint echo Linting is not available through the source tarball. echo Use the git repo instead: $ git clone https://github.com/nodejs/node.git +goto lint-md-build + +:lint-md-build +if not defined lint_md_build goto lint-md +SETLOCAL +echo Markdown linter: installing remark-cli into tools\ +cd tools\remark-cli +%npm_exe% install +cd ..\.. +if errorlevel 1 goto lint-md-build-failed +echo Markdown linter: installing remark-preset-lint-node into tools\ +cd tools\remark-preset-lint-node +%npm_exe% install +cd ..\.. +if errorlevel 1 goto lint-md-build-failed +ENDLOCAL +goto lint-md + +:if errorlevel 1 goto lint-md-build-failed +ENDLOCAL +echo Failed to install markdown linter +exit /b 1 + +:lint-md +if not defined lint_md goto exit +if not exist tools\remark-cli\node_modules goto lint-md-no-tools +if not exist tools\remark-preset-lint-node\node_modules goto lint-md-no-tools +echo Running Markdown linter on docs... +SETLOCAL ENABLEDELAYEDEXPANSION +set lint_md_files= +cd doc +for /D %%D IN (*) do ( + for %%F IN (%%D\*.md) do ( + set "lint_md_files="doc\%%F" !lint_md_files!" + ) +) +cd .. +%config%\node tools\remark-cli\cli.js -q -f %lint_md_files% +ENDLOCAL goto exit +:lint-md-no-tools +echo The markdown linter is not installed. +echo To install (requires internet access) run: vcbuild lint-md-build +goto exit + + :create-msvs-files-failed echo Failed to create vc project files. goto exit :help -echo vcbuild.bat [debug/release] [msi] [test/test-ci/test-all/test-addons/test-addons-napi/test-internet/test-pummel/test-simple/test-message/test-gc/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [noetw] [noperfctr] [licensetf] [sign] [ia32/x86/x64] [vs2017] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-js-ci] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [no-cctest] [openssl-no-asm] +echo vcbuild.bat [debug/release] [msi] [doc] [test/test-ci/test-all/test-addons/test-addons-napi/test-internet/test-pummel/test-simple/test-message/test-gc/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-async-hooks/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [noetw] [noperfctr] [licensetf] [sign] [ia32/x86/x64] [vs2017] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-js-ci/lint-md] [lint-md-build] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [no-cctest] [openssl-no-asm] echo Examples: echo vcbuild.bat : builds release build echo vcbuild.bat debug : builds debug build @@ -558,7 +638,7 @@ echo vcbuild.bat test : builds debug build and runs tests echo vcbuild.bat build-release : builds the release distribution as used by nodejs.org echo vcbuild.bat enable-vtune : builds nodejs with Intel VTune profiling support to profile JavaScript echo vcbuild.bat link-module my_module.js : bundles my_module as built-in module -echo vcbuild.bat lint : runs the C++ and JavaScript linter +echo vcbuild.bat lint : runs the C++, documentation and JavaScript linter echo vcbuild.bat no-cctest : skip building cctest.exe goto exit From 0b0370f8841c4c399ddeddea1c63ff80137971da Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Mon, 4 Jun 2018 17:38:23 +0000 Subject: [PATCH 050/150] test: remove unref in http2 test The bug referenced in this TODO was fixed and this test no longer requires this code to pass. PR-URL: https://github.com/nodejs/node/pull/21145 Reviewed-By: Rich Trott --- test/parallel/test-stream-pipeline.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/parallel/test-stream-pipeline.js b/test/parallel/test-stream-pipeline.js index b52d60529b0332..12733d88a7ac85 100644 --- a/test/parallel/test-stream-pipeline.js +++ b/test/parallel/test-stream-pipeline.js @@ -281,12 +281,6 @@ common.crashOnUnhandledRejection(); }); pipeline(rs, req, common.mustCall((err) => { - // TODO: this is working around an http2 bug - // where the client keeps the event loop going - // (replacing the rs.destroy() with req.end() - // exits it so seems to be a destroy bug there - client.unref(); - server.close(); client.close(); })); From ea4be72f22ce0d75307065ce95353b416bf543af Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 3 Jun 2018 11:49:41 +0200 Subject: [PATCH 051/150] child_process: swallow errors in internal communication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Much like with NODE_HANDLE_ACK, the internal protocol for communication about the sent socket should not expose its errors to the users when those async calls are not initiated by them. PR-URL: https://github.com/nodejs/node/pull/21108 Reviewed-By: Rich Trott Reviewed-By: Joyee Cheung Reviewed-By: Michaël Zasso --- lib/internal/socket_list.js | 14 +++++++------- test/parallel/parallel.status | 1 - .../test-internal-socket-list-receive.js | 6 +++--- test/parallel/test-internal-socket-list-send.js | 16 ++++++++-------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/internal/socket_list.js b/lib/internal/socket_list.js index 55077af1305121..d12686e1de107d 100644 --- a/lib/internal/socket_list.js +++ b/lib/internal/socket_list.js @@ -13,11 +13,11 @@ class SocketListSend extends EventEmitter { child.once('exit', () => this.emit('exit', this)); } - _request(msg, cmd, callback) { + _request(msg, cmd, swallowErrors, callback) { var self = this; if (!this.child.connected) return onclose(); - this.child.send(msg); + this.child._send(msg, undefined, swallowErrors); function onclose() { self.child.removeListener('internalMessage', onreply); @@ -40,14 +40,14 @@ class SocketListSend extends EventEmitter { this._request({ cmd: 'NODE_SOCKET_NOTIFY_CLOSE', key: this.key - }, 'NODE_SOCKET_ALL_CLOSED', callback); + }, 'NODE_SOCKET_ALL_CLOSED', true, callback); } getConnections(callback) { this._request({ cmd: 'NODE_SOCKET_GET_COUNT', key: this.key - }, 'NODE_SOCKET_COUNT', function(err, msg) { + }, 'NODE_SOCKET_COUNT', false, function(err, msg) { if (err) return callback(err); callback(null, msg.count); }); @@ -67,10 +67,10 @@ class SocketListReceive extends EventEmitter { function onempty(self) { if (!self.child.connected) return; - self.child.send({ + self.child._send({ cmd: 'NODE_SOCKET_ALL_CLOSED', key: self.key - }); + }, undefined, true); } this.child.on('internalMessage', (msg) => { @@ -84,7 +84,7 @@ class SocketListReceive extends EventEmitter { this.once('empty', onempty); } else if (msg.cmd === 'NODE_SOCKET_GET_COUNT') { if (!this.child.connected) return; - this.child.send({ + this.child._send({ cmd: 'NODE_SOCKET_COUNT', key: this.key, count: this.connections diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index fbb105e54cfae4..6f4c13a96f96ee 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -7,7 +7,6 @@ prefix parallel [true] # This section applies to all platforms [$system==win32] -test-child-process-fork-net-socket: PASS,FLAKY [$system==linux] diff --git a/test/parallel/test-internal-socket-list-receive.js b/test/parallel/test-internal-socket-list-receive.js index c0eb223719ae6f..e7e64887600f59 100644 --- a/test/parallel/test-internal-socket-list-receive.js +++ b/test/parallel/test-internal-socket-list-receive.js @@ -12,7 +12,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: false, - send: common.mustNotCall() + _send: common.mustNotCall() }); const list = new SocketListReceive(child, key); @@ -24,7 +24,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: common.mustCall((msg) => { + _send: common.mustCall((msg) => { assert.strictEqual(msg.cmd, 'NODE_SOCKET_ALL_CLOSED'); assert.strictEqual(msg.key, key); }) @@ -38,7 +38,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: common.mustCall((msg) => { + _send: common.mustCall((msg) => { assert.strictEqual(msg.cmd, 'NODE_SOCKET_COUNT'); assert.strictEqual(msg.key, key); assert.strictEqual(msg.count, 0); diff --git a/test/parallel/test-internal-socket-list-send.js b/test/parallel/test-internal-socket-list-send.js index 797baf43f4dd76..2e69edcedffd9f 100644 --- a/test/parallel/test-internal-socket-list-send.js +++ b/test/parallel/test-internal-socket-list-send.js @@ -16,7 +16,7 @@ const key = 'test-key'; const list = new SocketListSend(child, 'test'); - list._request('msg', 'cmd', common.mustCall((err) => { + list._request('msg', 'cmd', false, common.mustCall((err) => { common.expectsError({ code: 'ERR_CHILD_CLOSED_BEFORE_REPLY', type: Error, @@ -30,7 +30,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { process.nextTick(() => this.emit('internalMessage', { key, cmd: 'cmd' }) ); @@ -39,7 +39,7 @@ const key = 'test-key'; const list = new SocketListSend(child, key); - list._request('msg', 'cmd', common.mustCall((err, msg) => { + list._request('msg', 'cmd', false, common.mustCall((err, msg) => { assert.strictEqual(err, null); assert.strictEqual(msg.cmd, 'cmd'); assert.strictEqual(msg.key, key); @@ -53,12 +53,12 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { process.nextTick(() => this.emit('disconnect')); } + _send: function(msg) { process.nextTick(() => this.emit('disconnect')); } }); const list = new SocketListSend(child, key); - list._request('msg', 'cmd', common.mustCall((err) => { + list._request('msg', 'cmd', false, common.mustCall((err) => { common.expectsError({ code: 'ERR_CHILD_CLOSED_BEFORE_REPLY', type: Error, @@ -73,7 +73,7 @@ const key = 'test-key'; { const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { assert.strictEqual(msg.cmd, 'NODE_SOCKET_NOTIFY_CLOSE'); assert.strictEqual(msg.key, key); process.nextTick(() => @@ -98,7 +98,7 @@ const key = 'test-key'; const count = 1; const child = Object.assign(new EventEmitter(), { connected: true, - send: function(msg) { + _send: function(msg) { assert.strictEqual(msg.cmd, 'NODE_SOCKET_GET_COUNT'); assert.strictEqual(msg.key, key); process.nextTick(() => @@ -127,7 +127,7 @@ const key = 'test-key'; const count = 1; const child = Object.assign(new EventEmitter(), { connected: true, - send: function() { + _send: function() { process.nextTick(() => { this.emit('disconnect'); this.emit('internalMessage', { key, count, cmd: 'NODE_SOCKET_COUNT' }); From 04e8f0749e2cac0f6db8a02ce40d817cb668d094 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 7 Apr 2018 17:01:06 +0800 Subject: [PATCH 052/150] fs: support BigInt in fs.*stat and fs.watchFile Add the `bigint: true` option to all the `fs.*stat` methods and `fs.watchFile`. PR-URL: https://github.com/nodejs/node/pull/20220 Fixes: https://github.com/nodejs/node/issues/12115 Reviewed-By: Ben Noordhuis --- lib/fs.js | 56 +++++---- lib/internal/fs/promises.js | 19 ++- lib/internal/fs/utils.js | 16 ++- lib/internal/fs/watchers.js | 4 +- src/env-inl.h | 5 + src/env.cc | 1 + src/env.h | 3 + src/node_file.cc | 59 +++++---- src/node_file.h | 19 +-- src/node_internals.h | 12 +- src/node_stat_watcher.cc | 13 +- src/node_stat_watcher.h | 3 +- test/parallel/test-fs-stat-bigint.js | 145 ++++++++++++++++++++++ test/parallel/test-fs-sync-fd-leak.js | 2 +- test/parallel/test-fs-watchfile-bigint.js | 63 ++++++++++ 15 files changed, 341 insertions(+), 79 deletions(-) create mode 100644 test/parallel/test-fs-stat-bigint.js create mode 100644 test/parallel/test-fs-watchfile-bigint.js diff --git a/lib/fs.js b/lib/fs.js index 983625bc9b0af3..4a7eabc5117174 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -248,7 +248,7 @@ function readFileAfterOpen(err, fd) { const req = new FSReqWrap(); req.oncomplete = readFileAfterStat; req.context = context; - binding.fstat(fd, req); + binding.fstat(fd, false, req); } function readFileAfterStat(err, stats) { @@ -307,7 +307,7 @@ function readFile(path, options, callback) { function tryStatSync(fd, isUserFd) { const ctx = {}; - const stats = binding.fstat(fd, undefined, ctx); + const stats = binding.fstat(fd, false, undefined, ctx); if (ctx.errno !== undefined && !isUserFd) { fs.closeSync(fd); throw errors.uvException(ctx); @@ -760,55 +760,67 @@ function readdirSync(path, options) { return result; } -function fstat(fd, callback) { +function fstat(fd, options, callback) { + if (arguments.length < 3) { + callback = options; + options = {}; + } validateUint32(fd, 'fd'); - const req = new FSReqWrap(); + const req = new FSReqWrap(options.bigint); req.oncomplete = makeStatsCallback(callback); - binding.fstat(fd, req); + binding.fstat(fd, options.bigint, req); } -function lstat(path, callback) { +function lstat(path, options, callback) { + if (arguments.length < 3) { + callback = options; + options = {}; + } callback = makeStatsCallback(callback); path = getPathFromURL(path); validatePath(path); - const req = new FSReqWrap(); + const req = new FSReqWrap(options.bigint); req.oncomplete = callback; - binding.lstat(pathModule.toNamespacedPath(path), req); + binding.lstat(pathModule.toNamespacedPath(path), options.bigint, req); } -function stat(path, callback) { +function stat(path, options, callback) { + if (arguments.length < 3) { + callback = options; + options = {}; + } callback = makeStatsCallback(callback); path = getPathFromURL(path); validatePath(path); - const req = new FSReqWrap(); + const req = new FSReqWrap(options.bigint); req.oncomplete = callback; - binding.stat(pathModule.toNamespacedPath(path), req); + binding.stat(pathModule.toNamespacedPath(path), options.bigint, req); } -function fstatSync(fd) { +function fstatSync(fd, options = {}) { validateUint32(fd, 'fd'); const ctx = { fd }; - const stats = binding.fstat(fd, undefined, ctx); + const stats = binding.fstat(fd, options.bigint, undefined, ctx); handleErrorFromBinding(ctx); return getStatsFromBinding(stats); } -function lstatSync(path) { +function lstatSync(path, options = {}) { path = getPathFromURL(path); validatePath(path); const ctx = { path }; const stats = binding.lstat(pathModule.toNamespacedPath(path), - undefined, ctx); + options.bigint, undefined, ctx); handleErrorFromBinding(ctx); return getStatsFromBinding(stats); } -function statSync(path) { +function statSync(path, options = {}) { path = getPathFromURL(path); validatePath(path); const ctx = { path }; const stats = binding.stat(pathModule.toNamespacedPath(path), - undefined, ctx); + options.bigint, undefined, ctx); handleErrorFromBinding(ctx); return getStatsFromBinding(stats); } @@ -1264,7 +1276,7 @@ function watchFile(filename, options, listener) { if (stat === undefined) { if (!watchers) watchers = require('internal/fs/watchers'); - stat = new watchers.StatWatcher(); + stat = new watchers.StatWatcher(options.bigint); stat.start(filename, options.persistent, options.interval); statWatchers.set(filename, stat); } @@ -1379,7 +1391,7 @@ function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { const ctx = { path: base }; - binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); + binding.lstat(pathModule.toNamespacedPath(base), false, undefined, ctx); handleErrorFromBinding(ctx); knownHard[base] = true; } @@ -1421,7 +1433,7 @@ function realpathSync(p, options) { const baseLong = pathModule.toNamespacedPath(base); const ctx = { path: base }; - const stats = binding.lstat(baseLong, undefined, ctx); + const stats = binding.lstat(baseLong, false, undefined, ctx); handleErrorFromBinding(ctx); if (!isFileType(stats, S_IFLNK)) { @@ -1444,7 +1456,7 @@ function realpathSync(p, options) { } if (linkTarget === null) { const ctx = { path: base }; - binding.stat(baseLong, undefined, ctx); + binding.stat(baseLong, false, undefined, ctx); handleErrorFromBinding(ctx); linkTarget = binding.readlink(baseLong, undefined, undefined, ctx); handleErrorFromBinding(ctx); @@ -1465,7 +1477,7 @@ function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { const ctx = { path: base }; - binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); + binding.lstat(pathModule.toNamespacedPath(base), false, undefined, ctx); handleErrorFromBinding(ctx); knownHard[base] = true; } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index fb42ebb935a6a7..c966e0a897900a 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -81,8 +81,8 @@ class FileHandle { return readFile(this, options); } - stat() { - return fstat(this); + stat(options) { + return fstat(this, options); } truncate(len = 0) { @@ -106,7 +106,6 @@ class FileHandle { } } - function validateFileHandle(handle) { if (!(handle instanceof FileHandle)) throw new ERR_INVALID_ARG_TYPE('filehandle', 'FileHandle', handle); @@ -127,7 +126,7 @@ async function writeFileHandle(filehandle, data, options) { } async function readFileHandle(filehandle, options) { - const statFields = await binding.fstat(filehandle.fd, kUsePromises); + const statFields = await binding.fstat(filehandle.fd, false, kUsePromises); let size; if ((statFields[1/* mode */] & S_IFMT) === S_IFREG) { @@ -318,25 +317,25 @@ async function symlink(target, path, type_) { kUsePromises); } -async function fstat(handle) { +async function fstat(handle, options = { bigint: false }) { validateFileHandle(handle); - const result = await binding.fstat(handle.fd, kUsePromises); + const result = await binding.fstat(handle.fd, options.bigint, kUsePromises); return getStatsFromBinding(result); } -async function lstat(path) { +async function lstat(path, options = { bigint: false }) { path = getPathFromURL(path); validatePath(path); const result = await binding.lstat(pathModule.toNamespacedPath(path), - kUsePromises); + options.bigint, kUsePromises); return getStatsFromBinding(result); } -async function stat(path) { +async function stat(path, options = { bigint: false }) { path = getPathFromURL(path); validatePath(path); const result = await binding.stat(pathModule.toNamespacedPath(path), - kUsePromises); + options.bigint, kUsePromises); return getStatsFromBinding(result); } diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index a8c64e2b04d56b..7092bbcb633d81 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -114,7 +114,7 @@ function preprocessSymlinkDestination(path, type, linkPath) { } function dateFromNumeric(num) { - return new Date(num + 0.5); + return new Date(Number(num) + 0.5); } // Constructor for file stats. @@ -155,7 +155,15 @@ function Stats( } Stats.prototype._checkModeProperty = function(property) { - return ((this.mode & S_IFMT) === property); + if (isWindows && (property === S_IFIFO || property === S_IFBLK || + property === S_IFSOCK)) { + return false; // Some types are not available on Windows + } + if (typeof this.mode === 'bigint') { // eslint-disable-line valid-typeof + // eslint-disable-next-line no-undef + return (this.mode & BigInt(S_IFMT)) === BigInt(property); + } + return (this.mode & S_IFMT) === property; }; Stats.prototype.isDirectory = function() { @@ -189,9 +197,9 @@ Stats.prototype.isSocket = function() { function getStatsFromBinding(stats, offset = 0) { return new Stats(stats[0 + offset], stats[1 + offset], stats[2 + offset], stats[3 + offset], stats[4 + offset], stats[5 + offset], - stats[6 + offset] < 0 ? undefined : stats[6 + offset], + isWindows ? undefined : stats[6 + offset], // blksize stats[7 + offset], stats[8 + offset], - stats[9 + offset] < 0 ? undefined : stats[9 + offset], + isWindows ? undefined : stats[9 + offset], // blocks stats[10 + offset], stats[11 + offset], stats[12 + offset], stats[13 + offset]); } diff --git a/lib/internal/fs/watchers.js b/lib/internal/fs/watchers.js index 1007a5a13614d2..5fd6948f28b362 100644 --- a/lib/internal/fs/watchers.js +++ b/lib/internal/fs/watchers.js @@ -21,10 +21,10 @@ function emitStop(self) { self.emit('stop'); } -function StatWatcher() { +function StatWatcher(bigint) { EventEmitter.call(this); - this._handle = new _StatWatcher(); + this._handle = new _StatWatcher(bigint); // uv_fs_poll is a little more powerful than ev_stat but we curb it for // the sake of backwards compatibility diff --git a/src/env-inl.h b/src/env-inl.h index 165244895582ef..c84fdf0bb8215b 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -546,6 +546,11 @@ Environment::fs_stats_field_array() { return &fs_stats_field_array_; } +inline AliasedBuffer* +Environment::fs_stats_field_bigint_array() { + return &fs_stats_field_bigint_array_; +} + inline std::vector>& Environment::file_handle_read_wrap_freelist() { return file_handle_read_wrap_freelist_; diff --git a/src/env.cc b/src/env.cc index 8df59d1546dbdd..6f6e9f3920f4b1 100644 --- a/src/env.cc +++ b/src/env.cc @@ -116,6 +116,7 @@ Environment::Environment(IsolateData* isolate_data, #endif http_parser_buffer_(nullptr), fs_stats_field_array_(isolate_, kFsStatsFieldsLength * 2), + fs_stats_field_bigint_array_(isolate_, kFsStatsFieldsLength * 2), context_(context->GetIsolate(), context) { // We'll be creating new objects so make sure we've entered the context. v8::HandleScope handle_scope(isolate()); diff --git a/src/env.h b/src/env.h index ee734fe92058b6..9ad316c063ea72 100644 --- a/src/env.h +++ b/src/env.h @@ -694,6 +694,8 @@ class Environment { void set_debug_categories(const std::string& cats, bool enabled); inline AliasedBuffer* fs_stats_field_array(); + inline AliasedBuffer* + fs_stats_field_bigint_array(); // stat fields contains twice the number of entries because `fs.StatWatcher` // needs room to store data for *two* `fs.Stats` instances. @@ -914,6 +916,7 @@ class Environment { bool debug_enabled_[static_cast(DebugCategory::CATEGORY_COUNT)] = {0}; AliasedBuffer fs_stats_field_array_; + AliasedBuffer fs_stats_field_bigint_array_; std::vector> file_handle_read_wrap_freelist_; diff --git a/src/node_file.cc b/src/node_file.cc index d4f3cbfcfb99d7..a63e90fafb2926 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -49,6 +49,7 @@ namespace node { namespace fs { using v8::Array; +using v8::BigUint64Array; using v8::Context; using v8::EscapableHandleScope; using v8::Float64Array; @@ -413,7 +414,7 @@ void FSReqWrap::Reject(Local reject) { } void FSReqWrap::ResolveStat(const uv_stat_t* stat) { - Resolve(node::FillGlobalStatsArray(env(), stat)); + Resolve(node::FillGlobalStatsArray(env(), stat, use_bigint())); } void FSReqWrap::Resolve(Local value) { @@ -433,7 +434,7 @@ void FSReqWrap::SetReturnValue(const FunctionCallbackInfo& args) { void NewFSReqWrap(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); Environment* env = Environment::GetCurrent(args.GetIsolate()); - new FSReqWrap(env, args.This()); + new FSReqWrap(env, args.This(), args[0]->IsTrue()); } FSReqAfterScope::FSReqAfterScope(FSReqBase* wrap, uv_fs_t* req) @@ -670,11 +671,16 @@ inline int SyncCall(Environment* env, Local ctx, FSReqWrapSync* req_wrap, return err; } -inline FSReqBase* GetReqWrap(Environment* env, Local value) { +inline FSReqBase* GetReqWrap(Environment* env, Local value, + bool use_bigint = false) { if (value->IsObject()) { return Unwrap(value.As()); } else if (value->StrictEquals(env->fs_use_promises_symbol())) { - return new FSReqPromise(env); + if (use_bigint) { + return new FSReqPromise(env, use_bigint); + } else { + return new FSReqPromise(env, use_bigint); + } } return nullptr; } @@ -825,22 +831,23 @@ static void Stat(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); - if (req_wrap_async != nullptr) { // stat(path, req) + bool use_bigint = args[1]->IsTrue(); + FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + if (req_wrap_async != nullptr) { // stat(path, use_bigint, req) AsyncCall(env, req_wrap_async, args, "stat", UTF8, AfterStat, uv_fs_stat, *path); - } else { // stat(path, undefined, ctx) - CHECK_EQ(argc, 3); + } else { // stat(path, use_bigint, undefined, ctx) + CHECK_EQ(argc, 4); FSReqWrapSync req_wrap_sync; FS_SYNC_TRACE_BEGIN(stat); - int err = SyncCall(env, args[2], &req_wrap_sync, "stat", uv_fs_stat, *path); + int err = SyncCall(env, args[3], &req_wrap_sync, "stat", uv_fs_stat, *path); FS_SYNC_TRACE_END(stat); if (err != 0) { return; // error info is in ctx } Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + static_cast(req_wrap_sync.req.ptr), use_bigint); args.GetReturnValue().Set(arr); } } @@ -849,20 +856,21 @@ static void LStat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const int argc = args.Length(); - CHECK_GE(argc, 2); + CHECK_GE(argc, 3); BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); - if (req_wrap_async != nullptr) { // lstat(path, req) + bool use_bigint = args[1]->IsTrue(); + FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + if (req_wrap_async != nullptr) { // lstat(path, use_bigint, req) AsyncCall(env, req_wrap_async, args, "lstat", UTF8, AfterStat, uv_fs_lstat, *path); - } else { // lstat(path, undefined, ctx) - CHECK_EQ(argc, 3); + } else { // lstat(path, use_bigint, undefined, ctx) + CHECK_EQ(argc, 4); FSReqWrapSync req_wrap_sync; FS_SYNC_TRACE_BEGIN(lstat); - int err = SyncCall(env, args[2], &req_wrap_sync, "lstat", uv_fs_lstat, + int err = SyncCall(env, args[3], &req_wrap_sync, "lstat", uv_fs_lstat, *path); FS_SYNC_TRACE_END(lstat); if (err != 0) { @@ -870,7 +878,7 @@ static void LStat(const FunctionCallbackInfo& args) { } Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + static_cast(req_wrap_sync.req.ptr), use_bigint); args.GetReturnValue().Set(arr); } } @@ -884,22 +892,23 @@ static void FStat(const FunctionCallbackInfo& args) { CHECK(args[0]->IsInt32()); int fd = args[0].As()->Value(); - FSReqBase* req_wrap_async = GetReqWrap(env, args[1]); - if (req_wrap_async != nullptr) { // fstat(fd, req) + bool use_bigint = args[1]->IsTrue(); + FSReqBase* req_wrap_async = GetReqWrap(env, args[2], use_bigint); + if (req_wrap_async != nullptr) { // fstat(fd, use_bigint, req) AsyncCall(env, req_wrap_async, args, "fstat", UTF8, AfterStat, uv_fs_fstat, fd); - } else { // fstat(fd, undefined, ctx) - CHECK_EQ(argc, 3); + } else { // fstat(fd, use_bigint, undefined, ctx) + CHECK_EQ(argc, 4); FSReqWrapSync req_wrap_sync; FS_SYNC_TRACE_BEGIN(fstat); - int err = SyncCall(env, args[2], &req_wrap_sync, "fstat", uv_fs_fstat, fd); + int err = SyncCall(env, args[3], &req_wrap_sync, "fstat", uv_fs_fstat, fd); FS_SYNC_TRACE_END(fstat); if (err != 0) { return; // error info is in ctx } Local arr = node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + static_cast(req_wrap_sync.req.ptr), use_bigint); args.GetReturnValue().Set(arr); } } @@ -1911,6 +1920,10 @@ void Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "statValues"), env->fs_stats_field_array()->GetJSArray()).FromJust(); + target->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "bigintStatValues"), + env->fs_stats_field_bigint_array()->GetJSArray()).FromJust(); + StatWatcher::Initialize(env, target); // Create FunctionTemplate for FSReqWrap diff --git a/src/node_file.h b/src/node_file.h index 03e41097d5f12e..a14a1b0f85e108 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -26,8 +26,9 @@ class FSReqBase : public ReqWrap { public: typedef MaybeStackBuffer FSReqBuffer; - FSReqBase(Environment* env, Local req, AsyncWrap::ProviderType type) - : ReqWrap(env, req, type) { + FSReqBase(Environment* env, Local req, AsyncWrap::ProviderType type, + bool use_bigint) + : ReqWrap(env, req, type), use_bigint_(use_bigint) { } void Init(const char* syscall, @@ -66,11 +67,13 @@ class FSReqBase : public ReqWrap { enum encoding encoding() const { return encoding_; } size_t self_size() const override { return sizeof(*this); } + bool use_bigint() const { return use_bigint_; } private: enum encoding encoding_ = UTF8; bool has_data_ = false; const char* syscall_ = nullptr; + bool use_bigint_ = false; // Typically, the content of buffer_ is something like a file name, so // something around 64 bytes should be enough. @@ -81,8 +84,8 @@ class FSReqBase : public ReqWrap { class FSReqWrap : public FSReqBase { public: - FSReqWrap(Environment* env, Local req) - : FSReqBase(env, req, AsyncWrap::PROVIDER_FSREQWRAP) { } + FSReqWrap(Environment* env, Local req, bool use_bigint) + : FSReqBase(env, req, AsyncWrap::PROVIDER_FSREQWRAP, use_bigint) { } void Reject(Local reject) override; void Resolve(Local value) override; @@ -96,11 +99,12 @@ class FSReqWrap : public FSReqBase { template class FSReqPromise : public FSReqBase { public: - explicit FSReqPromise(Environment* env) + explicit FSReqPromise(Environment* env, bool use_bigint) : FSReqBase(env, env->fsreqpromise_constructor_template() ->NewInstance(env->context()).ToLocalChecked(), - AsyncWrap::PROVIDER_FSREQPROMISE), + AsyncWrap::PROVIDER_FSREQPROMISE, + use_bigint), stats_field_array_(env->isolate(), env->kFsStatsFieldsLength) { auto resolver = Promise::Resolver::New(env->context()).ToLocalChecked(); object()->Set(env->context(), env->promise_string(), @@ -135,8 +139,7 @@ class FSReqPromise : public FSReqBase { } void ResolveStat(const uv_stat_t* stat) override { - node::FillStatsArray(&stats_field_array_, stat); - Resolve(stats_field_array_.GetJSArray()); + Resolve(node::FillStatsArray(&stats_field_array_, stat)); } void SetReturnValue(const FunctionCallbackInfo& args) override { diff --git a/src/node_internals.h b/src/node_internals.h index f76aacd7e88a5d..49f1d4f230b6d7 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -337,14 +337,14 @@ v8::Local FillStatsArray(AliasedBuffer* fields_ptr, #if defined(__POSIX__) fields[offset + 6] = s->st_blksize; #else - fields[offset + 6] = -1; + fields[offset + 6] = 0; #endif fields[offset + 7] = s->st_ino; fields[offset + 8] = s->st_size; #if defined(__POSIX__) fields[offset + 9] = s->st_blocks; #else - fields[offset + 9] = -1; + fields[offset + 9] = 0; #endif // Dates. // NO-LINT because the fields are 'long' and we just want to cast to `unsigned` @@ -365,8 +365,14 @@ v8::Local FillStatsArray(AliasedBuffer* fields_ptr, inline v8::Local FillGlobalStatsArray(Environment* env, const uv_stat_t* s, + bool use_bigint = false, int offset = 0) { - return node::FillStatsArray(env->fs_stats_field_array(), s, offset); + if (use_bigint) { + return node::FillStatsArray( + env->fs_stats_field_bigint_array(), s, offset); + } else { + return node::FillStatsArray(env->fs_stats_field_array(), s, offset); + } } void SetupBootstrapObject(Environment* env, diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 3749c2e21f2ad9..3f7da197b2a74d 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -75,9 +75,10 @@ void StatWatcher::Initialize(Environment* env, Local target) { } -StatWatcher::StatWatcher(Environment* env, Local wrap) +StatWatcher::StatWatcher(Environment* env, Local wrap, bool use_bigint) : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_STATWATCHER), - watcher_(new uv_fs_poll_t) { + watcher_(new uv_fs_poll_t), + use_bigint_(use_bigint) { MakeWeak(); uv_fs_poll_init(env->event_loop(), watcher_); watcher_->data = static_cast(this); @@ -102,8 +103,10 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - Local arr = node::FillGlobalStatsArray(env, curr); - node::FillGlobalStatsArray(env, prev, env->kFsStatsFieldsLength); + Local arr = node::FillGlobalStatsArray(env, curr, + wrap->use_bigint_); + node::FillGlobalStatsArray(env, prev, wrap->use_bigint_, + env->kFsStatsFieldsLength); Local argv[2] { Integer::New(env->isolate(), status), @@ -116,7 +119,7 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, void StatWatcher::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); Environment* env = Environment::GetCurrent(args); - new StatWatcher(env, args.This()); + new StatWatcher(env, args.This(), args[0]->IsTrue()); } bool StatWatcher::IsActive() { diff --git a/src/node_stat_watcher.h b/src/node_stat_watcher.h index 587203ff1edf46..0d0d263d5cc698 100644 --- a/src/node_stat_watcher.h +++ b/src/node_stat_watcher.h @@ -39,7 +39,7 @@ class StatWatcher : public AsyncWrap { static void Initialize(Environment* env, v8::Local target); protected: - StatWatcher(Environment* env, v8::Local wrap); + StatWatcher(Environment* env, v8::Local wrap, bool use_bigint); static void New(const v8::FunctionCallbackInfo& args); static void Start(const v8::FunctionCallbackInfo& args); @@ -57,6 +57,7 @@ class StatWatcher : public AsyncWrap { bool IsActive(); uv_fs_poll_t* watcher_; + const bool use_bigint_; }; } // namespace node diff --git a/test/parallel/test-fs-stat-bigint.js b/test/parallel/test-fs-stat-bigint.js new file mode 100644 index 00000000000000..4030a06fae3bc8 --- /dev/null +++ b/test/parallel/test-fs-stat-bigint.js @@ -0,0 +1,145 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const promiseFs = require('fs').promises; +const path = require('path'); +const tmpdir = require('../common/tmpdir'); +const { isDate } = require('util').types; + +common.crashOnUnhandledRejection(); +tmpdir.refresh(); + +const fn = path.join(tmpdir.path, 'test-file'); +fs.writeFileSync(fn, 'test'); + +let link; +if (!common.isWindows) { + link = path.join(tmpdir.path, 'symbolic-link'); + fs.symlinkSync(fn, link); +} + +function verifyStats(bigintStats, numStats) { + for (const key of Object.keys(numStats)) { + const val = numStats[key]; + if (isDate(val)) { + const time = val.getTime(); + const time2 = bigintStats[key].getTime(); + assert( + Math.abs(time - time2) < 2, + `difference of ${key}.getTime() should < 2.\n` + + `Number version ${time}, BigInt version ${time2}n`); + } else if (key === 'mode') { + // eslint-disable-next-line no-undef + assert.strictEqual(bigintStats[key], BigInt(val)); + assert.strictEqual( + bigintStats.isBlockDevice(), + numStats.isBlockDevice() + ); + assert.strictEqual( + bigintStats.isCharacterDevice(), + numStats.isCharacterDevice() + ); + assert.strictEqual( + bigintStats.isDirectory(), + numStats.isDirectory() + ); + assert.strictEqual( + bigintStats.isFIFO(), + numStats.isFIFO() + ); + assert.strictEqual( + bigintStats.isFile(), + numStats.isFile() + ); + assert.strictEqual( + bigintStats.isSocket(), + numStats.isSocket() + ); + assert.strictEqual( + bigintStats.isSymbolicLink(), + numStats.isSymbolicLink() + ); + } else if (common.isWindows && (key === 'blksize' || key === 'blocks')) { + assert.strictEqual(bigintStats[key], undefined); + assert.strictEqual(numStats[key], undefined); + } else if (Number.isSafeInteger(val)) { + // eslint-disable-next-line no-undef + assert.strictEqual(bigintStats[key], BigInt(val)); + } else { + assert( + Math.abs(Number(bigintStats[key]) - val) < 1, + `${key} is not a safe integer, difference should < 1.\n` + + `Number version ${val}, BigInt version ${bigintStats[key]}n`); + } + } +} + +{ + const bigintStats = fs.statSync(fn, { bigint: true }); + const numStats = fs.statSync(fn); + verifyStats(bigintStats, numStats); +} + +if (!common.isWindows) { + const bigintStats = fs.lstatSync(link, { bigint: true }); + const numStats = fs.lstatSync(link); + verifyStats(bigintStats, numStats); +} + +{ + const fd = fs.openSync(fn, 'r'); + const bigintStats = fs.fstatSync(fd, { bigint: true }); + const numStats = fs.fstatSync(fd); + verifyStats(bigintStats, numStats); + fs.closeSync(fd); +} + +{ + fs.stat(fn, { bigint: true }, (err, bigintStats) => { + fs.stat(fn, (err, numStats) => { + verifyStats(bigintStats, numStats); + }); + }); +} + +if (!common.isWindows) { + fs.lstat(link, { bigint: true }, (err, bigintStats) => { + fs.lstat(link, (err, numStats) => { + verifyStats(bigintStats, numStats); + }); + }); +} + +{ + const fd = fs.openSync(fn, 'r'); + fs.fstat(fd, { bigint: true }, (err, bigintStats) => { + fs.fstat(fd, (err, numStats) => { + verifyStats(bigintStats, numStats); + fs.closeSync(fd); + }); + }); +} + +(async function() { + const bigintStats = await promiseFs.stat(fn, { bigint: true }); + const numStats = await promiseFs.stat(fn); + verifyStats(bigintStats, numStats); +})(); + +if (!common.isWindows) { + (async function() { + const bigintStats = await promiseFs.lstat(link, { bigint: true }); + const numStats = await promiseFs.lstat(link); + verifyStats(bigintStats, numStats); + })(); +} + +(async function() { + const handle = await promiseFs.open(fn, 'r'); + const bigintStats = await handle.stat({ bigint: true }); + const numStats = await handle.stat(); + verifyStats(bigintStats, numStats); + await handle.close(); +})(); diff --git a/test/parallel/test-fs-sync-fd-leak.js b/test/parallel/test-fs-sync-fd-leak.js index e7107546e5b217..52e40eb3901f4c 100644 --- a/test/parallel/test-fs-sync-fd-leak.js +++ b/test/parallel/test-fs-sync-fd-leak.js @@ -40,7 +40,7 @@ fs.writeSync = function() { throw new Error('BAM'); }; -process.binding('fs').fstat = function(fd, _, ctx) { +process.binding('fs').fstat = function(fd, bigint, _, ctx) { ctx.errno = uv.UV_EBADF; ctx.syscall = 'fstat'; }; diff --git a/test/parallel/test-fs-watchfile-bigint.js b/test/parallel/test-fs-watchfile-bigint.js new file mode 100644 index 00000000000000..89cefd12e0443f --- /dev/null +++ b/test/parallel/test-fs-watchfile-bigint.js @@ -0,0 +1,63 @@ +'use strict'; +const common = require('../common'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const tmpdir = require('../common/tmpdir'); + +const enoentFile = path.join(tmpdir.path, 'non-existent-file'); +const expectedStatObject = new fs.Stats( + 0n, // dev + 0n, // mode + 0n, // nlink + 0n, // uid + 0n, // gid + 0n, // rdev + common.isWindows ? undefined : 0n, // blksize + 0n, // ino + 0n, // size + common.isWindows ? undefined : 0n, // blocks + 0n, // atim_msec + 0n, // mtim_msec + 0n, // ctim_msec + 0n // birthtim_msec +); + +tmpdir.refresh(); + +// If the file initially didn't exist, and gets created at a later point of +// time, the callback should be invoked again with proper values in stat object +let fileExists = false; +const options = { interval: 0, bigint: true }; + +const watcher = + fs.watchFile(enoentFile, options, common.mustCall((curr, prev) => { + if (!fileExists) { + // If the file does not exist, all the fields should be zero and the date + // fields should be UNIX EPOCH time + assert.deepStrictEqual(curr, expectedStatObject); + assert.deepStrictEqual(prev, expectedStatObject); + // Create the file now, so that the callback will be called back once the + // event loop notices it. + fs.closeSync(fs.openSync(enoentFile, 'w')); + fileExists = true; + } else { + // If the ino (inode) value is greater than zero, it means that the file + // is present in the filesystem and it has a valid inode number. + assert(curr.ino > 0n); + // As the file just got created, previous ino value should be lesser than + // or equal to zero (non-existent file). + assert(prev.ino <= 0n); + // Stop watching the file + fs.unwatchFile(enoentFile); + watcher.stop(); // stopping a stopped watcher should be a noop + } + }, 2)); + +// 'stop' should only be emitted once - stopping a stopped watcher should +// not trigger a 'stop' event. +watcher.on('stop', common.mustCall(function onStop() {})); + +watcher.start(); // starting a started watcher should be a noop From fe5d35123bdd1f8a2c5cc8ab636491012b9584fe Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 23 Apr 2018 17:14:56 +0800 Subject: [PATCH 053/150] doc: document BigInt support in fs.Stats PR-URL: https://github.com/nodejs/node/pull/20220 Fixes: https://github.com/nodejs/node/issues/12115 Reviewed-By: Ben Noordhuis --- doc/api/fs.md | 145 ++++++++++++++++++++++++++++++++------- tools/doc/type-parser.js | 2 + 2 files changed, 121 insertions(+), 26 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 02bf4abdf8b13b..55ba07a002965a 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -415,6 +415,8 @@ A `fs.Stats` object provides information about a file. Objects returned from [`fs.stat()`][], [`fs.lstat()`][] and [`fs.fstat()`][] and their synchronous counterparts are of this type. +If `bigint` in the `options` passed to those methods is true, the numeric values +will be `bigint` instead of `number`. ```console Stats { @@ -438,6 +440,30 @@ Stats { birthtime: Mon, 10 Oct 2011 23:24:11 GMT } ``` +`bigint` version: + +```console +Stats { + dev: 2114n, + ino: 48064969n, + mode: 33188n, + nlink: 1n, + uid: 85n, + gid: 100n, + rdev: 0n, + size: 527n, + blksize: 4096n, + blocks: 8n, + atimeMs: 1318289051000n, + mtimeMs: 1318289051000n, + ctimeMs: 1318289051000n, + birthtimeMs: 1318289051000n, + atime: Mon, 10 Oct 2011 23:24:11 GMT, + mtime: Mon, 10 Oct 2011 23:24:11 GMT, + ctime: Mon, 10 Oct 2011 23:24:11 GMT, + birthtime: Mon, 10 Oct 2011 23:24:11 GMT } +``` + ### stats.isBlockDevice() -* {number} +* {number|bigint} The timestamp indicating the last time this file was accessed expressed in milliseconds since the POSIX Epoch. @@ -579,7 +605,7 @@ milliseconds since the POSIX Epoch. added: v8.1.0 --> -* {number} +* {number|bigint} The timestamp indicating the last time this file was modified expressed in milliseconds since the POSIX Epoch. @@ -589,7 +615,7 @@ milliseconds since the POSIX Epoch. added: v8.1.0 --> -* {number} +* {number|bigint} The timestamp indicating the last time the file status was changed expressed in milliseconds since the POSIX Epoch. @@ -599,7 +625,7 @@ in milliseconds since the POSIX Epoch. added: v8.1.0 --> -* {number} +* {number|bigint} The timestamp indicating the creation time of this file expressed in milliseconds since the POSIX Epoch. @@ -1643,7 +1669,7 @@ added: v0.1.96 Synchronous fdatasync(2). Returns `undefined`. -## fs.fstat(fd, callback) +## fs.fstat(fd[, options], callback) * `fd` {integer} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * `callback` {Function} * `err` {Error} * `stats` {fs.Stats} @@ -1666,12 +1699,20 @@ Asynchronous fstat(2). The callback gets two arguments `(err, stats)` where `stats` is an [`fs.Stats`][] object. `fstat()` is identical to [`stat()`][], except that the file to be stat-ed is specified by the file descriptor `fd`. -## fs.fstatSync(fd) +## fs.fstatSync(fd[, options]) * `fd` {integer} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {fs.Stats} Synchronous fstat(2). @@ -1937,7 +1978,7 @@ changes: Synchronous link(2). Returns `undefined`. -## fs.lstat(path, callback) +## fs.lstat(path[, options], callback) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * `callback` {Function} * `err` {Error} * `stats` {fs.Stats} @@ -1965,7 +2013,7 @@ Asynchronous lstat(2). The callback gets two arguments `(err, stats)` where except that if `path` is a symbolic link, then the link itself is stat-ed, not the file that it refers to. -## fs.lstatSync(path) +## fs.lstatSync(path[, options]) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {fs.Stats} Synchronous lstat(2). @@ -2693,7 +2748,7 @@ Synchronous rmdir(2). Returns `undefined`. Using `fs.rmdirSync()` on a file (not a directory) results in an `ENOENT` error on Windows and an `ENOTDIR` error on POSIX. -## fs.stat(path, callback) +## fs.stat(path[, options], callback) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * `callback` {Function} * `err` {Error} * `stats` {fs.Stats} @@ -2729,7 +2791,7 @@ error raised if the file is not available. To check if a file exists without manipulating it afterwards, [`fs.access()`] is recommended. -## fs.statSync(path) +## fs.statSync(path[, options]) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {fs.Stats} Synchronous stat(2). @@ -3516,10 +3585,18 @@ returned. The `FileHandle` has to support reading. -#### filehandle.stat() +#### filehandle.stat([options]) +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {Promise} Retrieves the [`fs.Stats`][] for the file. @@ -3844,12 +3921,20 @@ added: v10.0.0 Asynchronous link(2). The `Promise` is resolved with no arguments upon success. -### fsPromises.lstat(path) +### fsPromises.lstat(path[, options]) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {Promise} Asynchronous lstat(2). The `Promise` is resolved with the [`fs.Stats`][] object @@ -4030,12 +4115,20 @@ Using `fsPromises.rmdir()` on a file (not a directory) results in the `Promise` being rejected with an `ENOENT` error on Windows and an `ENOTDIR` error on POSIX. -### fsPromises.stat(path) +### fsPromises.stat(path[, options]) * `path` {string|Buffer|URL} +* `options` {Object} + * `bigint` {boolean} Whether the numeric values in the returned + [`fs.Stats`][] object should be `bigint`. **Default:** `false`. * Returns: {Promise} The `Promise` is resolved with the [`fs.Stats`][] object for the given `path`. @@ -4491,9 +4584,9 @@ the file contents. [`fs.chown()`]: #fs_fs_chown_path_uid_gid_callback [`fs.copyFile()`]: #fs_fs_copyfile_src_dest_flags_callback [`fs.exists()`]: fs.html#fs_fs_exists_path_callback -[`fs.fstat()`]: #fs_fs_fstat_fd_callback +[`fs.fstat()`]: #fs_fs_fstat_fd_options_callback [`fs.futimes()`]: #fs_fs_futimes_fd_atime_mtime_callback -[`fs.lstat()`]: #fs_fs_lstat_path_callback +[`fs.lstat()`]: #fs_fs_lstat_path_options_callback [`fs.mkdir()`]: #fs_fs_mkdir_path_mode_callback [`fs.mkdtemp()`]: #fs_fs_mkdtemp_prefix_options_callback [`fs.open()`]: #fs_fs_open_path_flags_mode_callback @@ -4502,7 +4595,7 @@ the file contents. [`fs.readFileSync()`]: #fs_fs_readfilesync_path_options [`fs.realpath()`]: #fs_fs_realpath_path_options_callback [`fs.rmdir()`]: #fs_fs_rmdir_path_callback -[`fs.stat()`]: #fs_fs_stat_path_callback +[`fs.stat()`]: #fs_fs_stat_path_options_callback [`fs.utimes()`]: #fs_fs_utimes_path_atime_mtime_callback [`fs.watch()`]: #fs_fs_watch_filename_options_listener [`fs.write()`]: #fs_fs_write_fd_buffer_offset_length_position_callback diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js index be72893832373a..87138668c07426 100644 --- a/tools/doc/type-parser.js +++ b/tools/doc/type-parser.js @@ -31,6 +31,8 @@ const customTypesMap = { 'AsyncIterator': 'https://github.com/tc39/proposal-async-iteration', + 'bigint': 'https://github.com/tc39/proposal-bigint', + 'Iterable': `${jsDocPrefix}Reference/Iteration_protocols#The_iterable_protocol`, 'Iterator': From 3d8ec8f85c6c83da381d1b8b0358bae3ac9336b3 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 4 Jun 2018 13:10:09 +0000 Subject: [PATCH 054/150] test: make url-parse-invalid-input engine agnostic test-url-parse-invalid-input checks the message of an error that is generated by the JavaScript engine. Error messages that change in the underlying JavaScript engine should not be breaking changes in Node.js and therefore should not cause tests to fail. Remove the message check and replace it with a check of the type of the Error object along with the absence of a `code` property. (If a `code` property were present, it would indicate that the error was coming from Node.js rather than the JavaScript engine.) This also makes this test usable without modification in the ChakraCore fork of Node.js. PR-URL: https://github.com/nodejs/node/pull/21132 Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat --- test/parallel/test-url-parse-invalid-input.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-url-parse-invalid-input.js b/test/parallel/test-url-parse-invalid-input.js index 0f113862c7b3c4..aad8462bfc2633 100644 --- a/test/parallel/test-url-parse-invalid-input.js +++ b/test/parallel/test-url-parse-invalid-input.js @@ -26,4 +26,12 @@ const url = require('url'); }); assert.throws(() => { url.parse('http://%E0%A4%A@fail'); }, - /^URIError: URI malformed$/); + (e) => { + // The error should be a URIError. + if (!(e instanceof URIError)) + return false; + + // The error should be from the JS engine and not from Node.js. + // JS engine errors do not have the `code` property. + return e.code === undefined; + }); From 14bb905d18e0fbf16a17c8fa55d626b4ea9146fd Mon Sep 17 00:00:00 2001 From: Yang Guo Date: Wed, 30 May 2018 09:08:12 +0200 Subject: [PATCH 055/150] deps: V8: cherry-pick a440efb27f from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [api] do not require source string for producing code cache. The embedder should not need to keep track of the source string. R=jgruber@chromium.org Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: Ie27df755a22fbcae7b6e87a435419d2d8f545558 Reviewed-on: https://chromium-review.googlesource.com/1013482 Reviewed-by: Jakob Gruber Commit-Queue: Yang Guo Cr-Commit-Position: refs/heads/master@{#52614} PR-URL: https://github.com/nodejs/node/pull/21022 Reviewed-By: Ben Noordhuis Reviewed-By: Michaël Zasso Reviewed-By: Gus Caplan Reviewed-By: Tiancheng "Timothy" Gu --- common.gypi | 2 +- deps/v8/include/v8.h | 6 ++++++ deps/v8/src/api.cc | 16 ++++++++++++---- deps/v8/src/d8.cc | 4 ++-- deps/v8/src/snapshot/code-serializer.cc | 13 +++++++------ deps/v8/src/snapshot/code-serializer.h | 5 ++--- deps/v8/test/cctest/test-api.cc | 3 +-- deps/v8/test/cctest/test-serialize.cc | 11 +++++------ 8 files changed, 36 insertions(+), 24 deletions(-) diff --git a/common.gypi b/common.gypi index 641aa90b17c1d3..dbf49d11ce3279 100644 --- a/common.gypi +++ b/common.gypi @@ -27,7 +27,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.7', + 'v8_embedder_string': '-node.8', # Enable disassembler for `--print-code` v8 options 'v8_enable_disassembler': 1, diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index dcee4eed4e8bc4..ab77238a77f97f 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -1578,6 +1578,9 @@ class V8_EXPORT ScriptCompiler { * This will return nullptr if the script cannot be serialized. The * CachedData returned by this function should be owned by the caller. */ + static CachedData* CreateCodeCache(Local unbound_script); + + // Deprecated. static CachedData* CreateCodeCache(Local unbound_script, Local source); @@ -1587,6 +1590,9 @@ class V8_EXPORT ScriptCompiler { * This will return nullptr if the script cannot be serialized. The * CachedData returned by this function should be owned by the caller. */ + static CachedData* CreateCodeCacheForFunction(Local function); + + // Deprecated. static CachedData* CreateCodeCacheForFunction(Local function, Local source); diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index 25506d3930868d..e9a5ec69ec4a71 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -2626,21 +2626,29 @@ uint32_t ScriptCompiler::CachedDataVersionTag() { ScriptCompiler::CachedData* ScriptCompiler::CreateCodeCache( Local unbound_script, Local source) { + return CreateCodeCache(unbound_script); +} + +ScriptCompiler::CachedData* ScriptCompiler::CreateCodeCache( + Local unbound_script) { i::Handle shared = i::Handle::cast( Utils::OpenHandle(*unbound_script)); - i::Handle source_str = Utils::OpenHandle(*source); DCHECK(shared->is_toplevel()); - return i::CodeSerializer::Serialize(shared, source_str); + return i::CodeSerializer::Serialize(shared); } ScriptCompiler::CachedData* ScriptCompiler::CreateCodeCacheForFunction( Local function, Local source) { + return CreateCodeCacheForFunction(function); +} + +ScriptCompiler::CachedData* ScriptCompiler::CreateCodeCacheForFunction( + Local function) { i::Handle shared( i::Handle::cast(Utils::OpenHandle(*function))->shared()); - i::Handle source_str = Utils::OpenHandle(*source); CHECK(shared->is_wrapped()); - return i::CodeSerializer::Serialize(shared, source_str); + return i::CodeSerializer::Serialize(shared); } MaybeLocal