Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 34d70ce

Browse filesBrowse files
addaleaxruyadorno
addaleax
authored and
ruyadorno
committed
vm: expose cachedDataRejected for vm.compileFunction
Having this information available is useful for functions just as it is for scripts. Therefore, expose it in the same way that other information related to code caching is reported. As part of this, de-duplify the code for setting the properties on the C++ side and add proper exception handling to it. PR-URL: #46320 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 88b904c commit 34d70ce
Copy full SHA for 34d70ce

File tree

Expand file treeCollapse file tree

5 files changed

+114
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+114
-55
lines changed
Open diff view settings
Collapse file

‎doc/api/vm.md‎

Copy file name to clipboardExpand all lines: doc/api/vm.md
+6Lines changed: 6 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,12 @@ const vm = require('node:vm');
962962
<!-- YAML
963963
added: v10.10.0
964964
changes:
965+
- version:
966+
- REPLACEME
967+
pr-url: https://github.com/nodejs/node/pull/46320
968+
description: The return value now includes `cachedDataRejected`
969+
with the same semantics as the `vm.Script` version
970+
if the `cachedData` option was passed.
965971
- version:
966972
- v17.0.0
967973
- v16.12.0
Collapse file

‎lib/internal/vm.js‎

Copy file name to clipboardExpand all lines: lib/internal/vm.js
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ function internalCompileFunction(code, params, options) {
9090
result.function.cachedData = result.cachedData;
9191
}
9292

93+
if (typeof result.cachedDataRejected === 'boolean') {
94+
result.function.cachedDataRejected = result.cachedDataRejected;
95+
}
96+
9397
if (importModuleDynamically !== undefined) {
9498
validateFunction(importModuleDynamically,
9599
'options.importModuleDynamically');
Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+76-49Lines changed: 76 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ using v8::HandleScope;
4949
using v8::IndexedPropertyHandlerConfiguration;
5050
using v8::Int32;
5151
using v8::Isolate;
52+
using v8::Just;
5253
using v8::Local;
5354
using v8::Maybe;
5455
using v8::MaybeLocal;
@@ -58,6 +59,7 @@ using v8::MicrotaskQueue;
5859
using v8::MicrotasksPolicy;
5960
using v8::Name;
6061
using v8::NamedPropertyHandlerConfiguration;
62+
using v8::Nothing;
6163
using v8::Number;
6264
using v8::Object;
6365
using v8::ObjectTemplate;
@@ -857,40 +859,75 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
857859
}
858860
contextify_script->script_.Reset(isolate, v8_script);
859861

860-
Local<Context> env_context = env->context();
861-
if (compile_options == ScriptCompiler::kConsumeCodeCache) {
862-
args.This()->Set(
863-
env_context,
864-
env->cached_data_rejected_string(),
865-
Boolean::New(isolate, source.GetCachedData()->rejected)).Check();
866-
} else if (produce_cached_data) {
867-
std::unique_ptr<ScriptCompiler::CachedData> cached_data{
868-
ScriptCompiler::CreateCodeCache(v8_script)};
869-
bool cached_data_produced = cached_data != nullptr;
870-
if (cached_data_produced) {
871-
MaybeLocal<Object> buf = Buffer::Copy(
872-
env,
873-
reinterpret_cast<const char*>(cached_data->data),
874-
cached_data->length);
875-
args.This()->Set(env_context,
876-
env->cached_data_string(),
877-
buf.ToLocalChecked()).Check();
878-
}
879-
args.This()->Set(
880-
env_context,
881-
env->cached_data_produced_string(),
882-
Boolean::New(isolate, cached_data_produced)).Check();
862+
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
863+
if (produce_cached_data) {
864+
new_cached_data.reset(ScriptCompiler::CreateCodeCache(v8_script));
883865
}
884866

885-
args.This()
886-
->Set(env_context,
887-
env->source_map_url_string(),
888-
v8_script->GetSourceMappingURL())
889-
.Check();
867+
if (StoreCodeCacheResult(env,
868+
args.This(),
869+
compile_options,
870+
source,
871+
produce_cached_data,
872+
std::move(new_cached_data))
873+
.IsNothing()) {
874+
return;
875+
}
876+
877+
if (args.This()
878+
->Set(env->context(),
879+
env->source_map_url_string(),
880+
v8_script->GetSourceMappingURL())
881+
.IsNothing())
882+
return;
890883

891884
TRACE_EVENT_END0(TRACING_CATEGORY_NODE2(vm, script), "ContextifyScript::New");
892885
}
893886

