Skip to content

Commit

Permalink
module: do not share the internal require function with public loaders
Browse files Browse the repository at this point in the history 
This patch removes `NativeModule.require` and
`NativeModule.requireWithFallbackInDeps`. The public loaders now
have to use a special method
`NativeModule.prototype.compileForPublicLoader()` to compile native
modules. In addition this patch moves the decisions of proxifying
exports and throwing unknown builtin errors entirely to public
loaders, and skip those during internal use - therefore `loaders.js`,
which is compiled during bootstrap, no longer needs to be aware of
the value of `--experimental-modules`.

PR-URL: #26549
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
@joyeecheung @targos
joyeecheung authored and targos committed Mar 27, 2019
1 parent 3c6f12c commit 235bb73
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 63 deletions.
57 changes: 27 additions & 30 deletions lib/internal/bootstrap/loaders.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// can be created using NODE_MODULE_CONTEXT_AWARE_CPP() with the flag
// NM_F_LINKED.
// - internalBinding(): the private internal C++ binding loader, inaccessible
// from user land because they are only available from NativeModule.require().
// from user land unless through `require('internal/test/binding')`.
// These C++ bindings are created using NODE_MODULE_CONTEXT_AWARE_INTERNAL()
// and have their nm_flags set to NM_F_INTERNAL.
//
Expand All @@ -42,7 +42,7 @@
// This file is compiled as if it's wrapped in a function with arguments
// passed by node::RunBootstrapping()
/* global process, getLinkedBinding, getInternalBinding */
/* global experimentalModules, exposeInternals, primordials */
/* global exposeInternals, primordials */

const {
Reflect,
Expand Down Expand Up @@ -185,27 +185,9 @@ function nativeModuleRequire(id) {
}

const mod = NativeModule.map.get(id);
if (!mod) {
// Model the error off the internal/errors.js model, but
// do not use that module given that it could actually be
// the one causing the error if there's a bug in Node.js.
// eslint-disable-next-line no-restricted-syntax
const err = new Error(`No such built-in module: ${id}`);
err.code = 'ERR_UNKNOWN_BUILTIN_MODULE';
err.name = 'Error [ERR_UNKNOWN_BUILTIN_MODULE]';
throw err;
}

if (mod.loaded || mod.loading) {
return mod.exports;
}

moduleLoadList.push(`NativeModule ${id}`);
mod.compile();
return mod.exports;
return mod.compile();
}

