[offthread] Introduce "transfer" handles for off thread

Introduce an OffThreadTransferHandle (and OffThreadTransferMaybeHandle),
which points to a piece of storage known to (and owned by) the
OffThreadIsolate. On Publish, the OffThreadIsolate converts this storage
stub from a raw pointer to an off-thread object into a main-thread
Handle.

This allows users of an OffThreadIsolate to not have to worry (as much)
about the dance of saving raw object pointers before off-thread finishes
and converting those to Handles before it off-thread isolate is
published.

Bug: chromium:1011762
Change-Id: I7ceb4ed85e770bd6e1867649188597bbcaedb32d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2161066
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67337}

src/codegen/compiler.cc

@@ -1320,14 +1320,13 @@ void BackgroundCompileTask::Run() {
 
       parser_->HandleSourceURLComments(off_thread_isolate_.get(), script);
 
+      outer_function_sfi_ =
+          off_thread_isolate_->TransferHandle(outer_function_sfi);
+
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                    "V8.FinalizeCodeBackground.Finish");
       off_thread_isolate_->FinishOffThread();
 
-      // Off-thread handles will become invalid after the handle scope closes,
-      // so save the raw object here.
-      outer_function_sfi_ = *outer_function_sfi;
-
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                    "V8.FinalizeCodeBackground.ReleaseParser");
       DCHECK_EQ(language_mode_, info_->language_mode());
@@ -2564,10 +2563,11 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                    "V8.OffThreadFinalization.Publish");
 
-      Handle<SharedFunctionInfo> sfi(task->outer_function_sfi(), isolate);
-      Handle<Script> script(Script::cast(sfi->script()), isolate);
       task->off_thread_isolate()->Publish(isolate);
 
+      Handle<SharedFunctionInfo> sfi = task->outer_function_sfi();
+      Handle<Script> script(Script::cast(sfi->script()), isolate);
+
       FixUpOffThreadAllocatedScript(isolate, script, source, script_details,
                                     origin_options, NOT_NATIVES_CODE);
 

src/codegen/compiler.h

@@ -12,6 +12,7 @@
 #include "src/codegen/bailout-reason.h"
 #include "src/common/globals.h"
 #include "src/execution/isolate.h"
+#include "src/execution/off-thread-isolate.h"
 #include "src/logging/code-events.h"
 #include "src/objects/contexts.h"
 #include "src/parsing/parse-info.h"
@@ -389,11 +390,9 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
     return finalize_on_background_thread_;
   }
   OffThreadIsolate* off_thread_isolate() { return off_thread_isolate_.get(); }
