From 190a830ae0687c4b367594e589eb6409ef8d46cf Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 14 Mar 2019 00:58:16 +0800 Subject: [PATCH 1/4] process: handle node --debug deprecation in pre-execution In addition, shim `process._deprecatedDebugBrk` in pre-execution. This is a non-semver-major v11.x backport for https://github.com/nodejs/node/pull/25828. Refs: https://github.com/nodejs/node/pull/25828 --- lib/internal/bootstrap/node.js | 14 -------------- lib/internal/bootstrap/pre_execution.js | 17 +++++++++++++++++ src/node_process_object.cc | 12 ------------ 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 0b0fd7c24acad1..af31a2c1a23910 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -222,20 +222,6 @@ Object.defineProperty(process, 'argv0', { }); process.argv[0] = process.execPath; -// Handle `--debug*` deprecation and invalidation. -if (process._invalidDebug) { - process.emitWarning( - '`node --debug` and `node --debug-brk` are invalid. ' + - 'Please use `node --inspect` or `node --inspect-brk` instead.', - 'DeprecationWarning', 'DEP0062', undefined, true); - process.exit(9); -} else if (process._deprecatedDebugBrk) { - process.emitWarning( - '`node --inspect --debug-brk` is deprecated. ' + - 'Please use `node --inspect-brk` instead.', - 'DeprecationWarning', 'DEP0062', undefined, true); -} - // TODO(jasnell): The following have been globals since around 2012. // That's just silly. The underlying perfctr support has been removed // so these are now deprecated non-ops that can be removed after one diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 02acf6b46f6338..7929cbf2460c08 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -106,6 +106,23 @@ function initializeDeprecations() { const { deprecate } = require('internal/util'); const pendingDeprecation = getOptionValue('--pending-deprecation'); + // Handle `--debug*` deprecation and invalidation. + if (getOptionValue('--debug')) { + if (!getOptionValue('--inspect')) { + process.emitWarning( + '`node --debug` and `node --debug-brk` are invalid. ' + + 'Please use `node --inspect` or `node --inspect-brk` instead.', + 'DeprecationWarning', 'DEP0062', undefined, true); + process.exit(9); + } else if (getOptionValue('--inspect-brk')) { + process._deprecatedDebugBrk = true; + process.emitWarning( + '`node --inspect --debug-brk` is deprecated. ' + + 'Please use `node --inspect-brk` instead.', + 'DeprecationWarning', 'DEP0062', undefined, true); + } + } + // DEP0103: access to `process.binding('util').isX` type checkers // TODO(addaleax): Turn into a full runtime deprecation. const utilBinding = internalBinding('util'); diff --git a/src/node_process_object.cc b/src/node_process_object.cc index e4d9d97c681bdd..e3b8eb8d54b7ba 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -256,18 +256,6 @@ MaybeLocal CreateProcessObject( "_breakNodeFirstLine", True(env->isolate())); } - // --inspect --debug-brk - if (env->options()->debug_options().deprecated_invocation()) { - READONLY_DONT_ENUM_PROPERTY(process, - "_deprecatedDebugBrk", True(env->isolate())); - } - - // --debug or, --debug-brk without --inspect - if (env->options()->debug_options().invalid_invocation()) { - READONLY_DONT_ENUM_PROPERTY(process, - "_invalidDebug", True(env->isolate())); - } - // --security-revert flags #define V(code, _, __) \ do { \ From e71c74ed5118715ce68133c5d8c744d568bae65e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 6 Mar 2019 12:02:04 +0100 Subject: [PATCH 2/4] process: call `prepareMainThreadExecution` in `node inspect` Since we should treat the node-inspect as third-party user code. PR-URL: https://github.com/nodejs/node/pull/26466 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- lib/internal/main/inspect.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/internal/main/inspect.js b/lib/internal/main/inspect.js index 7376f8984b13e1..f6777fe852bcfc 100644 --- a/lib/internal/main/inspect.js +++ b/lib/internal/main/inspect.js @@ -2,6 +2,12 @@ // `node inspect ...` or `node debug ...` +const { + prepareMainThreadExecution +} = require('internal/bootstrap/pre_execution'); + +prepareMainThreadExecution(); + if (process.argv[1] === 'debug') { process.emitWarning( '`node debug` is deprecated. Please use `node inspect` instead.', From 50e47229b327be5459df6d0d0c8f1bf5b8b9301f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 6 Mar 2019 12:05:23 +0100 Subject: [PATCH 3/4] process: set up process warning handler in pre-execution Since it depends on environment variables. PR-URL: https://github.com/nodejs/node/pull/26466 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- lib/internal/bootstrap/node.js | 5 +---- lib/internal/bootstrap/pre_execution.js | 12 ++++++++++++ lib/internal/main/worker_thread.js | 3 +++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index af31a2c1a23910..ac56ae3c00651b 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -115,12 +115,9 @@ if (isMainThread) { } const { - onWarning, emitWarning } = NativeModule.require('internal/process/warning'); -if (!process.noProcessWarnings && process.env.NODE_NO_WARNINGS !== '1') { - process.on('warning', onWarning); -} + process.emitWarning = emitWarning; const { diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 7929cbf2460c08..19ca59fa0cf7ff 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -8,6 +8,8 @@ let traceEventsAsyncHook; function prepareMainThreadExecution() { setupTraceCategoryState(); + setupWarningHandler(); + // Only main thread receives signals. setupSignalHandlers(); @@ -36,6 +38,15 @@ function prepareMainThreadExecution() { loadPreloadModules(); } +function setupWarningHandler() { + const { + onWarning + } = require('internal/process/warning'); + if (!process.noProcessWarnings && process.env.NODE_NO_WARNINGS !== '1') { + process.on('warning', onWarning); + } +} + function initializeReport() { if (!getOptionValue('--experimental-report')) { return; @@ -268,6 +279,7 @@ function loadPreloadModules() { } module.exports = { + setupWarningHandler, prepareMainThreadExecution, initializeDeprecations, initializeESMLoader, diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 0dc1b61ef94f45..4ecceff4e6ec41 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -4,6 +4,7 @@ // message port. const { + setupWarningHandler, initializeDeprecations, initializeESMLoader, initializeFrozenIntrinsics, @@ -39,6 +40,8 @@ const { const publicWorker = require('worker_threads'); const debug = require('util').debuglog('worker'); +setupWarningHandler(); + debug(`[${threadId}] is setting up worker child environment`); // Set up the message port and start listening From 95a38449b9218b17c6f01a6bbc3982376760326c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 6 Mar 2019 12:10:45 +0100 Subject: [PATCH 4/4] process: handle process.env.NODE_V8_COVERAGE in pre-execution Since this depends on environment variable, and the worker threads do not need to persist the variable value because they cannot switch cwd. PR-URL: https://github.com/nodejs/node/pull/26466 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- lib/internal/bootstrap/node.js | 23 -------------------- lib/internal/bootstrap/pre_execution.js | 29 +++++++++++++++++++++++++ lib/internal/main/worker_thread.js | 7 ++++++ 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index ac56ae3c00651b..aff746f9716032 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -303,29 +303,6 @@ Object.defineProperty(process, 'features', { hasUncaughtExceptionCaptureCallback; } -// User-facing NODE_V8_COVERAGE environment variable that writes -// ScriptCoverage to a specified file. -if (process.env.NODE_V8_COVERAGE) { - const originalReallyExit = process.reallyExit; - const cwd = NativeModule.require('internal/process/execution').tryGetCwd(); - const { resolve } = NativeModule.require('path'); - // Resolve the coverage directory to an absolute path, and - // overwrite process.env so that the original path gets passed - // to child processes even when they switch cwd. - const coverageDirectory = resolve(cwd, process.env.NODE_V8_COVERAGE); - process.env.NODE_V8_COVERAGE = coverageDirectory; - const { - writeCoverage, - setCoverageDirectory - } = NativeModule.require('internal/coverage-gen/with_profiler'); - setCoverageDirectory(coverageDirectory); - process.on('exit', writeCoverage); - process.reallyExit = (code) => { - writeCoverage(); - originalReallyExit(code); - }; -} - function setupProcessObject() { const EventEmitter = NativeModule.require('events'); const origProcProto = Object.getPrototypeOf(process); diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 19ca59fa0cf7ff..35c1a510f0b2f2 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -10,6 +10,14 @@ function prepareMainThreadExecution() { setupWarningHandler(); + // Resolve the coverage directory to an absolute path, and + // overwrite process.env so that the original path gets passed + // to child processes even when they switch cwd. + if (process.env.NODE_V8_COVERAGE) { + process.env.NODE_V8_COVERAGE = + setupCoverageHooks(process.env.NODE_V8_COVERAGE); + } + // Only main thread receives signals. setupSignalHandlers(); @@ -47,6 +55,26 @@ function setupWarningHandler() { } } +// Setup User-facing NODE_V8_COVERAGE environment variable that writes +// ScriptCoverage to a specified file. +function setupCoverageHooks(dir) { + const originalReallyExit = process.reallyExit; + const cwd = require('internal/process/execution').tryGetCwd(); + const { resolve } = require('path'); + const coverageDirectory = resolve(cwd, dir); + const { + writeCoverage, + setCoverageDirectory + } = require('internal/coverage-gen/with_profiler'); + setCoverageDirectory(coverageDirectory); + process.on('exit', writeCoverage); + process.reallyExit = (code) => { + writeCoverage(); + originalReallyExit(code); + }; + return coverageDirectory; +} + function initializeReport() { if (!getOptionValue('--experimental-report')) { return; @@ -279,6 +307,7 @@ function loadPreloadModules() { } module.exports = { + setupCoverageHooks, setupWarningHandler, prepareMainThreadExecution, initializeDeprecations, diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 4ecceff4e6ec41..0289f97fb1c110 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -4,6 +4,7 @@ // message port. const { + setupCoverageHooks, setupWarningHandler, initializeDeprecations, initializeESMLoader, @@ -42,6 +43,12 @@ const debug = require('util').debuglog('worker'); setupWarningHandler(); +// Since worker threads cannot switch cwd, we do not need to +// overwrite the process.env.NODE_V8_COVERAGE variable. +if (process.env.NODE_V8_COVERAGE) { + setupCoverageHooks(process.env.NODE_V8_COVERAGE); +} + debug(`[${threadId}] is setting up worker child environment`); // Set up the message port and start listening