[offthread] Make publish merging and handle fixup atomic

Make sure that any GCs required for off-thread heap merging happen
before any off-thread handle transferring (both transferring using
OffThreadTransferHandle, and the handles created for the string slot
fixups). This is to avoid the marker from walking Handle roots that
point into off-thread pages which the sweeper doesn't see (and can't
clear mark bits on)

Now, the merging and handle creation is atomic as far as the GC is
concerned. The merging is done before handle creation to avoid the
incremental marker from entering off-thread pages, but we ensure that
the raw objects pointers that point into the off-thread pages (which
are used for creating the main-thread handles) stay valid until the
handle creation completes.

Since handle transfer now happens in the middle of publishing, this
patch also moves the OffThreadTransferHandleStorage ownership over to
OffThreadHeap. This requires some header juggling to avoid leaking
OffThreadTransferHandleStorage into the off-thread-isolate header.

Bug: chromium:1086478, chromium:1011762
Change-Id: Id5e7622d6b5520400a4872c5f6ad396c74b30ca6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2218058Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68043}

src/execution/off-thread-isolate.cc

@@ -7,88 +7,12 @@
 #include "src/execution/isolate.h"
 #include "src/execution/thread-id.h"
 #include "src/handles/handles-inl.h"
+#include "src/handles/off-thread-transfer-handle-storage-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();
 }
@@ -98,32 +22,16 @@ OffThreadIsolate::OffThreadIsolate(Isolate* isolate, Zone* zone)
       heap_(isolate->heap()),
       isolate_(isolate),
       logger_(new OffThreadLogger()),
-      handle_zone_(zone),
-      off_thread_transfer_handles_head_(nullptr) {}
+      handle_zone_(zone) {}
 
 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());
 }
 
@@ -145,16 +53,5 @@ 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

@@ -131,7 +131,7 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
     if (handle.is_null()) {
       return OffThreadTransferHandle<T>();
     }
-    return OffThreadTransferHandle<T>(AddTransferHandleStorage(handle));
+    return OffThreadTransferHandle<T>(heap()->AddTransferHandleStorage(handle));
   }
 
   template <typename T>
@@ -141,7 +141,8 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
     if (!maybe_handle.ToHandle(&handle)) {
       return OffThreadTransferMaybeHandle<T>();
     }
-    return OffThreadTransferMaybeHandle<T>(AddTransferHandleStorage(handle));
+    return OffThreadTransferMaybeHandle<T>(
+        heap()->AddTransferHandleStorage(handle));
   }
 
   int GetNextScriptId();
@@ -159,8 +160,6 @@ 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
@@ -170,8 +169,6 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
   std::unique_ptr<OffThreadLogger> logger_;
   ThreadId thread_id_;
   Zone* handle_zone_;
-  std::unique_ptr<OffThreadTransferHandleStorage>
-      off_thread_transfer_handles_head_;
 };
 
 }  // namespace internal

