[execution] Replace UseAsSharedIsolate() with Isolate::NewShared()

Isolate::UseAsSharedIsolate() was invoked after the Isolate was already
created. I think it is cleaner to have the shared-flag right when
constructing an Isolate. This way we can use that property already
when setting up the isolate.

Bug: v8:11708
Change-Id: Ibbfee09122b7b0361a5af7a1b559796594834813
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2885041Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74495}

src/execution/isolate.cc

@@ -2904,14 +2904,25 @@ std::atomic<size_t> Isolate::non_disposed_isolates_;
 #endif  // DEBUG
 
 // static
-Isolate* Isolate::New() {
+Isolate* Isolate::New() { return Isolate::Allocate(false); }
+
+// static
+Isolate* Isolate::NewShared(const v8::Isolate::CreateParams& params) {
+  Isolate* isolate = Isolate::Allocate(true);
+  v8::Isolate::Initialize(reinterpret_cast<v8::Isolate*>(isolate), params);
+  return isolate;
+}
+
+// static
+Isolate* Isolate::Allocate(bool is_shared) {
   // IsolateAllocator allocates the memory for the Isolate object according to
   // the given allocation mode.
   std::unique_ptr<IsolateAllocator> isolate_allocator =
       std::make_unique<IsolateAllocator>();
   // Construct Isolate object in the allocated memory.
   void* isolate_ptr = isolate_allocator->isolate_memory();
-  Isolate* isolate = new (isolate_ptr) Isolate(std::move(isolate_allocator));
+  Isolate* isolate =
+      new (isolate_ptr) Isolate(std::move(isolate_allocator), is_shared);
 #ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
   DCHECK(IsAligned(isolate->isolate_root(), kPtrComprCageBaseAlignment));
   DCHECK_EQ(isolate->isolate_root(), isolate->cage_base());
@@ -2974,7 +2985,8 @@ v8::PageAllocator* Isolate::page_allocator() const {
   return isolate_allocator_->page_allocator();
 }
 
-Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator)
+Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator,
+                 bool is_shared)
     : isolate_data_(this, isolate_allocator->GetPtrComprCageBase()),
       isolate_allocator_(std::move(isolate_allocator)),
       id_(isolate_counter.fetch_add(1, std::memory_order_relaxed)),
@@ -2992,7 +3004,8 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator)
 #endif
       next_module_async_evaluating_ordinal_(
           SourceTextModule::kFirstAsyncEvaluatingOrdinal),
-      cancelable_task_manager_(new CancelableTaskManager()) {
+      cancelable_task_manager_(new CancelableTaskManager()),
+      is_shared_(is_shared) {
   TRACE_ISOLATE(constructor);
   CheckIsolateLayout();
 
@@ -3002,6 +3015,18 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator)
 
   handle_scope_data_.Initialize();
 
+  // When pointer compression is on with a per-Isolate cage, allocation in the
+  // shared Isolate can point into the per-Isolate RO heap as the offsets are
+  // constant across Isolates.
+  //
+  // When pointer compression is on with a shared cage or when pointer
+  // compression is off, a shared RO heap is required. Otherwise a shared
+  // allocation requested by a client Isolate could point into the client
+  // Isolate's RO space (e.g. an RO map) whose pages gets unmapped when it is
+  // disposed.
+  CHECK_IMPLIES(is_shared_, COMPRESS_POINTERS_IN_ISOLATE_CAGE_BOOL ||
+                                V8_SHARED_RO_HEAP_BOOL);
+
 #define ISOLATE_INIT_EXECUTE(type, name, initial_value) \
   name##_ = (initial_value);
   ISOLATE_INIT_LIST(ISOLATE_INIT_EXECUTE)

src/execution/isolate.h

@@ -554,6 +554,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   // new operator.
   static Isolate* New();
 
+  // Creates a new shared Isolate object.
+  static Isolate* NewShared(const v8::Isolate::CreateParams& params);
+
   // Deletes Isolate object. Must be used instead of delete operator.
   // Destroys the non-default isolates.
   // Sets default isolate into "has_been_disposed" state rather then destroying,
@@ -1767,22 +1770,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
     using IsDebugActive = HasAsyncEventDelegate::Next<bool, 1>;
   };
 
-  void UseAsSharedIsolate() {
-    // When pointer compression is on with a per-Isolate cage, allocation in the
-    // shared Isolate can point into the per-Isolate RO heap as the offsets are
-    // constant across Isolates.
-    //
-    // When pointer compression is on with a shared cage or when pointer
-    // compression is off, a shared RO heap is required. Otherwise a shared
-    // allocation requested by a client Isolate could point into the client
-    // Isolate's RO space (e.g. an RO map) whose pages gets unmapped when it is
-    // disposed.
-    CHECK(COMPRESS_POINTERS_IN_ISOLATE_CAGE_BOOL || V8_SHARED_RO_HEAP_BOOL);
-    DCHECK(!is_shared_);
-    DCHECK_NULL(shared_isolate_);
-    is_shared_ = true;
-  }
-
   bool is_shared() { return is_shared_; }
   Isolate* shared_isolate() { return shared_isolate_; }
 
@@ -1790,7 +1777,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void DetachFromSharedIsolate();
 
  private:
-  explicit Isolate(std::unique_ptr<IsolateAllocator> isolate_allocator);
+  explicit Isolate(std::unique_ptr<IsolateAllocator> isolate_allocator,
+                   bool is_shared);
   ~Isolate();
 
   bool Init(SnapshotData* startup_snapshot_data,
@@ -1801,6 +1789,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   void InitializeCodeRanges();
   void AddCodeMemoryRange(MemoryRange range);
 
+  // Common method to create an Isolate used by Isolate::New() and
+  // Isolate::NewShared().
+  static Isolate* Allocate(bool is_shared);
+
   static void RemoveContextIdCallback(const v8::WeakCallbackInfo<void>& data);
 
   class ThreadDataTable {
@@ -2180,7 +2172,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   ThreadDataTable thread_data_table_;
 
   // Set to true if this isolate is used as shared heap.
-  bool is_shared_ = false;
+  const bool is_shared_;
 
   // Stores the shared isolate for this client isolate. nullptr for shared
   // isolates or when no shared isolate is used.

test/cctest/heap/test-shared-heap.cc

@@ -57,17 +57,13 @@ UNINITIALIZED_TEST(ConcurrentAllocationInSharedOldSpace) {
 
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = allocator.get();
-  v8::Isolate* shared_isolate = v8::Isolate::New(create_params);
-
-  Isolate* i_shared_isolate = reinterpret_cast<Isolate*>(shared_isolate);
-  i_shared_isolate->UseAsSharedIsolate();
+  Isolate* shared_isolate = Isolate::NewShared(create_params);
 
   std::vector<std::unique_ptr<SharedSpaceAllocationThread>> threads;
   const int kThreads = 4;
 
   for (int i = 0; i < kThreads; i++) {
-    auto thread =
-        std::make_unique<SharedSpaceAllocationThread>(i_shared_isolate);
+    auto thread = std::make_unique<SharedSpaceAllocationThread>(shared_isolate);
     CHECK(thread->Start());
     threads.push_back(std::move(thread));
   }
@@ -76,7 +72,7 @@ UNINITIALIZED_TEST(ConcurrentAllocationInSharedOldSpace) {
     thread->Join();
   }
 
-  shared_isolate->Dispose();
+  Isolate::Delete(shared_isolate);
 }
 
 }  // namespace internal