Skip to content

Commit

Permalink
vm: implement isContext() directly in JS land with private symbol
Browse files Browse the repository at this point in the history
We are now directly checking the existence of a private symbol
in the object to determine if an object is a ContextifyContext
anyway, so there is no need to implement it in C++ anymore.

PR-URL: #51685
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
  • Loading branch information
joyeecheung authored and targos committed Feb 19, 2024
1 parent d7563a5 commit ba444a9
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 24 deletions.
12 changes: 6 additions & 6 deletions lib/internal/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const {
const {
ContextifyScript,
compileFunction,
isContext: _isContext,
} = internalBinding('contextify');
const {
runInContext,
Expand All @@ -21,18 +20,19 @@ const {
} = internalBinding('symbols');
const {
validateFunction,
validateObject,
kValidateObjectAllowArray,
} = require('internal/validators');

const {
getOptionValue,
} = require('internal/options');
const {
privateSymbols: {
contextify_context_private_symbol,
},
} = internalBinding('util');

function isContext(object) {
validateObject(object, 'object', kValidateObjectAllowArray);

return _isContext(object);
return object[contextify_context_private_symbol] !== undefined;
}

function getHostDefinedOptionId(importModuleDynamically, hint) {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/vm/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const {
TypeError,
} = primordials;

const { isContext } = internalBinding('contextify');
const {
isModuleNamespaceObject,
} = require('internal/util/types');
Expand Down Expand Up @@ -75,6 +74,8 @@ const kContext = Symbol('kContext');
const kPerContextModuleId = Symbol('kPerContextModuleId');
const kLink = Symbol('kLink');

const { isContext } = require('internal/vm');

class Module {
constructor(options) {
emitExperimentalWarning('VM Modules');
Expand Down
15 changes: 14 additions & 1 deletion lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const {
validateString,
validateStringArray,
validateUint32,
kValidateObjectAllowArray,
kValidateObjectAllowNullable,
} = require('internal/validators');
const {
Expand All @@ -59,14 +60,26 @@ const {
const {
getHostDefinedOptionId,
internalCompileFunction,
isContext,
isContext: _isContext,
registerImportModuleDynamically,
} = require('internal/vm');
const {
vm_dynamic_import_main_context_default,
} = internalBinding('symbols');
const kParsingContext = Symbol('script parsing context');

/**
* Check if object is a context object created by vm.createContext().
* @throws {TypeError} If object is not an object in the first place, throws TypeError.
* @param {object} object Object to check.
* @returns {boolean}
*/
function isContext(object) {
validateObject(object, 'object', kValidateObjectAllowArray);

return _isContext(object);
}

class Script extends ContextifyScript {
constructor(code, options = kEmptyObject) {
code = `${code}`;
Expand Down
16 changes: 0 additions & 16 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,13 @@ void ContextifyContext::CreatePerIsolateProperties(
IsolateData* isolate_data, Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
SetMethod(isolate, target, "makeContext", MakeContext);
SetMethod(isolate, target, "isContext", IsContext);
SetMethod(isolate, target, "compileFunction", CompileFunction);
SetMethod(isolate, target, "containsModuleSyntax", ContainsModuleSyntax);
}

void ContextifyContext::RegisterExternalReferences(
ExternalReferenceRegistry* registry) {
registry->Register(MakeContext);
registry->Register(IsContext);
registry->Register(CompileFunction);
registry->Register(ContainsModuleSyntax);
registry->Register(PropertyGetterCallback);
Expand Down Expand Up @@ -415,20 +413,6 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
}
}


void ContextifyContext::IsContext(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsObject());
Local<Object> sandbox = args[0].As<Object>();

Maybe<bool> result =
sandbox->HasPrivate(env->context(),
env->contextify_context_private_symbol());
args.GetReturnValue().Set(result.FromJust());
}


void ContextifyContext::WeakCallback(
const WeakCallbackInfo<ContextifyContext>& data) {
ContextifyContext* context = data.GetParameter();
Expand Down

0 comments on commit ba444a9

Please sign in to comment.