From b22c86ed3bd3242177b02a34292febe41275a52b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 26 Dec 2018 19:53:36 +0800 Subject: [PATCH] src: use `internalBinding('config').hasInspector` in JS land Instead of `process.config.variables.v8_enable_inspector` which depends on the variable name in gyp files, or detecting `internalBinding('inspector').Connection`. PR-URL: https://github.com/nodejs/node/pull/25291 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/inspector.js | 10 ++++++---- lib/internal/bootstrap/cache.js | 4 ++-- lib/internal/bootstrap/node.js | 7 ++++--- lib/internal/process/coverage.js | 5 +++-- lib/internal/util/inspector.js | 2 +- src/node_config.cc | 6 ++++++ 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/inspector.js b/lib/inspector.js index aac864901c9..565b3aa9727 100644 --- a/lib/inspector.js +++ b/lib/inspector.js @@ -1,6 +1,5 @@ 'use strict'; -const EventEmitter = require('events'); const { ERR_INSPECTOR_ALREADY_CONNECTED, ERR_INSPECTOR_CLOSED, @@ -9,13 +8,16 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_CALLBACK } = require('internal/errors').codes; + +const { hasInspector } = internalBinding('config'); +if (!hasInspector) + throw new ERR_INSPECTOR_NOT_AVAILABLE(); + +const EventEmitter = require('events'); const { validateString } = require('internal/validators'); const util = require('util'); const { Connection, open, url } = internalBinding('inspector'); -if (!Connection) - throw new ERR_INSPECTOR_NOT_AVAILABLE(); - const connectionSymbol = Symbol('connectionProperty'); const messageCallbacksSymbol = Symbol('messageCallbacks'); const nextIdSymbol = Symbol('nextId'); diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index 580e7a9cb0d..f69f1252ad8 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -9,7 +9,7 @@ const { NativeModule } = require('internal/bootstrap/loaders'); const { source, getCodeCache, compileFunction } = internalBinding('native_module'); -const { hasTracing } = process.binding('config'); +const { hasTracing, hasInspector } = process.binding('config'); const depsModule = Object.keys(source).filter( (key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps') @@ -33,7 +33,7 @@ const cannotUseCache = [ // Skip modules that cannot be required when they are not // built into the binary. -if (process.config.variables.v8_enable_inspector !== 1) { +if (!hasInspector) { cannotUseCache.push( 'inspector', 'internal/util/inspector', diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index db593e597d4..089143e5d01 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -19,6 +19,7 @@ const { internalBinding, NativeModule } = loaderExports; const { getOptionValue } = NativeModule.require('internal/options'); +const config = internalBinding('config'); function startup() { setupTraceCategoryState(); @@ -164,7 +165,7 @@ function startup() { NativeModule.require('internal/process/coverage').setupExitHooks(); } - if (process.config.variables.v8_enable_inspector) { + if (config.hasInspector) { NativeModule.require('internal/inspector_async_hook').setup(); } @@ -280,7 +281,7 @@ function startup() { // TODO(joyeecheung): this property has not been well-maintained, should we // deprecate it in favor of a better API? - const { isDebugBuild, hasOpenSSL } = internalBinding('config'); + const { isDebugBuild, hasOpenSSL } = config; Object.defineProperty(process, 'features', { enumerable: true, writable: false, @@ -630,7 +631,7 @@ function setupGlobalConsole() { writable: true }); // TODO(joyeecheung): can we skip this if inspector is not active? - if (process.config.variables.v8_enable_inspector) { + if (config.hasInspector) { const inspector = NativeModule.require('internal/console/inspector'); inspector.addInspectorApis(consoleFromNode, consoleFromVM); diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js index ef37deba8a5..95235c8ac91 100644 --- a/lib/internal/process/coverage.js +++ b/lib/internal/process/coverage.js @@ -51,12 +51,13 @@ function disableAllAsyncHooks() { exports.writeCoverage = writeCoverage; function setup() { - const { Connection } = internalBinding('inspector'); - if (!Connection) { + const { hasInspector } = internalBinding('config'); + if (!hasInspector) { process._rawDebug('inspector not enabled'); return; } + const { Connection } = internalBinding('inspector'); coverageConnection = new Connection((res) => { if (coverageConnection._coverageCallback) { coverageConnection._coverageCallback(res); diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 7d22fdc80ec..face748ab0d 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -1,6 +1,6 @@ 'use strict'; -const hasInspector = process.config.variables.v8_enable_inspector === 1; +const { hasInspector } = internalBinding('config'); const inspector = hasInspector ? require('inspector') : undefined; let session; diff --git a/src/node_config.cc b/src/node_config.cc index 7204e246562..8756969cda2 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -57,6 +57,12 @@ static void Initialize(Local target, READONLY_TRUE_PROPERTY(target, "hasTracing"); #endif +#if HAVE_INSPECTOR + READONLY_TRUE_PROPERTY(target, "hasInspector"); +#else + READONLY_FALSE_PROPERTY(target, "hasInspector"); +#endif + #if !defined(NODE_WITHOUT_NODE_OPTIONS) READONLY_TRUE_PROPERTY(target, "hasNodeOptions"); #endif