Skip to content

Commit

Permalink
vm: enhance cached data handling for empty functions and add validati…
Browse files Browse the repository at this point in the history
…on tests
  • Loading branch information
gurgunday committed Jan 2, 2025
1 parent 35742a2 commit d5f9742
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1564,9 +1564,15 @@ Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
return Object::New(env->isolate());

std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
if (produce_cached_data) {
new_cached_data.reset(ScriptCompiler::CreateCodeCacheForFunction(fn));
if (produce_cached_data && !fn.IsEmpty()) {
TryCatchScope try_cache(env);
Local<Value> resource_name = fn->GetScriptOrigin().ResourceName();
if (!resource_name.IsEmpty() &&
resource_name->ToString(parsing_context).ToLocalChecked()->Length() > 0) {
new_cached_data.reset(ScriptCompiler::CreateCodeCacheForFunction(fn));
}
}

if (StoreCodeCacheResult(env,
result,
options,
Expand Down
34 changes: 34 additions & 0 deletions test/parallel/test-vm-cached-data-validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';

require('../common');
const assert = require('assert');
const vm = require('vm');

// Test that creating cached data for an empty function doesn't crash
{
const script = new vm.Script('');
// Get cached data from empty script
const cachedData = script.createCachedData();

Check failure on line 12 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Trailing spaces not allowed
// Attempt to compile a function using this cached data
// This was previously causing a fatal V8 error
assert.doesNotThrow(() => {

Check failure on line 15 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead
vm.compileFunction('', undefined, {

Check failure on line 16 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Trailing spaces not allowed
cachedData,
produceCachedData: true

Check failure on line 18 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Trailing spaces not allowed
});
}, 'Should not crash when compiling empty function with cached data');
}

// Test normal case with actual function content still works
{
const script = new vm.Script('function test() { return 42; }');
const cachedData = script.createCachedData();

Check failure on line 27 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Trailing spaces not allowed
const fn = vm.compileFunction('return 42', undefined, {
cachedData,
produceCachedData: true
});

Check failure on line 32 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Trailing spaces not allowed
assert.strictEqual(fn(), 42, 'Function should compile and run correctly');

Check failure on line 33 in test/parallel/test-vm-cached-data-validation.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Do not use a literal for the third argument of assert.strictEqual()
}

0 comments on commit d5f9742

Please sign in to comment.