cppgc: Avoid initializing cppgc platform through V8
Embedders may use cppgc (or v8::CppHeap) earlier than V8's Isolate and
platform are initialized. Require explicit initialization of cppgc to
avoid recurring init calls with potentially conflicting parameters.
Bug: chromium:1056170
Change-Id: I613452954b322c9a5bf074eefd25107b4579958c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2682648
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72573}
diff --git a/include/cppgc/platform.h b/include/cppgc/platform.h
index 571aa80..e10022f 100644
--- a/include/cppgc/platform.h
+++ b/include/cppgc/platform.h
@@ -125,7 +125,7 @@
/**
* Process-global initialization of the garbage collector. Must be called before
- * creating a Heap.
+ * creating a Heap. Must only be called once per process.
*/
V8_EXPORT void InitializeProcess(PageAllocator*);
diff --git a/samples/cppgc/cppgc-sample.cc b/samples/cppgc/cppgc-sample.cc
index befda96..d76c16a 100644
--- a/samples/cppgc/cppgc-sample.cc
+++ b/samples/cppgc/cppgc-sample.cc
@@ -46,11 +46,7 @@
// backend allocation.
auto cppgc_platform = std::make_shared<cppgc::DefaultPlatform>();
// Initialize the process. This must happen before any cppgc::Heap::Create()
- // calls. cppgc::DefaultPlatform::InitializeProcess initializes both cppgc
- // and v8 (if cppgc is not used as a standalone) as needed.
- // If using a platform other than cppgc::DefaultPlatform, should call
- // cppgc::InitializeProcess (for standalone builds) or
- // v8::V8::InitializePlatform (for non-standalone builds) directly.
+ // calls.
cppgc::DefaultPlatform::InitializeProcess(cppgc_platform.get());
// Create a managed heap.
std::unique_ptr<cppgc::Heap> heap = cppgc::Heap::Create(cppgc_platform);
diff --git a/src/heap/cppgc/default-platform.cc b/src/heap/cppgc/default-platform.cc
index fd0a55b..46884d4 100644
--- a/src/heap/cppgc/default-platform.cc
+++ b/src/heap/cppgc/default-platform.cc
@@ -4,20 +4,11 @@
#include <include/cppgc/default-platform.h>
-#if !CPPGC_IS_STANDALONE
-#include <v8.h>
-#endif // !CPPGC_IS_STANDALONE
-
namespace cppgc {
// static
void DefaultPlatform::InitializeProcess(DefaultPlatform* platform) {
-#if CPPGC_IS_STANDALONE
cppgc::InitializeProcess(platform->GetPageAllocator());
-#else
- // v8::V8::InitializePlatform transitively calls cppgc::InitializeProcess.
- v8::V8::InitializePlatform(platform->v8_platform_.get());
-#endif // CPPGC_IS_STANDALONE
}
} // namespace cppgc
diff --git a/src/heap/cppgc/platform.cc b/src/heap/cppgc/platform.cc
index 4d64951..97e5635 100644
--- a/src/heap/cppgc/platform.cc
+++ b/src/heap/cppgc/platform.cc
@@ -16,7 +16,10 @@
}
void InitializeProcess(PageAllocator* page_allocator) {
+ static PageAllocator* allocator = nullptr;
+ CHECK(!allocator);
internal::GlobalGCInfoTable::Create(page_allocator);
+ allocator = page_allocator;
}
void ShutdownProcess() {}
diff --git a/src/init/v8.cc b/src/init/v8.cc
index 524091d..77014d2 100644
--- a/src/init/v8.cc
+++ b/src/init/v8.cc
@@ -162,7 +162,6 @@
platform_ = platform;
v8::base::SetPrintStackTrace(platform_->GetStackTracePrinter());
v8::tracing::TracingCategoryObserver::SetUp();
- cppgc::InitializeProcess(platform->GetPageAllocator());
}
void V8::ShutdownPlatform() {
diff --git a/test/cctest/cctest.cc b/test/cctest/cctest.cc
index 0ac0432..b872e01 100644
--- a/test/cctest/cctest.cc
+++ b/test/cctest/cctest.cc
@@ -27,6 +27,7 @@
#include "test/cctest/cctest.h"
+#include "include/cppgc/platform.h"
#include "include/libplatform/libplatform.h"
#include "include/v8.h"
#include "src/codegen/compiler.h"
@@ -333,6 +334,7 @@
v8::V8::InitializeICUDefaultLocation(argv[0]);
std::unique_ptr<v8::Platform> platform(v8::platform::NewDefaultPlatform());
v8::V8::InitializePlatform(platform.get());
+ cppgc::InitializeProcess(platform->GetPageAllocator());
using HelpOptions = v8::internal::FlagList::HelpOptions;
v8::internal::FlagList::SetFlagsFromCommandLine(
&argc, argv, true, HelpOptions(HelpOptions::kExit, usage.c_str()));
diff --git a/test/unittests/heap/cppgc/run-all-unittests.cc b/test/unittests/heap/cppgc/run-all-unittests.cc
index cdc862e..dc30e75 100644
--- a/test/unittests/heap/cppgc/run-all-unittests.cc
+++ b/test/unittests/heap/cppgc/run-all-unittests.cc
@@ -2,8 +2,30 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "include/cppgc/platform.h"
+#include "test/unittests/heap/cppgc/test-platform.h"
#include "testing/gmock/include/gmock/gmock.h"
+namespace {
+
+class DefaultPlatformEnvironment final : public ::testing::Environment {
+ public:
+ DefaultPlatformEnvironment() = default;
+
+ void SetUp() override {
+ platform_ =
+ std::make_unique<cppgc::internal::testing::TestPlatform>(nullptr);
+ cppgc::InitializeProcess(platform_->GetPageAllocator());
+ }
+
+ void TearDown() override { cppgc::ShutdownProcess(); }
+
+ private:
+ std::shared_ptr<cppgc::internal::testing::TestPlatform> platform_;
+};
+
+} // namespace
+
int main(int argc, char** argv) {
// Don't catch SEH exceptions and continue as the following tests might hang
// in an broken environment on windows.
@@ -13,5 +35,6 @@
testing::FLAGS_gtest_death_test_style = "threadsafe";
testing::InitGoogleMock(&argc, argv);
+ testing::AddGlobalTestEnvironment(new DefaultPlatformEnvironment);
return RUN_ALL_TESTS();
}
diff --git a/test/unittests/heap/cppgc/tests.cc b/test/unittests/heap/cppgc/tests.cc
index 9c4d4e4..b2bed85 100644
--- a/test/unittests/heap/cppgc/tests.cc
+++ b/test/unittests/heap/cppgc/tests.cc
@@ -20,12 +20,10 @@
void TestWithPlatform::SetUpTestSuite() {
platform_ = std::make_unique<TestPlatform>(
std::make_unique<DelegatingTracingController>());
- cppgc::InitializeProcess(platform_->GetPageAllocator());
}
// static
void TestWithPlatform::TearDownTestSuite() {
- cppgc::ShutdownProcess();
platform_.reset();
}
diff --git a/test/unittests/run-all-unittests.cc b/test/unittests/run-all-unittests.cc
index 2511cdd..5ef3fe3 100644
--- a/test/unittests/run-all-unittests.cc
+++ b/test/unittests/run-all-unittests.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "include/cppgc/platform.h"
#include "include/libplatform/libplatform.h"
#include "include/v8.h"
#include "src/base/compiler-specific.h"
@@ -18,6 +19,7 @@
0, v8::platform::IdleTaskSupport::kEnabled);
ASSERT_TRUE(platform_.get() != nullptr);
v8::V8::InitializePlatform(platform_.get());
+ cppgc::InitializeProcess(platform_->GetPageAllocator());
ASSERT_TRUE(v8::V8::Initialize());
}
