From 696063a47c3ad33c91d57abfe13cd93b79e211a4 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 20 Feb 2024 22:16:24 +0100 Subject: [PATCH] src: stop the profiler and the inspector before snapshot serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/51815 Reviewed-By: Juan José Arboleda Reviewed-By: Yagiz Nizipli Reviewed-By: Chengzhong Wu --- src/env.h | 3 ++ src/node_snapshotable.cc | 8 ++++ test/parallel/test-snapshot-coverage.js | 61 +++++++++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 test/parallel/test-snapshot-coverage.js diff --git a/src/env.h b/src/env.h index 6e9f5a466ac63e..994b9573822fd3 100644 --- a/src/env.h +++ b/src/env.h @@ -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); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 4d00e75804b4c2..0309aba6bdc24a 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -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); diff --git a/test/parallel/test-snapshot-coverage.js b/test/parallel/test-snapshot-coverage.js new file mode 100644 index 00000000000000..77fc7ae51ba575 --- /dev/null +++ b/test/parallel/test-snapshot-coverage.js @@ -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); +}