From a55820df3ec113382938701938cced46eb1d3430 Mon Sep 17 00:00:00 2001 From: Andreas Haas Date: Tue, 28 Jan 2025 14:50:47 +0100 Subject: [PATCH] Unregister isolate after Isolate::Dispose During Isolate::Dispose, CppHeap sweeping gets started, which tries to load a ForegroundTaskRunner. Loading the ForegroundTaskRunner is, however, not possible anymore after NodePlatform::UnregisterIsolate. --- src/heap_utils.cc | 2 +- src/node_main_instance.cc | 2 +- src/node_worker.cc | 2 +- src/util.cc | 2 +- test/cctest/node_test_fixture.h | 2 +- test/cctest/test_cppgc.cc | 2 +- test/cctest/test_environment.cc | 4 ++-- test/cctest/test_platform.cc | 2 +- test/fuzzers/fuzz_env.cc | 2 +- test/fuzzers/fuzz_strings.cc | 2 +- 10 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/heap_utils.cc b/src/heap_utils.cc index 7b93698c7fe125..f4aa793e554b9c 100644 --- a/src/heap_utils.cc +++ b/src/heap_utils.cc @@ -89,7 +89,7 @@ class JSGraph : public EmbedderGraph { } Node* V8Node(const Local& value) override { - return V8Node(value.As()); + return V8Node(Local(value)); } Node* AddNode(std::unique_ptr node) override { diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 4119ac1b002681..180875a6ed1c3f 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -81,9 +81,9 @@ NodeMainInstance::~NodeMainInstance() { // This should only be done on a main instance that owns its isolate. // IsolateData must be freed before UnregisterIsolate() is called. isolate_data_.reset(); - platform_->UnregisterIsolate(isolate_); } isolate_->Dispose(); + platform_->UnregisterIsolate(isolate_); } ExitCode NodeMainInstance::Run() { diff --git a/src/node_worker.cc b/src/node_worker.cc index e89f60afc3984c..f1df6eb93315cb 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -235,8 +235,8 @@ class WorkerThreadData { // new Isolate at the same address can successfully be registered with // the platform. // (Refs: https://github.com/nodejs/node/issues/30846) - w_->platform_->UnregisterIsolate(isolate); isolate->Dispose(); + w_->platform_->UnregisterIsolate(isolate); // Wait until the platform has cleaned up all relevant resources. while (!platform_finished) { diff --git a/src/util.cc b/src/util.cc index 8bf239db6e47d4..73e56009eaf033 100644 --- a/src/util.cc +++ b/src/util.cc @@ -748,8 +748,8 @@ RAIIIsolateWithoutEntering::RAIIIsolateWithoutEntering(const SnapshotData* data) } RAIIIsolateWithoutEntering::~RAIIIsolateWithoutEntering() { - per_process::v8_platform.Platform()->UnregisterIsolate(isolate_); isolate_->Dispose(); + per_process::v8_platform.Platform()->UnregisterIsolate(isolate_); } RAIIIsolate::RAIIIsolate(const SnapshotData* data) diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index 3414c0be8ad777..82013ffe7667d5 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -123,8 +123,8 @@ class NodeTestFixture : public NodeZeroIsolateTestFixture { void TearDown() override { platform->DrainTasks(isolate_); isolate_->Exit(); - platform->UnregisterIsolate(isolate_); isolate_->Dispose(); + platform->UnregisterIsolate(isolate_); isolate_ = nullptr; NodeZeroIsolateTestFixture::TearDown(); } diff --git a/test/cctest/test_cppgc.cc b/test/cctest/test_cppgc.cc index edd413ae9b956b..350b7945c9e302 100644 --- a/test/cctest/test_cppgc.cc +++ b/test/cctest/test_cppgc.cc @@ -107,8 +107,8 @@ TEST_F(NodeZeroIsolateTestFixture, ExistingCppHeapTest) { platform->DrainTasks(isolate); } - platform->UnregisterIsolate(isolate); isolate->Dispose(); + platform->UnregisterIsolate(isolate); // Check that all the objects are created and destroyed properly. EXPECT_EQ(CppGCed::kConstructCount, 100); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index bcf1ffac81b002..1bfb2810eb652d 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -543,8 +543,8 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) { node::FreeIsolateData(isolate_data); } - data->platform->UnregisterIsolate(isolate); isolate->Dispose(); + data->platform->UnregisterIsolate(isolate); uv_run(&loop, UV_RUN_DEFAULT); CHECK_EQ(uv_loop_close(&loop), 0); @@ -673,8 +673,8 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) { } // Cleanup. - platform->UnregisterIsolate(isolate); isolate->Dispose(); + platform->UnregisterIsolate(isolate); } #endif // _WIN32 diff --git a/test/cctest/test_platform.cc b/test/cctest/test_platform.cc index 54bcc97ecc8866..2cde3eec9ed795 100644 --- a/test/cctest/test_platform.cc +++ b/test/cctest/test_platform.cc @@ -103,8 +103,8 @@ TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) { // Graceful shutdown delegate->Shutdown(); - platform->UnregisterIsolate(isolate); isolate->Dispose(); + platform->UnregisterIsolate(isolate); } TEST_F(PlatformTest, TracingControllerNullptr) { diff --git a/test/fuzzers/fuzz_env.cc b/test/fuzzers/fuzz_env.cc index bace3051f8cecd..5ca295848a974c 100644 --- a/test/fuzzers/fuzz_env.cc +++ b/test/fuzzers/fuzz_env.cc @@ -65,8 +65,8 @@ class FuzzerFixtureHelper { void Teardown() { platform->DrainTasks(isolate_); isolate_->Exit(); - platform->UnregisterIsolate(isolate_); isolate_->Dispose(); + platform->UnregisterIsolate(isolate_); isolate_ = nullptr; } }; diff --git a/test/fuzzers/fuzz_strings.cc b/test/fuzzers/fuzz_strings.cc index 8f5e1a473e3148..936876cdae20d2 100644 --- a/test/fuzzers/fuzz_strings.cc +++ b/test/fuzzers/fuzz_strings.cc @@ -72,8 +72,8 @@ class FuzzerFixtureHelper { void Teardown() { platform->DrainTasks(isolate_); isolate_->Exit(); - platform->UnregisterIsolate(isolate_); isolate_->Dispose(); + platform->UnregisterIsolate(isolate_); isolate_ = nullptr; } };