-  SharedFunctionInfo outer_function_sfi() {
-    // Make sure that this is an off-thread object, so that it won't have been
-    // moved by the GC.
-    DCHECK(Heap::InOffThreadSpace(outer_function_sfi_));
-    return outer_function_sfi_;
+  Handle<SharedFunctionInfo> outer_function_sfi() {
+    DCHECK_NOT_NULL(off_thread_isolate_);
+    return outer_function_sfi_.ToHandle();
   }
 
  private:
@@ -414,8 +413,7 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   // should add some stricter type-safety or DCHECKs to ensure that the user of
   // the task knows this.
   std::unique_ptr<OffThreadIsolate> off_thread_isolate_;
-  // This is a raw pointer to the off-thread allocated SharedFunctionInfo.
-  SharedFunctionInfo outer_function_sfi_;
+  OffThreadTransferHandle<SharedFunctionInfo> outer_function_sfi_;
 
   // Single function data for top-level function compilation.
   int start_position_;

src/execution/off-thread-isolate.cc

@@ -6,25 +6,124 @@
 
 #include "src/execution/isolate.h"
 #include "src/execution/thread-id.h"
+#include "src/handles/handles-inl.h"
 #include "src/logging/off-thread-logger.h"
 
 namespace v8 {
 namespace internal {
 
+class OffThreadTransferHandleStorage {
+ public:
+  enum State { kOffThreadHandle, kRawObject, kHandle };
+
+  explicit OffThreadTransferHandleStorage(
+      Address* off_thread_handle_location,
+      std::unique_ptr<OffThreadTransferHandleStorage> next)
+      : handle_location_(off_thread_handle_location),
+        next_(std::move(next)),
+        state_(kOffThreadHandle) {
+    CheckValid();
+  }
+
+  void ConvertFromOffThreadHandleOnFinish() {
+    CheckValid();
+    DCHECK_EQ(state_, kOffThreadHandle);
+    raw_obj_ptr_ = *handle_location_;
+    state_ = kRawObject;
+    CheckValid();
+  }
+
+  void ConvertToHandleOnPublish(Isolate* isolate) {
+    CheckValid();
+    DCHECK_EQ(state_, kRawObject);
+    handle_location_ = handle(Object(raw_obj_ptr_), isolate).location();
+    state_ = kHandle;
+    CheckValid();
+  }
+
+  Address* handle_location() const {
+    DCHECK_EQ(state_, kHandle);
+    DCHECK(
+        Object(*handle_location_).IsSmi() ||
+        !Heap::InOffThreadSpace(HeapObject::cast(Object(*handle_location_))));
+    return handle_location_;
+  }
+
+  OffThreadTransferHandleStorage* next() { return next_.get(); }
+
+  State state() const { return state_; }
+
+ private:
+  void CheckValid() {
+#ifdef DEBUG
+    Object obj;
+
+    switch (state_) {
+      case kHandle:
+      case kOffThreadHandle:
+        DCHECK_NOT_NULL(handle_location_);
+        obj = Object(*handle_location_);
+        break;
+      case kRawObject:
+        obj = Object(raw_obj_ptr_);
+        break;
+    }
+
+    // Smis are always fine.
+    if (obj.IsSmi()) return;
+
+    // The object that is not yet in a main-thread handle should be in
+    // off-thread space. Main-thread handles can still point to off-thread space
+    // during Publish, so that invariant is taken care of on main-thread handle
+    // access.
+    DCHECK_IMPLIES(state_ != kHandle,
+                   Heap::InOffThreadSpace(HeapObject::cast(obj)));
+#endif
+  }
+
+  union {
+    Address* handle_location_;
+    Address raw_obj_ptr_;
+  };
+  std::unique_ptr<OffThreadTransferHandleStorage> next_;
+  State state_;
+};
+
+Address* OffThreadTransferHandleBase::ToHandleLocation() const {
+  return storage_ == nullptr ? nullptr : storage_->handle_location();
+}
+
 OffThreadIsolate::OffThreadIsolate(Isolate* isolate, Zone* zone)
     : HiddenOffThreadFactory(isolate),
       heap_(isolate->heap()),
       isolate_(isolate),
       logger_(new OffThreadLogger()),
-      handle_zone_(zone) {}
-OffThreadIsolate::~OffThreadIsolate() { delete logger_; }
+      handle_zone_(zone),
+      off_thread_transfer_handles_head_(nullptr) {}
+
+OffThreadIsolate::~OffThreadIsolate() = default;
 
 void OffThreadIsolate::FinishOffThread() {
   heap()->FinishOffThread();
+
+  OffThreadTransferHandleStorage* storage =
+      off_thread_transfer_handles_head_.get();
+  while (storage != nullptr) {
+    storage->ConvertFromOffThreadHandleOnFinish();
+    storage = storage->next();
+  }
+
   handle_zone_ = nullptr;
 }
 
 void OffThreadIsolate::Publish(Isolate* isolate) {
+  OffThreadTransferHandleStorage* storage =
+      off_thread_transfer_handles_head_.get();
+  while (storage != nullptr) {
+    storage->ConvertToHandleOnPublish(isolate);
+    storage = storage->next();
+  }
+
   heap()->Publish(isolate->heap());
 }
 
@@ -51,5 +150,16 @@ void OffThreadIsolate::PinToCurrentThread() {
   thread_id_ = ThreadId::Current();
 }
 
+OffThreadTransferHandleStorage* OffThreadIsolate::AddTransferHandleStorage(
+    HandleBase handle) {
+  DCHECK_IMPLIES(off_thread_transfer_handles_head_ != nullptr,
+                 off_thread_transfer_handles_head_->state() ==
+                     OffThreadTransferHandleStorage::kOffThreadHandle);
+  off_thread_transfer_handles_head_ =
+      std::make_unique<OffThreadTransferHandleStorage>(
+          handle.location(), std::move(off_thread_transfer_handles_head_));
+  return off_thread_transfer_handles_head_.get();
+}
+
 }  // namespace internal
 }  // namespace v8

src/execution/off-thread-isolate.h

@@ -8,6 +8,7 @@
 #include "src/base/logging.h"
 #include "src/execution/thread-id.h"
 #include "src/handles/handles.h"
+#include "src/handles/maybe-handles.h"
 #include "src/heap/off-thread-factory.h"
 #include "src/heap/off-thread-heap.h"
 
@@ -16,6 +17,45 @@ namespace internal {
 
 class Isolate;
 class OffThreadLogger;
+class OffThreadTransferHandleStorage;
+
+class OffThreadTransferHandleBase {
+ protected:
+  explicit OffThreadTransferHandleBase(OffThreadTransferHandleStorage* storage)
+      : storage_(storage) {}
+
+  V8_EXPORT_PRIVATE Address* ToHandleLocation() const;
+
+ private:
+  OffThreadTransferHandleStorage* storage_;
+};
+
+// Helper class for transferring ownership of an off-thread allocated object's
+// handler to the main thread. OffThreadTransferHandles should be created before
+// the OffThreadIsolate is finished, and can be accessed as a Handle after the
+// OffThreadIsolate is published.
+template <typename T>
+class OffThreadTransferHandle : public OffThreadTransferHandleBase {
+ public:
+  OffThreadTransferHandle() : OffThreadTransferHandleBase(nullptr) {}
+  explicit OffThreadTransferHandle(OffThreadTransferHandleStorage* storage)
+      : OffThreadTransferHandleBase(storage) {}
+
+  Handle<T> ToHandle() const { return Handle<T>(ToHandleLocation()); }
+};
+
+template <typename T>
+class OffThreadTransferMaybeHandle : public OffThreadTransferHandleBase {
+ public:
+  OffThreadTransferMaybeHandle() : OffThreadTransferHandleBase(nullptr) {}
+  explicit OffThreadTransferMaybeHandle(OffThreadTransferHandleStorage* storage)
+      : OffThreadTransferHandleBase(storage) {}
+
+  MaybeHandle<T> ToHandle() const {
+    Address* location = ToHandleLocation();
+    return location ? Handle<T>(location) : MaybeHandle<T>();
+  }
+};
 
 // HiddenOffThreadFactory parallels Isolate's HiddenFactory
 class V8_EXPORT_PRIVATE HiddenOffThreadFactory : private OffThreadFactory {
@@ -74,6 +114,25 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
     return location;
   }
 
+  template <typename T>
+  OffThreadTransferHandle<T> TransferHandle(Handle<T> handle) {
+    DCHECK_NOT_NULL(handle_zone_);
+    if (handle.is_null()) {
+      return OffThreadTransferHandle<T>();
+    }
+    return OffThreadTransferHandle<T>(AddTransferHandleStorage(handle));
+  }
+
+  template <typename T>
+  OffThreadTransferMaybeHandle<T> TransferHandle(MaybeHandle<T> maybe_handle) {
+    DCHECK_NOT_NULL(handle_zone_);
+    Handle<T> handle;
+    if (!maybe_handle.ToHandle(&handle)) {
+      return OffThreadTransferMaybeHandle<T>();
+    }
+    return OffThreadTransferMaybeHandle<T>(AddTransferHandleStorage(handle));
+  }
+
   int GetNextScriptId();
 #if V8_SFI_HAS_UNIQUE_ID
   int GetNextUniqueSharedFunctionInfoId();
@@ -82,7 +141,7 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
   bool NeedsSourcePositionsForProfiling();
   bool is_collecting_type_profile();
 
-  OffThreadLogger* logger() { return logger_; }
+  OffThreadLogger* logger() { return logger_.get(); }
 
   void PinToCurrentThread();
   ThreadId thread_id() { return thread_id_; }
@@ -90,15 +149,19 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
  private:
   friend class v8::internal::OffThreadFactory;
 
+  OffThreadTransferHandleStorage* AddTransferHandleStorage(HandleBase handle);
+
   OffThreadHeap heap_;
 
   // TODO(leszeks): Extract out the fields of the Isolate we want and store
   // those instead of the whole thing.
   Isolate* isolate_;
 
-  OffThreadLogger* logger_;
+  std::unique_ptr<OffThreadLogger> logger_;
   ThreadId thread_id_;
   Zone* handle_zone_;
+  std::unique_ptr<OffThreadTransferHandleStorage>
+      off_thread_transfer_handles_head_;
 };
 
 }  // namespace internal

