Skip to content

Commit

Permalink
src: stop the profiler and the inspector before snapshot serialization
Browse files Browse the repository at this point in the history
Otherwise NODE_V8_COVERAGE would crash in snapshot tests because V8
cannot serialize the leftover debug infos. This ensures that we clean
them all up before serialization.

PR-URL: #51815
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
  • Loading branch information
joyeecheung authored and richardlau committed Mar 25, 2024
1 parent 3dfee7e commit 696063a
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,9 @@ class Environment : public MemoryRetainer {
inline inspector::Agent* inspector_agent() const {
return inspector_agent_.get();
}
inline void StopInspector() {
inspector_agent_.reset();
}

inline bool is_in_inspector_console_call() const;
inline void set_is_in_inspector_console_call(bool value);
Expand Down
8 changes: 8 additions & 0 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,14 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
fprintf(stderr, "Environment = %p\n", env);
}

// Clean up the states left by the inspector because V8 cannot serialize
// them. They don't need to be persisted and can be created from scratch
// after snapshot deserialization.
RunAtExit(env);
#if HAVE_INSPECTOR
env->StopInspector();
#endif

// Serialize the native states
out->isolate_data_info = setup->isolate_data()->Serialize(creator);
out->env_info = env->Serialize(creator);
Expand Down
61 changes: 61 additions & 0 deletions test/parallel/test-snapshot-coverage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
'use strict';

// This tests that the snapshot works with built-in coverage collection.

const common = require('../common');
common.skipIfInspectorDisabled();

const { spawnSyncAndExitWithoutError } = require('../common/child_process');
const tmpdir = require('../common/tmpdir');
const fixtures = require('../common/fixtures');
const fs = require('fs');
const assert = require('assert');

tmpdir.refresh();
const blobPath = tmpdir.resolve('snapshot.blob');
const file = fixtures.path('empty.js');

function filterCoverageFiles(name) {
return name.startsWith('coverage') && name.endsWith('.json');
}
{
// Create the snapshot.
const { child } = spawnSyncAndExitWithoutError(process.execPath, [
'--snapshot-blob',
blobPath,
'--build-snapshot',
file,
], {
cwd: tmpdir.path,
env: {
...process.env,
NODE_V8_COVERAGE: tmpdir.path,
NODE_DEBUG_NATIVE: 'inspector_profiler',
}
});
const files = fs.readdirSync(tmpdir.path);
console.log('Files in tmpdir.path', files); // Log for debugging the test.
const coverage = files.filter(filterCoverageFiles);
console.log(child.stderr.toString());
assert.strictEqual(coverage.length, 1);
}

{
const { child } = spawnSyncAndExitWithoutError(process.execPath, [
'--snapshot-blob',
blobPath,
file,
], {
cwd: tmpdir.path,
env: {
...process.env,
NODE_V8_COVERAGE: tmpdir.path,
NODE_DEBUG_NATIVE: 'inspector_profiler',
},
});
const files = fs.readdirSync(tmpdir.path);
console.log('Files in tmpdir.path', files); // Log for debugging the test.
const coverage = files.filter(filterCoverageFiles);
console.log(child.stderr.toString());
assert.strictEqual(coverage.length, 2);
}

0 comments on commit 696063a

Please sign in to comment.