Skip to content

bootstrap: handle snapshot errors gracefully #43531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Changes from 3 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
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.8',
'v8_embedder_string': '-node.9',

##### V8 defaults for Node.js #####

9 changes: 6 additions & 3 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
@@ -483,7 +483,6 @@ SnapshotCreator::SnapshotCreator(const intptr_t* external_references,

SnapshotCreator::~SnapshotCreator() {
SnapshotCreatorData* data = SnapshotCreatorData::cast(data_);
DCHECK(data->created_);
Isolate* isolate = data->isolate_;
isolate->Exit();
isolate->Dispose();
@@ -590,8 +589,12 @@ StartupData SnapshotCreator::CreateBlob(
SnapshotCreator::FunctionCodeHandling function_code_handling) {
SnapshotCreatorData* data = SnapshotCreatorData::cast(data_);
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(data->isolate_);
DCHECK(!data->created_);
DCHECK(!data->default_context_.IsEmpty());
Utils::ApiCheck(!data->created_, "v8::SnapshotCreator::CreateBlob",
"CreateBlob() cannot be called more than once on the same "
"SnapshotCreator.");
Utils::ApiCheck(
!data->default_context_.IsEmpty(), "v8::SnapshotCreator::CreateBlob",
"CreateBlob() cannot be called before the default context is set.");

const int num_additional_contexts = static_cast<int>(data->contexts_.Size());
const int num_contexts = num_additional_contexts + 1; // The default context.
25 changes: 25 additions & 0 deletions deps/v8/test/cctest/test-serialize.cc
Original file line number Diff line number Diff line change
@@ -2840,6 +2840,31 @@ TEST(Regress503552) {
delete cache_data;
}

UNINITIALIZED_TEST(SnapshotCreatorBlobNotCreated) {
DisableAlwaysOpt();
DisableEmbeddedBlobRefcounting();
{
v8::SnapshotCreator creator;
v8::Isolate* isolate = creator.GetIsolate();
{
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = v8::Context::New(isolate);
v8::Context::Scope context_scope(context);
v8::TryCatch try_catch(isolate);
v8::Local<v8::String> code = v8_str("throw new Error('test');");
CHECK(v8::Script::Compile(context, code)
.ToLocalChecked()
->Run(context)
.IsEmpty());
CHECK(try_catch.HasCaught());
}
// SnapshotCreator should be destroyed just fine even when no
// blob is created.
}

FreeCurrentEmbeddedBlob();
}

UNINITIALIZED_TEST(SnapshotCreatorMultipleContexts) {
DisableAlwaysOpt();
DisableEmbeddedBlobRefcounting();
3 changes: 3 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
@@ -3822,6 +3822,9 @@ cases:
options were set, but the port number chosen was invalid or unavailable.
* `13` **Unfinished Top-Level Await**: `await` was used outside of a function
in the top-level code, but the passed `Promise` never resolved.
* `14` **Snapshot Failure**: Node.js was started to build a V8 startup
snapshot and it failed because certain requirements of the state of
the application were not met.
* `>128` **Signal Exits**: If Node.js receives a fatal signal such as
`SIGKILL` or `SIGHUP`, then its exit code will be `128` plus the
value of the signal code. This is a standard POSIX practice, since
21 changes: 18 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
@@ -991,13 +991,17 @@ struct EnvSerializeInfo {
};

struct SnapshotData {
// The result of v8::SnapshotCreator::CreateBlob() during the snapshot
// building process.
v8::StartupData v8_snapshot_blob_data;
enum class DataOwnership { kOwned, kNotOwned };

static const size_t kNodeBaseContextIndex = 0;
static const size_t kNodeMainContextIndex = kNodeBaseContextIndex + 1;

DataOwnership data_ownership;

// The result of v8::SnapshotCreator::CreateBlob() during the snapshot
// building process.
v8::StartupData v8_snapshot_blob_data;

std::vector<size_t> isolate_data_indices;
// TODO(joyeecheung): there should be a vector of env_info once we snapshot
// the worker environments.
@@ -1007,6 +1011,17 @@ struct SnapshotData {
// read only space. We use native_module::CodeCacheInfo because
// v8::ScriptCompiler::CachedData is not copyable.
std::vector<native_module::CodeCacheInfo> code_cache;

static std::unique_ptr<SnapshotData> New();
~SnapshotData();

SnapshotData(const SnapshotData&) = delete;
SnapshotData& operator=(const SnapshotData&) = delete;
SnapshotData(SnapshotData&&) = delete;
SnapshotData& operator=(SnapshotData&&) = delete;

// Only invoked by New().
SnapshotData() = default;
};

class Environment : public MemoryRetainer {
11 changes: 6 additions & 5 deletions src/node_snapshot_builder.h
Original file line number Diff line number Diff line change
@@ -15,13 +15,14 @@ struct SnapshotData;

class NODE_EXTERN_PRIVATE SnapshotBuilder {
public:
static std::string Generate(const std::vector<std::string> args,
const std::vector<std::string> exec_args);
static int Generate(std::ostream& out,
const std::vector<std::string> args,
const std::vector<std::string> exec_args);

// Generate the snapshot into out.
static void Generate(SnapshotData* out,
const std::vector<std::string> args,
const std::vector<std::string> exec_args);
static int Generate(SnapshotData* out,
const std::vector<std::string> args,
const std::vector<std::string> exec_args);

// If nullptr is returned, the binary is not built with embedded
// snapshot.
335 changes: 178 additions & 157 deletions src/node_snapshotable.cc

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions test/fixtures/snapshot/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('test');
17 changes: 9 additions & 8 deletions tools/snapshot/node_mksnapshot.cc
Original file line number Diff line number Diff line change
@@ -83,17 +83,18 @@ int BuildSnapshot(int argc, char* argv[]) {
return 1;
}

int exit_code = 0;
{
std::string snapshot =
node::SnapshotBuilder::Generate(result.args, result.exec_args);
out << snapshot;

if (!out) {
std::cerr << "Failed to write " << out_path << "\n";
return 1;
exit_code =
node::SnapshotBuilder::Generate(out, result.args, result.exec_args);
if (exit_code == 0) {
if (!out) {
std::cerr << "Failed to write " << out_path << "\n";
exit_code = 1;
}
}
}

node::TearDownOncePerProcess();
return 0;
return exit_code;
}