887+
Maybe<bool> StoreCodeCacheResult(
888+
Environment* env,
889+
Local<Object> target,
890+
ScriptCompiler::CompileOptions compile_options,
891+
const v8::ScriptCompiler::Source& source,
892+
bool produce_cached_data,
893+
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data) {
894+
Local<Context> context;
895+
if (!target->GetCreationContext().ToLocal(&context)) {
896+
return Nothing<bool>();
897+
}
898+
if (compile_options == ScriptCompiler::kConsumeCodeCache) {
899+
if (target
900+
->Set(
901+
context,
902+
env->cached_data_rejected_string(),
903+
Boolean::New(env->isolate(), source.GetCachedData()->rejected))
904+
.IsNothing()) {
905+
return Nothing<bool>();
906+
}
907+
}
908+
if (produce_cached_data) {
909+
bool cached_data_produced = new_cached_data != nullptr;
910+
if (cached_data_produced) {
911+
MaybeLocal<Object> buf =
912+
Buffer::Copy(env,
913+
reinterpret_cast<const char*>(new_cached_data->data),
914+
new_cached_data->length);
915+
if (target->Set(context, env->cached_data_string(), buf.ToLocalChecked())
916+
.IsNothing()) {
917+
return Nothing<bool>();
918+
}
919+
}
920+
if (target
921+
->Set(context,
922+
env->cached_data_produced_string(),
923+
Boolean::New(env->isolate(), cached_data_produced))
924+
.IsNothing()) {
925+
return Nothing<bool>();
926+
}
927+
}
928+
return Just(true);
929+
}
930+
894931
bool ContextifyScript::InstanceOf(Environment* env,
895932
const Local<Value>& value) {
896933
return !value.IsEmpty() &&
@@ -1242,28 +1279,18 @@ void ContextifyContext::CompileFunction(
12421279
.IsNothing())
12431280
return;
12441281

1282+
std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
12451283
if (produce_cached_data) {
1246-
const std::unique_ptr<ScriptCompiler::CachedData> cached_data(
1247-
ScriptCompiler::CreateCodeCacheForFunction(fn));
1248-
bool cached_data_produced = cached_data != nullptr;
1249-
if (cached_data_produced) {
1250-
MaybeLocal<Object> buf = Buffer::Copy(
1251-
env,
1252-
reinterpret_cast<const char*>(cached_data->data),
1253-
cached_data->length);
1254-
if (result
1255-
->Set(parsing_context,
1256-
env->cached_data_string(),
1257-
buf.ToLocalChecked())
1258-
.IsNothing())
1259-
return;
1260-
}
1261-
if (result
1262-
->Set(parsing_context,
1263-
env->cached_data_produced_string(),
1264-
Boolean::New(isolate, cached_data_produced))
1265-
.IsNothing())
1266-
return;
1284+
new_cached_data.reset(ScriptCompiler::CreateCodeCacheForFunction(fn));
1285+
}
1286+
if (StoreCodeCacheResult(env,
1287+
result,
1288+
options,
1289+
source,
1290+
produce_cached_data,
1291+
std::move(new_cached_data))
1292+
.IsNothing()) {
1293+
return;
12671294
}
12681295

12691296
args.GetReturnValue().Set(result);
Collapse file

‎src/node_contextify.h‎

Copy file name to clipboardExpand all lines: src/node_contextify.h
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,14 @@ class CompiledFnEntry final : public BaseObject {
199199
static void WeakCallback(const v8::WeakCallbackInfo<CompiledFnEntry>& data);
200200
};
201201

202+
v8::Maybe<bool> StoreCodeCacheResult(
203+
Environment* env,
204+
v8::Local<v8::Object> target,
205+
v8::ScriptCompiler::CompileOptions compile_options,
206+
const v8::ScriptCompiler::Source& source,
207+
bool produce_cached_data,
208+
std::unique_ptr<v8::ScriptCompiler::CachedData> new_cached_data);
209+
202210
} // namespace contextify
203211
} // namespace node
204212

Collapse file

‎test/parallel/test-vm-basic.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-vm-basic.js
+20-6Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,13 +323,27 @@ const vm = require('vm');
323323
global
324324
);
325325

326-
// Test compileFunction produceCachedData option
327-
const result = vm.compileFunction('console.log("Hello, World!")', [], {
328-
produceCachedData: true,
329-
});
326+
{
327+
const source = 'console.log("Hello, World!")';
328+
// Test compileFunction produceCachedData option
329+
const result = vm.compileFunction(source, [], {
330+
produceCachedData: true,
331+
});
330332

331-
assert.ok(result.cachedDataProduced);
332-
assert.ok(result.cachedData.length > 0);
333+
assert.ok(result.cachedDataProduced);
334+
assert.ok(result.cachedData.length > 0);
335+
336+
// Test compileFunction cachedData consumption
337+
const result2 = vm.compileFunction(source, [], {
338+
cachedData: result.cachedData
339+
});
340+
assert.strictEqual(result2.cachedDataRejected, false);
341+
342+
const result3 = vm.compileFunction('console.log("wrong source")', [], {
343+
cachedData: result.cachedData
344+
});
345+
assert.strictEqual(result3.cachedDataRejected, true);
346+
}
333347

334348
// Resetting value
335349
Error.stackTraceLimit = oldLimit;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.