test/unittests/heap/off-thread-factory-unittest.cc

@@ -133,7 +133,7 @@ TEST_F(OffThreadFactoryTest, OneByteInternalizedString_IsAddedToStringTable) {
   uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
       string_vector.begin(), string_vector.length(), HashSeed(isolate()));
 
-  FixedArray off_thread_wrapper;
+  OffThreadTransferHandle<FixedArray> wrapper;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
@@ -141,15 +141,15 @@ TEST_F(OffThreadFactoryTest, OneByteInternalizedString_IsAddedToStringTable) {
         off_thread_factory()->NewOneByteInternalizedString(string_vector,
                                                            hash_field);
 
-    off_thread_wrapper =
-        *off_thread_factory()->StringWrapperForTest(off_thread_string);
+    wrapper =
+        off_thread_isolate()->TransferHandle(WrapString(off_thread_string));
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<FixedArray> wrapper = handle(off_thread_wrapper, isolate());
   off_thread_isolate()->Publish(isolate());
 
-  Handle<String> string = handle(String::cast(wrapper->get(0)), isolate());
+  Handle<String> string =
+      handle(String::cast(wrapper.ToHandle()->get(0)), isolate());
 
   EXPECT_TRUE(string->IsOneByteEqualTo(CStrVector("foo")));
   EXPECT_TRUE(string->IsInternalizedString());
@@ -172,8 +172,8 @@ TEST_F(OffThreadFactoryTest,
   uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
       string_vector.begin(), string_vector.length(), HashSeed(isolate()));
 
-  FixedArray off_thread_wrapper_1;
-  FixedArray off_thread_wrapper_2;
+  OffThreadTransferHandle<FixedArray> wrapper_1;
+  OffThreadTransferHandle<FixedArray> wrapper_2;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
@@ -184,17 +184,19 @@ TEST_F(OffThreadFactoryTest,
         off_thread_factory()->NewOneByteInternalizedString(string_vector,
                                                            hash_field);
 
-    off_thread_wrapper_1 = *WrapString(off_thread_string_1);
-    off_thread_wrapper_2 = *WrapString(off_thread_string_2);
+    wrapper_1 =
+        off_thread_isolate()->TransferHandle(WrapString(off_thread_string_1));
+    wrapper_2 =
+        off_thread_isolate()->TransferHandle(WrapString(off_thread_string_2));
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<FixedArray> wrapper_1 = handle(off_thread_wrapper_1, isolate());
-  Handle<FixedArray> wrapper_2 = handle(off_thread_wrapper_2, isolate());
   off_thread_isolate()->Publish(isolate());
 
-  Handle<String> string_1 = handle(String::cast(wrapper_1->get(0)), isolate());
-  Handle<String> string_2 = handle(String::cast(wrapper_2->get(0)), isolate());
+  Handle<String> string_1 =
+      handle(String::cast(wrapper_1.ToHandle()->get(0)), isolate());
+  Handle<String> string_2 =
+      handle(String::cast(wrapper_2.ToHandle()->get(0)), isolate());
 
   EXPECT_TRUE(string_1->IsOneByteEqualTo(CStrVector("foo")));
   EXPECT_TRUE(string_1->IsInternalizedString());
@@ -207,20 +209,21 @@ TEST_F(OffThreadFactoryTest, AstRawString_IsInternalized) {
 
   const AstRawString* raw_string = ast_value_factory.GetOneByteString("foo");
 
-  FixedArray off_thread_wrapper;
+  OffThreadTransferHandle<FixedArray> wrapper;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
     ast_value_factory.Internalize(off_thread_isolate());
 
-    off_thread_wrapper = *WrapString(raw_string->string());
+    wrapper =
+        off_thread_isolate()->TransferHandle(WrapString(raw_string->string()));
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<FixedArray> wrapper = handle(off_thread_wrapper, isolate());
   off_thread_isolate()->Publish(isolate());
 
-  Handle<String> string = handle(String::cast(wrapper->get(0)), isolate());
+  Handle<String> string =
+      handle(String::cast(wrapper.ToHandle()->get(0)), isolate());
 
   EXPECT_TRUE(string->IsOneByteEqualTo(CStrVector("foo")));
   EXPECT_TRUE(string->IsInternalizedString());
@@ -230,7 +233,7 @@ TEST_F(OffThreadFactoryTest, AstConsString_CreatesConsString) {
   AstValueFactory ast_value_factory(zone(), isolate()->ast_string_constants(),
                                     HashSeed(isolate()));
 
-  FixedArray off_thread_wrapper;
+  OffThreadTransferHandle<FixedArray> wrapper;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
@@ -242,15 +245,15 @@ TEST_F(OffThreadFactoryTest, AstConsString_CreatesConsString) {
 
     ast_value_factory.Internalize(off_thread_isolate());
 
-    off_thread_wrapper =
-        *WrapString(foobar_string->GetString(off_thread_isolate()));
+    wrapper = off_thread_isolate()->TransferHandle(
+        WrapString(foobar_string->GetString(off_thread_isolate())));
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<FixedArray> wrapper = handle(off_thread_wrapper, isolate());
   off_thread_isolate()->Publish(isolate());
 
-  Handle<String> string = handle(String::cast(wrapper->get(0)), isolate());
+  Handle<String> string =
+      handle(String::cast(wrapper.ToHandle()->get(0)), isolate());
 
   EXPECT_TRUE(string->IsConsString());
   EXPECT_TRUE(string->Equals(*isolate()->factory()->NewStringFromStaticChars(
@@ -260,18 +263,19 @@ TEST_F(OffThreadFactoryTest, AstConsString_CreatesConsString) {
 TEST_F(OffThreadFactoryTest, EmptyScript) {
   FunctionLiteral* program = ParseProgram("");
 
-  SharedFunctionInfo shared;
+  OffThreadTransferHandle<SharedFunctionInfo> shared;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
-    shared = *off_thread_factory()->NewSharedFunctionInfoForLiteral(
-        program, script(), true);
+    shared = off_thread_isolate()->TransferHandle(
+        off_thread_factory()->NewSharedFunctionInfoForLiteral(program, script(),
+                                                              true));
 
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<SharedFunctionInfo> root_sfi = handle(shared, isolate());
   off_thread_isolate()->Publish(isolate());
+  Handle<SharedFunctionInfo> root_sfi = shared.ToHandle();
 
   EXPECT_EQ(root_sfi->function_literal_id(), 0);
 }
@@ -284,18 +288,19 @@ TEST_F(OffThreadFactoryTest, LazyFunction) {
                               ->AsFunctionDeclaration()
                               ->fun();
 
-  SharedFunctionInfo shared;
+  OffThreadTransferHandle<SharedFunctionInfo> shared;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
-    shared = *off_thread_factory()->NewSharedFunctionInfoForLiteral(
-        lazy, script(), true);
+    shared = off_thread_isolate()->TransferHandle(
+        off_thread_factory()->NewSharedFunctionInfoForLiteral(lazy, script(),
+                                                              true));
 
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<SharedFunctionInfo> lazy_sfi = handle(shared, isolate());
   off_thread_isolate()->Publish(isolate());
+  Handle<SharedFunctionInfo> lazy_sfi = shared.ToHandle();
 
   EXPECT_EQ(lazy_sfi->function_literal_id(), 1);
   EXPECT_TRUE(lazy_sfi->Name().IsOneByteEqualTo(CStrVector("lazy")));
@@ -311,18 +316,19 @@ TEST_F(OffThreadFactoryTest, EagerFunction) {
                                ->expression()
                                ->AsFunctionLiteral();
 
-  SharedFunctionInfo shared;
+  OffThreadTransferHandle<SharedFunctionInfo> shared;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
-    shared = *off_thread_factory()->NewSharedFunctionInfoForLiteral(
-        eager, script(), true);
+    shared = off_thread_isolate()->TransferHandle(
+        off_thread_factory()->NewSharedFunctionInfoForLiteral(eager, script(),
+                                                              true));
 
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<SharedFunctionInfo> eager_sfi = handle(shared, isolate());
   off_thread_isolate()->Publish(isolate());
+  Handle<SharedFunctionInfo> eager_sfi = shared.ToHandle();
 
   EXPECT_EQ(eager_sfi->function_literal_id(), 1);
   EXPECT_TRUE(eager_sfi->Name().IsOneByteEqualTo(CStrVector("eager")));
@@ -345,18 +351,19 @@ TEST_F(OffThreadFactoryTest, ImplicitNameFunction) {
                                        ->value()
                                        ->AsFunctionLiteral();
 
-  SharedFunctionInfo shared;
+  OffThreadTransferHandle<SharedFunctionInfo> shared;
   {
     OffThreadHandleScope handle_scope(off_thread_isolate());
 
-    shared = *off_thread_factory()->NewSharedFunctionInfoForLiteral(
-        implicit_name, script(), true);
+    shared = off_thread_isolate()->TransferHandle(
+        off_thread_factory()->NewSharedFunctionInfoForLiteral(implicit_name,
+                                                              script(), true));
 
     off_thread_isolate()->FinishOffThread();
   }
 
-  Handle<SharedFunctionInfo> implicit_name_sfi = handle(shared, isolate());
   off_thread_isolate()->Publish(isolate());
+  Handle<SharedFunctionInfo> implicit_name_sfi = shared.ToHandle();
 
   EXPECT_EQ(implicit_name_sfi->function_literal_id(), 1);
   EXPECT_TRUE(