NativeModule.require = nativeModuleRequire;
NativeModule.exists = function(id) {
return NativeModule.map.has(id);
};
Expand All @@ -217,11 +199,25 @@ NativeModule.canBeRequiredByUsers = function(id) {

// Allow internal modules from dependencies to require
// other modules from dependencies by providing fallbacks.
NativeModule.requireWithFallbackInDeps = function(request) {
function requireWithFallbackInDeps(request) {
if (!NativeModule.map.has(request)) {
request = `internal/deps/${request}`;
}
return NativeModule.require(request);
return nativeModuleRequire(request);
}

// This is exposed for public loaders
NativeModule.prototype.compileForPublicLoader = function(needToProxify) {
if (!this.canBeRequiredByUsers) {
// No code because this is an assertion against bugs
// eslint-disable-next-line no-restricted-syntax
throw new Error(`Should not compile ${this.id} for public use`);
}
this.compile();
if (needToProxify && !this.exportKeys) {
this.proxifyExports();
}
return this.exports;
};

const getOwn = (target, property, receiver) => {
Expand Down Expand Up @@ -289,26 +285,27 @@ NativeModule.prototype.proxifyExports = function() {
};

NativeModule.prototype.compile = function() {
const id = this.id;
if (this.loaded || this.loading) {
return this.exports;
}

const id = this.id;
this.loading = true;

try {
const requireFn = this.id.startsWith('internal/deps/') ?
NativeModule.requireWithFallbackInDeps :
NativeModule.require;
requireWithFallbackInDeps : nativeModuleRequire;

const fn = compileFunction(id);
fn(this.exports, requireFn, this, process, internalBinding, primordials);

if (experimentalModules && this.canBeRequiredByUsers) {
this.proxifyExports();
}

this.loaded = true;
} finally {
this.loading = false;
}

moduleLoadList.push(`NativeModule ${id}`);
return this.exports;
};

// This will be passed to internal/bootstrap/node.js.
Expand Down
40 changes: 19 additions & 21 deletions lib/internal/bootstrap/node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,12 @@

// This file is compiled as if it's wrapped in a function with arguments
// passed by node::RunBootstrapping()
/* global process, loaderExports, isMainThread, ownsProcessState */
/* global process, require, internalBinding, isMainThread, ownsProcessState */
/* global primordials */

const { internalBinding, NativeModule } = loaderExports;
const { Object, Symbol } = primordials;
const config = internalBinding('config');
const { deprecate } = NativeModule.require('internal/util');
const { deprecate } = require('internal/util');

setupProcessObject();

Expand All @@ -50,17 +49,17 @@ process.domain = null;
process._exiting = false;

// Bootstrappers for all threads, including worker threads and main thread
const perThreadSetup = NativeModule.require('internal/process/per_thread');
const perThreadSetup = require('internal/process/per_thread');
// Bootstrappers for the main thread only
let mainThreadSetup;
// Bootstrappers for the worker threads only
let workerThreadSetup;
if (ownsProcessState) {
mainThreadSetup = NativeModule.require(
mainThreadSetup = require(
'internal/process/main_thread_only'
);
} else {
workerThreadSetup = NativeModule.require(
workerThreadSetup = require(
'internal/process/worker_thread_only'
);
}
Expand Down Expand Up @@ -116,14 +115,14 @@ if (isMainThread) {

const {
emitWarning
} = NativeModule.require('internal/process/warning');
} = require('internal/process/warning');

process.emitWarning = emitWarning;

const {
nextTick,
runNextTicks
} = NativeModule.require('internal/process/next_tick').setup();
} = require('internal/process/next_tick').setup();

process.nextTick = nextTick;
// Used to emulate a tick manually in the JS land.
Expand Down Expand Up @@ -161,7 +160,7 @@ if (credentials.implementsPosixCredentials) {

if (isMainThread) {
const { getStdout, getStdin, getStderr } =
NativeModule.require('internal/process/stdio').getMainThreadStdio();
require('internal/process/stdio').getMainThreadStdio();
setupProcessStdio(getStdout, getStdin, getStderr);
} else {
const { getStdout, getStdin, getStderr } =
Expand All @@ -173,7 +172,7 @@ if (isMainThread) {
// process. They use the same functions as the JS embedder API. These callbacks
// are setup immediately to prevent async_wrap.setupHooks() from being hijacked
// and the cost of doing so is negligible.
const { nativeHooks } = NativeModule.require('internal/async_hooks');
const { nativeHooks } = require('internal/async_hooks');
internalBinding('async_wrap').setupHooks(nativeHooks);

// XXX(joyeecheung): this has to be done after the initial load of
Expand All @@ -183,7 +182,7 @@ if (config.hasInspector) {
const {
enable,
disable
} = NativeModule.require('internal/inspector_async_hook');
} = require('internal/inspector_async_hook');
internalBinding('inspector').registerAsyncHook(enable, disable);
}

Expand All @@ -193,22 +192,22 @@ if (!config.noBrowserGlobals) {
// https://console.spec.whatwg.org/#console-namespace
exposeNamespace(global, 'console', createGlobalConsole(global.console));

const { URL, URLSearchParams } = NativeModule.require('internal/url');
const { URL, URLSearchParams } = require('internal/url');
// https://url.spec.whatwg.org/#url
exposeInterface(global, 'URL', URL);
// https://url.spec.whatwg.org/#urlsearchparams
exposeInterface(global, 'URLSearchParams', URLSearchParams);

const {
TextEncoder, TextDecoder
} = NativeModule.require('internal/encoding');
} = require('internal/encoding');
// https://encoding.spec.whatwg.org/#textencoder
exposeInterface(global, 'TextEncoder', TextEncoder);
// https://encoding.spec.whatwg.org/#textdecoder
exposeInterface(global, 'TextDecoder', TextDecoder);

// https://html.spec.whatwg.org/multipage/webappapis.html#windoworworkerglobalscope
const timers = NativeModule.require('timers');
const timers = require('timers');
defineOperation(global, 'clearInterval', timers.clearInterval);
defineOperation(global, 'clearTimeout', timers.clearTimeout);
defineOperation(global, 'setInterval', timers.setInterval);
Expand Down Expand Up @@ -302,7 +301,7 @@ Object.defineProperty(process, 'features', {
fatalException,
setUncaughtExceptionCaptureCallback,
hasUncaughtExceptionCaptureCallback
} = NativeModule.require('internal/process/execution');
} = require('internal/process/execution');

process._fatalException = fatalException;
process.setUncaughtExceptionCaptureCallback =
Expand All @@ -312,7 +311,7 @@ Object.defineProperty(process, 'features', {
}

function setupProcessObject() {
const EventEmitter = NativeModule.require('events');
const EventEmitter = require('events');
const origProcProto = Object.getPrototypeOf(process);
Object.setPrototypeOf(origProcProto, EventEmitter.prototype);
EventEmitter.call(process);
Expand Down Expand Up @@ -391,7 +390,7 @@ function setupGlobalProxy() {
}

function setupBuffer() {
const { Buffer } = NativeModule.require('buffer');
const { Buffer } = require('buffer');
const bufferBinding = internalBinding('buffer');

// Only after this point can C++ use Buffer::New()
Expand All @@ -404,9 +403,9 @@ function setupBuffer() {

function createGlobalConsole(consoleFromVM) {
const consoleFromNode =
NativeModule.require('internal/console/global');
require('internal/console/global');
if (config.hasInspector) {
const inspector = NativeModule.require('internal/util/inspector');
const inspector = require('internal/util/inspector');
// This will be exposed by `require('inspector').console` later.
inspector.consoleFromVM = consoleFromVM;
// TODO(joyeecheung): postpone this until the first time inspector
Expand All @@ -424,8 +423,7 @@ function setupQueueMicrotask() {
get() {
process.emitWarning('queueMicrotask() is experimental.',
'ExperimentalWarning');
const { queueMicrotask } =
NativeModule.require('internal/queue_microtask');
const { queueMicrotask } = require('internal/queue_microtask');

Object.defineProperty(global, 'queueMicrotask', {
value: queueMicrotask,
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,7 @@ E('ERR_UNHANDLED_ERROR',
if (err === undefined) return msg;
return `${msg} (${err})`;
}, Error);
E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);

Expand Down
9 changes: 5 additions & 4 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,8 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {

// Check the cache for the requested file.
// 1. If a module already exists in the cache: return its exports object.
// 2. If the module is native: call `NativeModule.require()` with the
// filename and return the result.
// 2. If the module is native: call
// `NativeModule.prototype.compileForPublicLoader()` and return the exports.
// 3. Otherwise, create a new module for the file and save it to the cache.
// Then have it load the file contents before returning its exports
// object.
Expand All @@ -585,9 +585,10 @@ Module._load = function(request, parent, isMain) {
return cachedModule.exports;
}

if (NativeModule.canBeRequiredByUsers(filename)) {
const mod = NativeModule.map.get(filename);
if (mod && mod.canBeRequiredByUsers) {
debug('load native module %s', request);
return NativeModule.require(filename);
return mod.compileForPublicLoader(experimentalModules);
}

// Don't call updateChildren(), Module constructor already does.
Expand Down
9 changes: 7 additions & 2 deletions lib/internal/modules/esm/translators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const { URL } = require('url');
const { debuglog } = require('internal/util/debuglog');
const { promisify } = require('internal/util');
const esmLoader = require('internal/process/esm_loader');

const {
ERR_UNKNOWN_BUILTIN_MODULE
} = require('internal/errors').codes;
const readFileAsync = promisify(fs.readFile);
const readFileSync = fs.readFileSync;
const StringReplace = FunctionPrototype.call.bind(StringPrototype.replace);
Expand Down Expand Up @@ -86,8 +88,11 @@ translators.set('builtin', async (url) => {
debug(`Translating BuiltinModule ${url}`);
// slice 'node:' scheme
const id = url.slice(5);
NativeModule.require(id);
const module = NativeModule.map.get(id);
if (!module) {
throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
}
module.compileForPublicLoader(true);
return createDynamicModule(
[...module.exportKeys, 'default'], url, (reflect) => {
debug(`Loading BuiltinModule ${url}`);
Expand Down
12 changes: 6 additions & 6 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,6 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
env->process_string(),
FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"),
FIXED_ONE_BYTE_STRING(isolate, "getInternalBinding"),
// --experimental-modules
FIXED_ONE_BYTE_STRING(isolate, "experimentalModules"),
// --expose-internals
FIXED_ONE_BYTE_STRING(isolate, "exposeInternals"),
env->primordials_string()};
Expand All @@ -311,7 +309,6 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
env->NewFunctionTemplate(binding::GetInternalBinding)
->GetFunction(context)
.ToLocalChecked(),
Boolean::New(isolate, env->options()->experimental_modules),
Boolean::New(isolate, env->options()->expose_internals),
env->primordials()};

Expand All @@ -333,16 +330,19 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
loader_exports_obj->Get(context, env->require_string()).ToLocalChecked();
env->set_native_module_require(require.As<Function>());

// process, loaderExports, isMainThread, ownsProcessState, primordials
// process, require, internalBinding, isMainThread,
// ownsProcessState, primordials
std::vector<Local<String>> node_params = {
env->process_string(),
FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
env->require_string(),
env->internal_binding_string(),
FIXED_ONE_BYTE_STRING(isolate, "isMainThread"),
FIXED_ONE_BYTE_STRING(isolate, "ownsProcessState"),
env->primordials_string()};
std::vector<Local<Value>> node_args = {
process,
loader_exports_obj,
require,
internal_binding_loader,
Boolean::New(isolate, env->is_main_thread()),
Boolean::New(isolate, env->owns_process_state()),
env->primordials()};
Expand Down

0 comments on commit 235bb73

Please sign in to comment.