From 361a643d8b5354fe18ee9f62d2fa284c3c1682ed Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 13 Jul 2022 17:40:15 +0800 Subject: [PATCH] deps: V8: backport f3cad8cec656 Original commit message: [serializer] allow SnapshotCreator to destruct without a blob Previously SnapshotCreator demanded a blob to be created before it can be destructed in debug build, this patch removes the DCHECK so that the embedder can choose not to create the blob when e.g. the snapshot building isn't successful due to errors. Change-Id: I72939be1e0d79b257b9761f48a72e45325a1f6d8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3716682 Reviewed-by: Camillo Bruni Commit-Queue: Joyee Cheung Cr-Commit-Position: refs/heads/main@{#81644} Refs: https://github.com/v8/v8/commit/f3cad8cec656fb88e3f16417c12ab8d408167f49 PR-URL: https://github.com/nodejs/node/pull/43531 Refs: https://github.com/nodejs/node/issues/35711 Reviewed-By: Chengzhong Wu Reviewed-By: Darshan Sen --- deps/v8/src/api/api.cc | 9 ++++++--- deps/v8/test/cctest/test-serialize.cc | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index a0ab21d71a378a..393f5471914dd8 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -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(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(data->contexts_.Size()); const int num_contexts = num_additional_contexts + 1; // The default context. diff --git a/deps/v8/test/cctest/test-serialize.cc b/deps/v8/test/cctest/test-serialize.cc index 8c4d6b4722cb61..7d3e57af49d92c 100644 --- a/deps/v8/test/cctest/test-serialize.cc +++ b/deps/v8/test/cctest/test-serialize.cc @@ -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 context = v8::Context::New(isolate); + v8::Context::Scope context_scope(context); + v8::TryCatch try_catch(isolate); + v8::Local 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();