Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,34 @@ IsolateGroup GetOrCreateIsolateGroup() {
return IsolateGroup::GetDefault();
}

// V8 shares the read-only heap between isolates and requires them all to be
// created from the same snapshot, so every isolate gets the blob and external
// references the first NewIsolate() call used. ~SnapshotData() leaves that blob
// alone because its owner may be gone before the last isolate is created.
static Mutex first_snapshot_mutex;
static bool first_snapshot_recorded = false;
static v8::StartupData first_snapshot_blob{nullptr, 0};
static const intptr_t* first_external_references = nullptr;

static void UseFirstSnapshot(Isolate::CreateParams* params) {
Mutex::ScopedLock lock(first_snapshot_mutex);
if (!first_snapshot_recorded) {
first_snapshot_recorded = true;
if (params->snapshot_blob != nullptr) {
first_snapshot_blob = *params->snapshot_blob;
}
first_external_references = params->external_references;
}
params->snapshot_blob =
first_snapshot_blob.data != nullptr ? &first_snapshot_blob : nullptr;
params->external_references = first_external_references;
}

bool IsFirstSnapshotBlob(const char* data) {
Mutex::ScopedLock lock(first_snapshot_mutex);
return first_snapshot_recorded && data == first_snapshot_blob.data;
}

// TODO(joyeecheung): we may want to expose this, but then we need to be
// careful about what we override in the params.
Isolate* NewIsolate(Isolate::CreateParams* params,
Expand All @@ -327,15 +355,7 @@ Isolate* NewIsolate(Isolate::CreateParams* params,
SnapshotBuilder::InitializeIsolateParams(snapshot_data, params);
}

{
// Because it uses a shared readonly-heap, V8 requires all snapshots used
// for creating Isolates to be identical. This isn't really memory-safe
// but also otherwise just doesn't work, and the only real alternative
// is disabling shared-readonly-heap mode altogether.
static Isolate::CreateParams first_params = *params;
params->snapshot_blob = first_params.snapshot_blob;
params->external_references = first_params.external_references;
}
UseFirstSnapshot(params);

// Register the isolate on the platform before the isolate gets initialized,
// so that the isolate can access the platform during initialization.
Expand Down
3 changes: 3 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,9 @@ class NODE_EXTERN CommonEnvironmentSetup {
// will be empty.
// env_args will be passed through as arguments to CreateEnvironment(), after
// `isolate_data` and `context`.
// `snapshot_data` has to stay alive as long as the setup created from it,
// and every setup in a process has to use the same snapshot: all isolates
// are created from the blob the first one used.
template <typename... EnvironmentArgs>
static std::unique_ptr<CommonEnvironmentSetup> Create(
MultiIsolatePlatform* platform,
Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
// addresses, so this should be used with care.
v8::IsolateGroup GetOrCreateIsolateGroup();

// The blob every isolate is created from, see NewIsolate(). It is never freed.
bool IsFirstSnapshotBlob(const char* data);
v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
uv_loop_t* event_loop,
MultiIsolatePlatform* platform,
Expand Down
3 changes: 2 additions & 1 deletion src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,8 @@ bool SnapshotData::Check() const {

SnapshotData::~SnapshotData() {
if (data_ownership == DataOwnership::kOwned &&
v8_snapshot_blob_data.data != nullptr) {
v8_snapshot_blob_data.data != nullptr &&
!IsFirstSnapshotBlob(v8_snapshot_blob_data.data)) {
delete[] v8_snapshot_blob_data.data;
}
}
Expand Down
16 changes: 14 additions & 2 deletions test/embedding/embedtest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,20 @@ NODE_MAIN(int argc, node::argv_type raw_argv[]) {
cppgc::InitializeProcess(platform->GetPageAllocator());
V8::Initialize();

int ret =
RunNodeInstance(platform.get(), result->args(), result->exec_args());
// --embedder-run-twice: two sequential instances in one process, each
// loading (and freeing) its own copy of the snapshot.
std::vector<std::string> instance_args = result->args();
auto twice = std::find(
instance_args.begin(), instance_args.end(), "--embedder-run-twice");
int runs = 1;
if (twice != instance_args.end()) {
instance_args.erase(twice);
runs = 2;
}
int ret = 0;
for (int i = 0; i < runs && ret == 0; i++) {
ret = RunNodeInstance(platform.get(), instance_args, result->exec_args());
}

V8::Dispose();
V8::DisposePlatform();
Expand Down
39 changes: 39 additions & 0 deletions test/embedding/test-embedding-snapshot-twice.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

// Tests that an embedder can free the EmbedderSnapshotData it created an
// instance from and create a second instance from a fresh copy afterwards.

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

const embedtest = common.resolveBuiltBinary('embedtest');
const snapshotFixture = fixtures.path('snapshot', 'echo-args.js');
const blob = tmpdir.resolve('embedder-snapshot.blob');

tmpdir.refresh();

spawnSyncAndExitWithoutError(
embedtest,
[
'--',
`eval(require("fs").readFileSync(${JSON.stringify(snapshotFixture)}, "utf8"))`,
'arg1', 'arg2', '--embedder-snapshot-blob', blob, '--embedder-snapshot-create',
],
{ cwd: tmpdir.path });

spawnSyncAndAssert(
embedtest,
['--', 'arg3', '--embedder-snapshot-blob', blob, '--embedder-run-twice'],
{ cwd: tmpdir.path },
{
stdout(output) {
assert.strictEqual(output.split('arg3').length, 3);
return true;
},
});
Loading