src/handles/off-thread-transfer-handle-storage-inl.h

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_INL_H_
+#define V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_INL_H_
+
+#include "src/handles/handles-inl.h"
+#include "src/handles/off-thread-transfer-handle-storage.h"
+
+namespace v8 {
+namespace internal {
+
+OffThreadTransferHandleStorage::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 OffThreadTransferHandleStorage::ConvertFromOffThreadHandleOnFinish() {
+  CheckValid();
+  DCHECK_EQ(state_, kOffThreadHandle);
+  raw_obj_ptr_ = *handle_location_;
+  state_ = kRawObject;
+  CheckValid();
+}
+
+void OffThreadTransferHandleStorage::ConvertToHandleOnPublish(
+    Isolate* isolate, DisallowHeapAllocation* no_gc) {
+  CheckValid();
+  DCHECK_EQ(state_, kRawObject);
+  handle_location_ = handle(Object(raw_obj_ptr_), isolate).location();
+  state_ = kHandle;
+  CheckValid();
+}
+
+Address* OffThreadTransferHandleStorage::handle_location() const {
+  CheckValid();
+  DCHECK_EQ(state_, kHandle);
+  return handle_location_;
+}
+
+void OffThreadTransferHandleStorage::CheckValid() const {
+#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 main-thread handle should not be in off-thread space, and vice verse.
+  // Raw object pointers can point to the main-thread heap during Publish, so
+  // we don't check that.
+  DCHECK_IMPLIES(state_ == kOffThreadHandle,
+                 Heap::InOffThreadSpace(HeapObject::cast(obj)));
+  DCHECK_IMPLIES(state_ == kHandle,
+                 !Heap::InOffThreadSpace(HeapObject::cast(obj)));
+#endif
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_INL_H_

src/handles/off-thread-transfer-handle-storage.h

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_H_
+#define V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_H_
+
+#include "src/common/assert-scope.h"
+#include "src/handles/handles.h"
+
+namespace v8 {
+namespace internal {
+
+class OffThreadTransferHandleStorage {
+ public:
+  enum State { kOffThreadHandle, kRawObject, kHandle };
+
+  inline explicit OffThreadTransferHandleStorage(
+      Address* off_thread_handle_location,
+      std::unique_ptr<OffThreadTransferHandleStorage> next);
+
+  inline void ConvertFromOffThreadHandleOnFinish();
+
+  inline void ConvertToHandleOnPublish(Isolate* isolate,
+                                       DisallowHeapAllocation* no_gc);
+
+  inline Address* handle_location() const;
+
+  OffThreadTransferHandleStorage* next() { return next_.get(); }
+
+  State state() const { return state_; }
+
+ private:
+  inline void CheckValid() const;
+
+  union {
+    Address* handle_location_;
+    Address raw_obj_ptr_;
+  };
+  std::unique_ptr<OffThreadTransferHandleStorage> next_;
+  State state_;
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HANDLES_OFF_THREAD_TRANSFER_HANDLE_STORAGE_H_

src/heap/off-thread-heap.cc

@@ -4,7 +4,9 @@
 
 #include "src/heap/off-thread-heap.h"
 
+#include "src/common/assert-scope.h"
 #include "src/common/globals.h"
+#include "src/handles/off-thread-transfer-handle-storage-inl.h"
 #include "src/heap/paged-spaces-inl.h"
 #include "src/heap/spaces-inl.h"
 #include "src/objects/objects-body-descriptors-inl.h"
@@ -17,7 +19,10 @@
 namespace v8 {
 namespace internal {
 
-OffThreadHeap::OffThreadHeap(Heap* heap) : space_(heap), lo_space_(heap) {}
+OffThreadHeap::OffThreadHeap(Heap* heap)
+    : space_(heap),
+      lo_space_(heap),
+      off_thread_transfer_handles_head_(nullptr) {}
 
 bool OffThreadHeap::Contains(HeapObject obj) {
   return space_.Contains(obj) || lo_space_.Contains(obj);
@@ -80,6 +85,13 @@ void OffThreadHeap::FinishOffThread() {
 
   string_slots_ = std::move(string_slot_collector.string_slots);
 
+  OffThreadTransferHandleStorage* storage =
+      off_thread_transfer_handles_head_.get();
+  while (storage != nullptr) {
+    storage->ConvertFromOffThreadHandleOnFinish();
+    storage = storage->next();
+  }
+
   is_finished = true;
 }
 
@@ -88,14 +100,58 @@ void OffThreadHeap::Publish(Heap* heap) {
   Isolate* isolate = heap->isolate();
   ReadOnlyRoots roots(isolate);
 
+  // Before we do anything else, ensure that the old-space can expand to the
+  // size needed for the off-thread objects. Use capacity rather than size since
+  // we're adding entire pages.
+  size_t off_thread_size = space_.Capacity() + lo_space_.Size();
+  if (!heap->CanExpandOldGeneration(off_thread_size)) {
+    heap->CollectAllAvailableGarbage(GarbageCollectionReason::kLastResort);
+    if (!heap->CanExpandOldGeneration(off_thread_size)) {
+      heap->FatalProcessOutOfMemory(
+          "Can't expand old-space enough to merge off-thread pages.");
+    }
+  }
+
+  // Merging and transferring handles should be atomic from the point of view
+  // of the GC, since we neither want the GC to walk main-thread handles that
+  // point into off-thread pages, nor do we want the GC to move the raw
+  // pointers we have into off-thread pages before we've had a chance to turn
+  // them into real handles.
+  // TODO(leszeks): This could be a stronger assertion, that we don't GC at
+  // all.
+  DisallowHeapAllocation no_gc;
+
+  // Merge the spaces.
+  {
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
+                 "V8.OffThreadFinalization.Publish.Merge");
+
+    heap->old_space()->MergeLocalSpace(&space_);
+    heap->lo_space()->MergeOffThreadSpace(&lo_space_);
+
+    DCHECK(heap->CanExpandOldGeneration(0));
+  }
+
+  // Transfer all the transfer handles to be real handles. Make sure to do this
+  // before creating any handle scopes, to allow these handles to live in the
+  // caller's handle scope.
+  OffThreadTransferHandleStorage* storage =
+      off_thread_transfer_handles_head_.get();
+  while (storage != nullptr) {
+    storage->ConvertToHandleOnPublish(isolate, &no_gc);
+    storage = storage->next();
+  }
+
+  // Create a new handle scope after transferring handles, for the slot holder
+  // handles below.
   HandleScope handle_scope(isolate);
 
-  // First, handlify all the string slot holder objects, so that we can keep
-  // track of them if they move.
+  // Handlify all the string slot holder objects, so that we can keep track of
+  // them if they move.
   //
   // TODO(leszeks): We might be able to create a HandleScope-compatible
-  // structure off-thread and merge it into the current handle scope all in one
-  // go (DeferredHandles maybe?).
+  // structure off-thread and merge it into the current handle scope all in
+  // one go (DeferredHandles maybe?).
   std::vector<std::pair<Handle<HeapObject>, Handle<Map>>> heap_object_handles;
   std::vector<Handle<Script>> script_handles;
   {
@@ -103,8 +159,8 @@ void OffThreadHeap::Publish(Heap* heap) {
                  "V8.OffThreadFinalization.Publish.CollectHandles");
     heap_object_handles.reserve(string_slots_.size());
     for (RelativeSlot relative_slot : string_slots_) {
-      // TODO(leszeks): Group slots in the same parent object to avoid creating
-      // multiple duplicate handles.
+      // TODO(leszeks): Group slots in the same parent object to avoid
+      // creating multiple duplicate handles.
       HeapObject obj = HeapObject::FromAddress(relative_slot.object_address);
       heap_object_handles.push_back(
           {handle(obj, isolate), handle(obj.map(), isolate)});
@@ -123,45 +179,20 @@ void OffThreadHeap::Publish(Heap* heap) {
     }
   }
 
-  // Then merge the spaces. At this point, we are allowed to point between (no
-  // longer) off-thread pages and main-thread heap pages, and objects in the
-  // previously off-thread page can move.
-  {
-    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
-                 "V8.OffThreadFinalization.Publish.Merge");
-    Heap* heap = isolate->heap();
+  // After this point, all objects are transferred and all handles are valid,
+  // so we can GC again.
+  no_gc.Release();
 
-    // Ensure that the old-space can expand do the size needed for the
-    // off-thread objects. Use capacity rather than size since we're adding
-    // entire pages.
-    size_t off_thread_size = space_.Capacity() + lo_space_.Size();
-    if (!heap->CanExpandOldGeneration(off_thread_size)) {
-      heap->InvokeNearHeapLimitCallback();
-      if (!heap->CanExpandOldGeneration(off_thread_size)) {
-        heap->CollectAllAvailableGarbage(GarbageCollectionReason::kLastResort);
-        if (!heap->CanExpandOldGeneration(off_thread_size)) {
-          heap->FatalProcessOutOfMemory(
-              "Can't expand old-space enough to merge off-thread pages.");
-        }
-      }
-    }
+  // Possibly trigger a GC if we're close to exhausting the old generation.
+  // TODO(leszeks): Adjust the heuristics here.
+  heap->StartIncrementalMarkingIfAllocationLimitIsReached(
+      heap->GCFlagsForIncrementalMarking(),
+      kGCCallbackScheduleIdleGarbageCollection);
 
-    heap->old_space()->MergeLocalSpace(&space_);
-    heap->lo_space()->MergeOffThreadSpace(&lo_space_);
-
-    DCHECK(heap->CanExpandOldGeneration(0));
-
-    // Possibly trigger a GC if we're close to exhausting the old generation.
-    // TODO(leszeks): Adjust the heuristics here.
-    heap->StartIncrementalMarkingIfAllocationLimitIsReached(
-        heap->GCFlagsForIncrementalMarking(),
-        kGCCallbackScheduleIdleGarbageCollection);
-
-    if (!heap->ShouldExpandOldGenerationOnSlowAllocation() ||
-        !heap->CanExpandOldGeneration(1 * MB)) {
-      heap->CollectGarbage(OLD_SPACE,
-                           GarbageCollectionReason::kAllocationFailure);
-    }
+  if (!heap->ShouldExpandOldGenerationOnSlowAllocation() ||
+      !heap->CanExpandOldGeneration(1 * MB)) {
+    heap->CollectGarbage(OLD_SPACE,
+                         GarbageCollectionReason::kAllocationFailure);
   }
 
   // Iterate the string slots, as an offset from the holders we have handles to.
@@ -277,6 +308,17 @@ HeapObject OffThreadHeap::CreateFillerObjectAt(
   return filler;
 }
 
+OffThreadTransferHandleStorage* OffThreadHeap::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/heap/off-thread-heap.h

@@ -16,6 +16,8 @@
 namespace v8 {
 namespace internal {
 
+class OffThreadTransferHandleStorage;
+
 class V8_EXPORT_PRIVATE OffThreadHeap {
  public:
   explicit OffThreadHeap(Heap* heap);
@@ -40,6 +42,8 @@ class V8_EXPORT_PRIVATE OffThreadHeap {
   HeapObject CreateFillerObjectAt(Address addr, int size,
                                   ClearFreedMemoryMode clear_memory_mode);
 
+  OffThreadTransferHandleStorage* AddTransferHandleStorage(HandleBase handle);
+
   void FinishOffThread();
   void Publish(Heap* heap);
 
@@ -61,6 +65,8 @@ class V8_EXPORT_PRIVATE OffThreadHeap {
   OffThreadLargeObjectSpace lo_space_;
   std::vector<RelativeSlot> string_slots_;
   std::vector<Script> script_list_;
+  std::unique_ptr<OffThreadTransferHandleStorage>
+      off_thread_transfer_handles_head_;
   bool is_finished = false;
 };
 

src/heap/paged-spaces.cc

@@ -18,6 +18,7 @@
 #include "src/heap/read-only-heap.h"
 #include "src/logging/counters.h"
 #include "src/objects/string.h"
+#include "src/utils/utils.h"
 
 namespace v8 {
 namespace internal {
@@ -194,6 +195,14 @@ void PagedSpace::MergeLocalSpace(LocalSpace* other) {
 
     if (merging_from_off_thread) {
       DCHECK_NULL(p->sweeping_slot_set());
+
+      // Make sure the page is entirely white.
+      CHECK(heap()
+                ->incremental_marking()
+                ->non_atomic_marking_state()
+                ->bitmap(p)
+                ->IsClean());
+
       p->SetOldGenerationPageFlags(heap()->incremental_marking()->IsMarking());
       if (heap()->incremental_marking()->black_allocation()) {
         p->CreateBlackArea(p->area_start(), p->HighWaterMark());

test/cctest/heap/heap-tester.h

@@ -33,6 +33,7 @@
   V(InvalidatedSlotsFastToSlow)                             \
   V(InvalidatedSlotsSomeInvalidatedRanges)                  \
   V(TestNewSpaceRefsInCopiedCode)                           \
+  V(GCDuringOffThreadMergeWithTransferHandle)               \
   V(GCFlags)                                                \
   V(MarkCompactCollector)                                   \
   V(MarkCompactEpochCounter)                                \

test/cctest/heap/test-heap.cc

@@ -36,6 +36,7 @@
 #include "src/debug/debug.h"
 #include "src/deoptimizer/deoptimizer.h"
 #include "src/execution/execution.h"
+#include "src/execution/off-thread-isolate.h"
 #include "src/handles/global-handles.h"
 #include "src/heap/combined-heap.h"
 #include "src/heap/factory.h"
@@ -7002,6 +7003,44 @@ TEST(Regress978156) {
   marking_state->GreyToBlack(filler);
 }
 
+HEAP_TEST(GCDuringOffThreadMergeWithTransferHandle) {
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  HandleScope handle_scope(isolate);
+
+  Zone zone(isolate->allocator(), ZONE_NAME);
+  OffThreadIsolate off_thread_isolate(isolate, &zone);
+
+  OffThreadTransferHandle<FixedArray> transfer_handle;
+
+  {
+    OffThreadHandleScope handle_scope(&off_thread_isolate);
+    Handle<FixedArray> obj =
+        off_thread_isolate.factory()->NewFixedArray(10, AllocationType::kOld);
+
+    transfer_handle = off_thread_isolate.TransferHandle(obj);
+
+    off_thread_isolate.FinishOffThread();
+  }
+
+  heap->set_force_oom(true);
+
+  heap->AddNearHeapLimitCallback(
+      [](void* data, size_t current_heap_limit,
+         size_t initial_heap_limit) -> size_t {
+        Heap* heap = static_cast<Heap*>(data);
+        heap->set_force_oom(false);
+        return 0;
+      },
+      heap);
+
+  off_thread_isolate.Publish(isolate);
+  CHECK(transfer_handle.ToHandle()->IsFixedArray());
+  CHECK_EQ(transfer_handle.ToHandle()->length(), 10);
+}
+
 }  // namespace heap
 }  // namespace internal
 }  // namespace v8

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

@@ -371,5 +371,37 @@ TEST_F(OffThreadFactoryTest, ImplicitNameFunction) {
       implicit_name_sfi->Name().IsOneByteEqualTo(CStrVector("implicit_name")));
 }
 
+TEST_F(OffThreadFactoryTest, GCDuringPublish) {
+  FunctionLiteral* program = ParseProgram("let implicit_name = function() {}");
+  FunctionLiteral* implicit_name = program->body()
+                                       ->at(0)
+                                       ->AsBlock()
+                                       ->statements()
+                                       ->at(0)
+                                       ->AsExpressionStatement()
+                                       ->expression()
+                                       ->AsAssignment()
+                                       ->value()
+                                       ->AsFunctionLiteral();
+
+  OffThreadTransferHandle<SharedFunctionInfo> shared;
+  {
+    OffThreadHandleScope handle_scope(off_thread_isolate());
+
+    shared = off_thread_isolate()->TransferHandle(
+        off_thread_factory()->NewSharedFunctionInfoForLiteral(implicit_name,
+                                                              script(), true));
+
+    off_thread_isolate()->FinishOffThread();
+  }
+
+  off_thread_isolate()->Publish(isolate());
+  Handle<SharedFunctionInfo> implicit_name_sfi = shared.ToHandle();
+
+  EXPECT_EQ(implicit_name_sfi->function_literal_id(), 1);
+  EXPECT_TRUE(
+      implicit_name_sfi->Name().IsOneByteEqualTo(CStrVector("implicit_name")));
+}
+
 }  // namespace internal
 }  // namespace v8