[api] Ensure correct startup and shutdown order

The startup and shutdown order is as follows:

  v8::V8::InitializePlatform(platform);
  v8::V8::Initialize();
  v8::Isolate* isolate = v8::Isolate::New(...);
  ...
  isolate->Dispose();
  v8::V8::Dispose();
  v8::V8::DisposePlatform();

Bug: v8:12309
Change-Id: I043c19173e36b08b02677081a8f14c2b313f6891
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3300129Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78679}

src/execution/isolate.cc

@@ -460,9 +460,7 @@ size_t Isolate::HashIsolateForEmbeddedBlob() {
 
 base::Thread::LocalStorageKey Isolate::isolate_key_;
 base::Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_;
-#if DEBUG
 std::atomic<bool> Isolate::isolate_key_created_{false};
-#endif
 
 namespace {
 // A global counter for all generated Isolates, might overflow.
@@ -519,14 +517,18 @@ Isolate::PerIsolateThreadData* Isolate::FindPerThreadDataForThread(
 
 void Isolate::InitializeOncePerProcess() {
   isolate_key_ = base::Thread::CreateThreadLocalKey();
-#if DEBUG
   bool expected = false;
-  DCHECK_EQ(true, isolate_key_created_.compare_exchange_strong(
+  CHECK(isolate_key_created_.compare_exchange_strong(
       expected, true, std::memory_order_relaxed));
-#endif
   per_isolate_thread_data_key_ = base::Thread::CreateThreadLocalKey();
 }
 
+void Isolate::DisposeOncePerProcess() {
+  bool expected = true;
+  CHECK(isolate_key_created_.compare_exchange_strong(
+      expected, false, std::memory_order_relaxed));
+}
+
 Address Isolate::get_address_from_id(IsolateAddressId id) {
   return isolate_addresses_[id];
 }

src/execution/isolate.h

@@ -579,6 +579,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   };
 
   static void InitializeOncePerProcess();
+  static void DisposeOncePerProcess();
 
   // Creates Isolate object. Must be used instead of constructing Isolate with
   // new operator.
@@ -1982,10 +1983,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   static base::Thread::LocalStorageKey per_isolate_thread_data_key_;
   static base::Thread::LocalStorageKey isolate_key_;
-
-#ifdef DEBUG
   static std::atomic<bool> isolate_key_created_;
-#endif
 
   void Deinit();
 

src/init/v8.cc

@@ -39,30 +39,79 @@
 namespace v8 {
 namespace internal {
 
-V8_DECLARE_ONCE(init_once);
+v8::Platform* V8::platform_ = nullptr;
+
+namespace {
+enum class V8StartupState {
+  kIdle,
+  kPlatformInitializing,
+  kPlatformInitialized,
+  kV8Initializing,
+  kV8Initialized,
+  kV8Disposing,
+  kV8Disposed,
+  kPlatformDisposing,
+  kPlatformDisposed
+};
+
+std::atomic<V8StartupState> v8_startup_state_(V8StartupState::kIdle);
+
+void AdvanceStartupState(V8StartupState expected_next_state) {
+  V8StartupState current_state = v8_startup_state_;
+  CHECK_NE(current_state, V8StartupState::kPlatformDisposed);
+  V8StartupState next_state =
+      static_cast<V8StartupState>(static_cast<int>(current_state) + 1);
+  if (next_state != expected_next_state) {
+    // Ensure the following order:
+    // v8::V8::InitializePlatform(platform);
+    // v8::V8::Initialize();
+    // v8::Isolate* isolate = v8::Isolate::New(...);
+    // ...
+    // isolate->Dispose();
+    // v8::V8::Dispose();
+    // v8::V8::DisposePlatform();
+    FATAL("Wrong intialization order: got %d expected %d!", current_state,
+          next_state);
+  }
+  if (!v8_startup_state_.compare_exchange_strong(current_state, next_state)) {
+    FATAL(
+        "Multiple threads are initializating V8 in the wrong order: expected "
+        "%d got %d!",
+        current_state, v8_startup_state_.load());
+  }
+}
+
+}  // namespace
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
-V8_DECLARE_ONCE(init_natives_once);
 V8_DECLARE_ONCE(init_snapshot_once);
 #endif
 
-v8::Platform* V8::platform_ = nullptr;
-
-void V8::Initialize() { base::CallOnce(&init_once, &InitializeOncePerProcess); }
-
-void V8::Dispose() {
-#if V8_ENABLE_WEBASSEMBLY
-  wasm::WasmEngine::GlobalTearDown();
-#endif  // V8_ENABLE_WEBASSEMBLY
-#if defined(USE_SIMULATOR)
-  Simulator::GlobalTearDown();
+void V8::InitializePlatform(v8::Platform* platform) {
+  AdvanceStartupState(V8StartupState::kPlatformInitializing);
+  CHECK(!platform_);
+  CHECK_NOT_NULL(platform);
+  platform_ = platform;
+  v8::base::SetPrintStackTrace(platform_->GetStackTracePrinter());
+  v8::tracing::TracingCategoryObserver::SetUp();
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+  if (FLAG_enable_system_instrumentation) {
+    // TODO(sartang@microsoft.com): Move to platform specific diagnostics object
+    v8::internal::ETWJITInterface::Register();
+  }
 #endif
-  CallDescriptors::TearDown();
-  ElementsAccessor::TearDown();
-  RegisteredExtension::UnregisterAll();
-  FlagList::ResetAllFlags();  // Frees memory held by string arguments.
+  AdvanceStartupState(V8StartupState::kPlatformInitialized);
 }
 
+#ifdef V8_SANDBOX
+bool V8::InitializeSandbox() {
+  // Platform must have been initialized already.
+  CHECK(platform_);
+  v8::VirtualAddressSpace* vas = GetPlatformVirtualAddressSpace();
+  return GetProcessWideSandbox()->Initialize(vas);
+}
+#endif  // V8_SANDBOX
+
 #define DISABLE_FLAG(flag)                                                    \
   if (FLAG_##flag) {                                                          \
     PrintF(stderr,                                                            \
@@ -70,7 +119,8 @@ void V8::Dispose() {
     FLAG_##flag = false;                                                      \
   }
 
-void V8::InitializeOncePerProcess() {
+void V8::Initialize() {
+  AdvanceStartupState(V8StartupState::kV8Initializing);
   CHECK(platform_);
 
 #ifdef V8_SANDBOX
@@ -201,32 +251,31 @@ void V8::InitializeOncePerProcess() {
 #endif  // V8_ENABLE_WEBASSEMBLY
 
   ExternalReferenceTable::InitializeOncePerProcess();
-}
 
-void V8::InitializePlatform(v8::Platform* platform) {
-  CHECK(!platform_);
-  CHECK(platform);
-  platform_ = platform;
-  v8::base::SetPrintStackTrace(platform_->GetStackTracePrinter());
-  v8::tracing::TracingCategoryObserver::SetUp();
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (FLAG_enable_system_instrumentation) {
-    // TODO(sartang@microsoft.com): Move to platform specific diagnostics object
-    v8::internal::ETWJITInterface::Register();
-  }
-#endif
+  AdvanceStartupState(V8StartupState::kV8Initialized);
 }
 
-#ifdef V8_SANDBOX
-bool V8::InitializeSandbox() {
-  // Platform must have been initialized already.
+#undef DISABLE_FLAG
+
+void V8::Dispose() {
+  AdvanceStartupState(V8StartupState::kV8Disposing);
   CHECK(platform_);
-  v8::VirtualAddressSpace* vas = GetPlatformVirtualAddressSpace();
-  return GetProcessWideSandbox()->Initialize(vas);
-}
+#if V8_ENABLE_WEBASSEMBLY
+  wasm::WasmEngine::GlobalTearDown();
+#endif  // V8_ENABLE_WEBASSEMBLY
+#if defined(USE_SIMULATOR)
+  Simulator::GlobalTearDown();
 #endif
+  CallDescriptors::TearDown();
+  ElementsAccessor::TearDown();
+  RegisteredExtension::UnregisterAll();
+  Isolate::DisposeOncePerProcess();
+  FlagList::ResetAllFlags();  // Frees memory held by string arguments.
+  AdvanceStartupState(V8StartupState::kV8Disposed);
+}
 
 void V8::DisposePlatform() {
+  AdvanceStartupState(V8StartupState::kPlatformDisposing);
   CHECK(platform_);
 #if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
   if (FLAG_enable_system_instrumentation) {
@@ -243,6 +292,7 @@ void V8::DisposePlatform() {
 #endif
 
   platform_ = nullptr;
+  AdvanceStartupState(V8StartupState::kPlatformDisposed);
 }
 
 v8::Platform* V8::GetCurrentPlatform() {

src/init/v8.h

@@ -19,7 +19,6 @@ class Isolate;
 class V8 : public AllStatic {
  public:
   // Global actions.
-
   static void Initialize();
   static void Dispose();
 
@@ -43,10 +42,6 @@ class V8 : public AllStatic {
   static void SetSnapshotBlob(StartupData* snapshot_blob);
 
  private:
-  static void InitializeOncePerProcessImpl();
-  static void InitializeOncePerProcess();
-
-  // v8::Platform to use.
   static v8::Platform* platform_;
 };
 

test/cctest/test-api.cc

@@ -169,22 +169,6 @@ static void Returns42(const v8::FunctionCallbackInfo<v8::Value>& info) {
   info.GetReturnValue().Set(42);
 }
 
-// Tests that call v8::V8::Dispose() cannot be threaded.
-UNINITIALIZED_TEST(InitializeAndDisposeOnce) {
-  CHECK(v8::V8::Initialize());
-  CHECK(v8::V8::Dispose());
-}
-
-
-// Tests that call v8::V8::Dispose() cannot be threaded.
-UNINITIALIZED_TEST(InitializeAndDisposeMultiple) {
-  for (int i = 0; i < 3; ++i) CHECK(v8::V8::Dispose());
-  for (int i = 0; i < 3; ++i) CHECK(v8::V8::Initialize());
-  for (int i = 0; i < 3; ++i) CHECK(v8::V8::Dispose());
-  for (int i = 0; i < 3; ++i) CHECK(v8::V8::Initialize());
-  for (int i = 0; i < 3; ++i) CHECK(v8::V8::Dispose());
-}
-
 THREADED_TEST(Handles) {
   v8::HandleScope scope(CcTest::isolate());
   Local<Context> local_env;