module: use compileFunction over Module.wrap

lib/assert.js

@@ -39,7 +39,6 @@ let isDeepEqual;
 let isDeepStrictEqual;
 let parseExpressionAt;
 let findNodeAround;
-let columnOffset = 0;
 let decoder;
 
 function lazyLoadComparison(){
@@ -256,16 +255,6 @@ function getErrMessage(message, fn){
  const line = call.getLineNumber() - 1;
  let column = call.getColumnNumber() - 1;
 
- // Line number one reports the wrong column due to being wrapped in a
- // function. Remove that offset to get the actual call.
- if (line === 0){
- if (columnOffset === 0){
- const{wrapper } = require('internal/modules/cjs/loader');
- columnOffset = wrapper[0].length;
- }
- column -= columnOffset;
- }
-
  const identifier = `${filename}${line}${column}`;
 
  if (errorCache.has(identifier)){

lib/internal/modules/cjs/helpers.js

@@ -1,6 +1,9 @@
 'use strict'
 
 const{validateString } = require('internal/validators');
+const path = require('path');
+const{pathToFileURL } = require('internal/url');
+const{URL } = require('url');
 
 const{
  CHAR_LINE_FEED,
@@ -145,10 +148,18 @@ function addBuiltinLibsToObject(object){
  });
 }
 
+function normalizeReferrerURL(referrer){
+ if (typeof referrer === 'string' && path.isAbsolute(referrer)){
+ return pathToFileURL(referrer).href;
+ }
+ return new URL(referrer).href;
+}
+
 module.exports = exports ={
  addBuiltinLibsToObject,
  builtinLibs,
  makeRequireFunction,
+ normalizeReferrerURL,
  requireDepth: 0,
  stripBOM,
  stripShebang

lib/internal/modules/cjs/loader.js

@@ -29,14 +29,14 @@ const assert = require('assert').ok;
 const fs = require('fs');
 const internalFS = require('internal/fs/utils');
 const path = require('path');
-const{URL } = require('url');
 const{
  internalModuleReadJSON,
  internalModuleStat
 } = internalBinding('fs');
 const{safeGetenv } = internalBinding('credentials');
 const{
  makeRequireFunction,
+ normalizeReferrerURL,
  requireDepth,
  stripBOM,
  stripShebang
@@ -48,6 +48,7 @@ const experimentalModules = getOptionValue('--experimental-modules');
 const manifest = getOptionValue('--experimental-policy') ?
  require('internal/process/policy').manifest :
  null;
+const{compileFunction } = internalBinding('contextify');
 
 const{
  ERR_INVALID_ARG_VALUE,
@@ -131,15 +132,52 @@ Module._extensions = Object.create(null);
 var modulePaths = [];
 Module.globalPaths = [];
 
-Module.wrap = function(script){
+let patched = false;
+
+// eslint-disable-next-line func-style
+let wrap = function(script){
  return Module.wrapper[0] + script + Module.wrapper[1];
 };
 
-Module.wrapper = [
+const wrapper = [
  '(function (exports, require, module, __filename, __dirname){',
  '\n});'
 ];
 
+let wrapperProxy = new Proxy(wrapper,{
+ set(target, property, value, receiver){
+ patched = true;
+ return Reflect.set(target, property, value, receiver);
+ },
+
+ defineProperty(target, property, descriptor){
+ patched = true;
+ return Object.defineProperty(target, property, descriptor);
+ }
+});
+
+Object.defineProperty(Module, 'wrap',{
+ get(){
+ return wrap;
+ },
+
+ set(value){
+ patched = true;
+ wrap = value;
+ }
+});
+
+Object.defineProperty(Module, 'wrapper',{
+ get(){
+ return wrapperProxy;
+ },
+
+ set(value){
+ patched = true;
+ wrapperProxy = value;
+ }
+});
+
 const debug = util.debuglog('module');
 
 Module._debug = util.deprecate(debug, 'Module._debug is deprecated.',
@@ -671,13 +709,6 @@ Module.prototype.require = function(id){
 // (needed for setting breakpoint when called with --inspect-brk)
 var resolvedArgv;
 
-function normalizeReferrerURL(referrer){
- if (typeof referrer === 'string' && path.isAbsolute(referrer)){
- return pathToFileURL(referrer).href;
- }
- return new URL(referrer).href;
-}
-
 
 // Run the file contents in the correct scope or sandbox. Expose
 // the correct helper variables (require, module, exports) to
@@ -691,19 +722,48 @@ Module.prototype._compile = function(content, filename){
 
  content = stripShebang(content);
 
- // create wrapper function
- var wrapper = Module.wrap(content);
-
- var compiledWrapper = vm.runInThisContext(wrapper,{
- filename: filename,
- lineOffset: 0,
- displayErrors: true,
- importModuleDynamically: experimentalModules ? async (specifier) =>{
- if (asyncESM === undefined) lazyLoadESM();
- const loader = await asyncESM.loaderPromise;
- return loader.import(specifier, normalizeReferrerURL(filename));
- } : undefined,
- });
+ let compiledWrapper;
+ if (patched){
+ const wrapper = Module.wrap(content);
+ compiledWrapper = vm.runInThisContext(wrapper,{
+ filename,
+ lineOffset: 0,
+ displayErrors: true,
+ importModuleDynamically: experimentalModules ? async (specifier) =>{
+ if (asyncESM === undefined) lazyLoadESM();
+ const loader = await asyncESM.loaderPromise;
+ return loader.import(specifier, normalizeReferrerURL(filename));
+ } : undefined,
+ });
+ } else{
+ compiledWrapper = compileFunction(
+ content,
+ filename,
+ 0,
+ 0,
+ undefined,
+ false,
+ undefined,
+ [],
+ [
+ 'exports',
+ 'require',
+ 'module',
+ '__filename',
+ '__dirname',
+ ]
+ );
+ if (experimentalModules){
+ const{callbackMap } = internalBinding('module_wrap');
+ callbackMap.set(compiledWrapper,{
+ importModuleDynamically: async (specifier) =>{
+ if (asyncESM === undefined) lazyLoadESM();
+ const loader = await asyncESM.loaderPromise;
+ return loader.import(specifier, normalizeReferrerURL(filename));
+ }
+ });
+ }
+ }
 
  var inspectorWrapper = null;
  if (process._breakFirstLine && process._eval == null){

src/env-inl.h

@@ -459,6 +459,9 @@ inline uint32_t Environment::get_next_module_id(){
 inline uint32_t Environment::get_next_script_id(){
  return script_id_counter_++;
 }
+inline uint32_t Environment::get_next_function_id(){
+ return function_id_counter_++;
+}
 
 Environment::ShouldNotAbortOnUncaughtScope::ShouldNotAbortOnUncaughtScope(
  Environment* env)

src/env.cc

@@ -242,6 +242,11 @@ Environment::Environment(IsolateData* isolate_data,
  isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
 }
 
+CompileFnEntry::CompileFnEntry(Environment* env, uint32_t id)
+ : env(env), id(id){
+ env->compile_fn_entries.insert(this);
+}
+
 Environment::~Environment(){
  isolate()->GetHeapProfiler()->RemoveBuildEmbedderGraphCallback(
  BuildEmbedderGraph, this);
@@ -250,6 +255,12 @@ Environment::~Environment(){
  // CleanupHandles() should have removed all of them.
  CHECK(file_handle_read_wrap_freelist_.empty());
 
+ // dispose the Persistent references to the compileFunction
+ // wrappers used in the dynamic import callback
+ for (auto& entry : compile_fn_entries){
+ delete entry;
+ }
+
  HandleScope handle_scope(isolate());
 
 #if HAVE_INSPECTOR

src/env.h

@@ -442,6 +442,12 @@ struct ContextInfo{
  bool is_default = false;
 };
 
+struct CompileFnEntry{
+ Environment* env;
+ uint32_t id;
+ CompileFnEntry(Environment* env, uint32_t id);
+};
+
 // Listing the AsyncWrap provider types first enables us to cast directly
 // from a provider type to a debug category.
 #define DEBUG_CATEGORY_NAMES(V) \
@@ -703,9 +709,12 @@ class Environment{
  std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
  std::unordered_map<uint32_t, contextify::ContextifyScript*>
  id_to_script_map;
+ std::unordered_set<CompileFnEntry*> compile_fn_entries;
+ std::unordered_map<uint32_t, Persistent<v8::Function>> id_to_function_map;
 
  inline uint32_t get_next_module_id();
  inline uint32_t get_next_script_id();
+ inline uint32_t get_next_function_id();
 
  std::unordered_map<std::string, const loader::PackageConfig>
  package_json_cache;
@@ -972,6 +981,7 @@ class Environment{
 
  uint32_t module_id_counter_ = 0;
  uint32_t script_id_counter_ = 0;
+ uint32_t function_id_counter_ = 0;
 
  AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
  int should_not_abort_scope_counter_ = 0;

src/module_wrap.cc

@@ -752,6 +752,8 @@ static MaybeLocal<Promise> ImportModuleDynamically(
  } else if (type == ScriptType::kModule){
  ModuleWrap* wrap = ModuleWrap::GetFromID(env, id);
  object = wrap->object();
+ } else if (type == ScriptType::kFunction){
+ object = env->id_to_function_map.find(id)->second.Get(iso);
  } else{
  UNREACHABLE();
  }

src/module_wrap.h

@@ -20,6 +20,7 @@ enum PackageMainCheck : bool{
 enum ScriptType : int{
  kScript,
  kModule,
+ kFunction,
 };
 
 enum HostDefinedOptions : int{

src/node_contextify.cc

@@ -286,6 +286,15 @@ void ContextifyContext::WeakCallback(
  delete context;
 }
 
+void ContextifyContext::WeakCallbackCompileFn(
+ const WeakCallbackInfo<CompileFnEntry>& data){
+ CompileFnEntry* entry = data.GetParameter();
+ if (entry->env->compile_fn_entries.erase(entry) != 0){
+ entry->env->id_to_function_map.erase(entry->id);
+ delete entry;
+ }
+}
+
 // static
 ContextifyContext* ContextifyContext::ContextFromContextifiedSandbox(
  Environment* env,
@@ -1007,7 +1016,30 @@ void ContextifyContext::CompileFunction(
  data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
  }
 
- ScriptOrigin origin(filename, line_offset, column_offset, True(isolate));
+ // Get the function id
+ uint32_t id = env->get_next_function_id();
+
+ // Set host_defined_options
+ Local<PrimitiveArray> host_defined_options =
+ PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
+ host_defined_options->Set(
+ isolate,
+ loader::HostDefinedOptions::kType,
+ Number::New(isolate, loader::ScriptType::kFunction));
+ host_defined_options->Set(
+ isolate, loader::HostDefinedOptions::kID, Number::New(isolate, id));
+
+ ScriptOrigin origin(filename,
+ line_offset, // line offset
+ column_offset, // column offset
+ True(isolate), // is cross origin
+ Local<Integer>(), // script id
+ Local<Value>(), // source map URL
+ False(isolate), // is opaque (?)
+ False(isolate), // is WASM
+ False(isolate), // is ES Module
+ host_defined_options);
+
  ScriptCompiler::Source source(code, origin, cached_data);
  ScriptCompiler::CompileOptions options;
  if (source.GetCachedData() == nullptr){
@@ -1041,38 +1073,45 @@ void ContextifyContext::CompileFunction(
  }
  }
 
- MaybeLocal<Function> maybe_fun = ScriptCompiler::CompileFunctionInContext(
+ MaybeLocal<Function> maybe_fn = ScriptCompiler::CompileFunctionInContext(
  parsing_context, &source, params.size(), params.data(),
  context_extensions.size(), context_extensions.data(), options);
 
- Local<Function> fun;
- if (maybe_fun.IsEmpty() || !maybe_fun.ToLocal(&fun)){
+ if (maybe_fn.IsEmpty()){
  DecorateErrorStack(env, try_catch);
  try_catch.ReThrow();
  return;
  }
+ Local<Function> fn = maybe_fn.ToLocalChecked();
+ env->id_to_function_map.emplace(std::piecewise_construct,
+ std::make_tuple(id),
+ std::make_tuple(isolate, fn));
+ CompileFnEntry* gc_entry = new CompileFnEntry(env, id);
+ env->id_to_function_map[id].SetWeak(gc_entry,
+ WeakCallbackCompileFn,
+ v8::WeakCallbackType::kParameter);
 
  if (produce_cached_data){
  const std::unique_ptr<ScriptCompiler::CachedData> cached_data(
- ScriptCompiler::CreateCodeCacheForFunction(fun));
+ ScriptCompiler::CreateCodeCacheForFunction(fn));
  bool cached_data_produced = cached_data != nullptr;
  if (cached_data_produced){
  MaybeLocal<Object> buf = Buffer::Copy(
  env,
  reinterpret_cast<const char*>(cached_data->data),
  cached_data->length);
- if (fun->Set(
+ if (fn->Set(
  parsing_context,
  env->cached_data_string(),
  buf.ToLocalChecked()).IsNothing()) return;
  }
- if (fun->Set(
+ if (fn->Set(
  parsing_context,
  env->cached_data_produced_string(),
  Boolean::New(isolate, cached_data_produced)).IsNothing()) return;
  }
 
- args.GetReturnValue().Set(fun);
+ args.GetReturnValue().Set(fn);
 }