Skip to content

Commit 8f10543

Browse files
joyeecheungmarco-ippolito
authored andcommitted
src: stop the profiler and the inspector before snapshot serialization
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>
1 parent a5a17a1 commit 8f10543

File tree

3 files changed

+72
-0
lines changed

3 files changed

+72
-0
lines changed

src/env.h

+3
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,9 @@ class Environment : public MemoryRetainer {
881881
inline inspector::Agent* inspector_agent() const {
882882
return inspector_agent_.get();
883883
}
884+
inline void StopInspector() {
885+
inspector_agent_.reset();
886+
}
884887

885888
inline bool is_in_inspector_console_call() const;
886889
inline void set_is_in_inspector_console_call(bool value);

src/node_snapshotable.cc

+8
Original file line numberDiff line numberDiff line change
@@ -1132,6 +1132,14 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
11321132
fprintf(stderr, "Environment = %p\n", env);
11331133
}
11341134

1135+
// Clean up the states left by the inspector because V8 cannot serialize
1136+
// them. They don't need to be persisted and can be created from scratch
1137+
// after snapshot deserialization.
1138+
RunAtExit(env);
1139+
#if HAVE_INSPECTOR
1140+
env->StopInspector();
1141+
#endif
1142+
11351143
// Serialize the native states
11361144
out->isolate_data_info = setup->isolate_data()->Serialize(creator);
11371145
out->env_info = env->Serialize(creator);
+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
3+
// This tests that the snapshot works with built-in coverage collection.
4+
5+
const common = require('../common');
6+
common.skipIfInspectorDisabled();
7+
8+
const { spawnSyncAndExitWithoutError } = require('../common/child_process');
9+
const tmpdir = require('../common/tmpdir');
10+
const fixtures = require('../common/fixtures');
11+
const fs = require('fs');
12+
const assert = require('assert');
13+
14+
tmpdir.refresh();
15+
const blobPath = tmpdir.resolve('snapshot.blob');
16+
const file = fixtures.path('empty.js');
17+
18+
function filterCoverageFiles(name) {
19+
return name.startsWith('coverage') && name.endsWith('.json');
20+
}
21+
{
22+
// Create the snapshot.
23+
const { child } = spawnSyncAndExitWithoutError(process.execPath, [
24+
'--snapshot-blob',
25+
blobPath,
26+
'--build-snapshot',
27+
file,
28+
], {
29+
cwd: tmpdir.path,
30+
env: {
31+
...process.env,
32+
NODE_V8_COVERAGE: tmpdir.path,
33+
NODE_DEBUG_NATIVE: 'inspector_profiler',
34+
}
35+
});
36+
const files = fs.readdirSync(tmpdir.path);
37+
console.log('Files in tmpdir.path', files); // Log for debugging the test.
38+
const coverage = files.filter(filterCoverageFiles);
39+
console.log(child.stderr.toString());
40+
assert.strictEqual(coverage.length, 1);
41+
}
42+
43+
{
44+
const { child } = spawnSyncAndExitWithoutError(process.execPath, [
45+
'--snapshot-blob',
46+
blobPath,
47+
file,
48+
], {
49+
cwd: tmpdir.path,
50+
env: {
51+
...process.env,
52+
NODE_V8_COVERAGE: tmpdir.path,
53+
NODE_DEBUG_NATIVE: 'inspector_profiler',
54+
},
55+
});
56+
const files = fs.readdirSync(tmpdir.path);
57+
console.log('Files in tmpdir.path', files); // Log for debugging the test.
58+
const coverage = files.filter(filterCoverageFiles);
59+
console.log(child.stderr.toString());
60+
assert.strictEqual(coverage.length, 2);
61+
}

0 commit comments

Comments
 (0)