[api] Add [Shared]ArrayBuffer::GetBackingStore()

This adds an additional V8 API to get the backing store of an array
buffer. Unlike the existing API, the backing store comes wrapped
in a std::shared_ptr, making lifetime management with the embedder
explicit. This obviates the need for the old GetContents() and
Externalize() APIs, which will be deprecated in a future CL.

Contributed by titzer@chromium.org


Bug: v8:9380
Change-Id: I8a87f5dc141dab684693fe536b636e33f6e45173
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1807354Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63883}

include/v8-internal.h

@@ -381,6 +381,10 @@ V8_EXPORT internal::Isolate* IsolateFromNeverReadOnlySpaceObject(Address obj);
 // language mode is strict.
 V8_EXPORT bool ShouldThrowOnError(v8::internal::Isolate* isolate);
 
+// A base class for backing stores, which is needed due to vagaries of
+// how static casts work with std::shared_ptr.
+class BackingStoreBase {};
+
 }  // namespace internal
 }  // namespace v8
 

include/v8.h

@@ -4711,6 +4711,33 @@ class V8_EXPORT WasmModuleObjectBuilderStreaming final {
 
 enum class ArrayBufferCreationMode { kInternalized, kExternalized };
 
+/**
+ * A wrapper around the backing store (i.e. the raw memory) of an array buffer.
+ *
+ * The allocation and destruction of backing stores is generally managed by
+ * V8. Clients should always use standard C++ memory ownership types (i.e.
+ * std::unique_ptr and std::shared_ptr) to manage lifetimes of backing stores
+ * properly, since V8 internal objects may alias backing stores.
+ */
+class V8_EXPORT BackingStore : public v8::internal::BackingStoreBase {
+ public:
+  ~BackingStore();
+
+  /**
+   * Return a pointer to the beginning of the memory block for this backing
+   * store. The pointer is only valid as long as this backing store object
+   * lives.
+   */
+  void* Data() const;
+
+  /**
+   * The length (in bytes) of this backing store.
+   */
+  size_t ByteLength() const;
+
+ private:
+  BackingStore();
+};
 
 /**
  * An instance of the built-in ArrayBuffer constructor (ES6 draft 15.13.5).
@@ -4878,6 +4905,15 @@ class V8_EXPORT ArrayBuffer : public Object {
    */
   Contents Externalize();
 
+  /**
+   * Marks this ArrayBuffer external given a witness that the embedder
+   * has fetched the backing store using the new GetBackingStore() function.
+   *
+   * With the new lifetime management of backing stores there is no need for
+   * externalizing, so this function exists only to make the transition easier.
+   */
+  void Externalize(const std::shared_ptr<BackingStore>& backing_store);
+
   /**
    * Get a pointer to the ArrayBuffer's underlying memory block without
    * externalizing it. If the ArrayBuffer is not externalized, this pointer
@@ -4888,6 +4924,16 @@ class V8_EXPORT ArrayBuffer : public Object {
    */
   Contents GetContents();
 
+  /**
+   * Get a shared pointer to the backing store of this array buffer. This
+   * pointer coordinates the lifetime management of the internal storage
+   * with any live ArrayBuffers on the heap, even across isolates. The embedder
+   * should not attempt to manage lifetime of the storage through other means.
+   *
+   * This function replaces both Externalize() and GetContents().
+   */
+  std::shared_ptr<BackingStore> GetBackingStore();
+
   V8_INLINE static ArrayBuffer* Cast(Value* obj);
 
   static const int kInternalFieldCount = V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT;
@@ -5293,6 +5339,15 @@ class V8_EXPORT SharedArrayBuffer : public Object {
    */
   Contents Externalize();
 
+  /**
+   * Marks this SharedArrayBuffer external given a witness that the embedder
+   * has fetched the backing store using the new GetBackingStore() function.
+   *
+   * With the new lifetime management of backing stores there is no need for
+   * externalizing, so this function exists only to make the transition easier.
+   */
+  void Externalize(const std::shared_ptr<BackingStore>& backing_store);
+
   /**
    * Get a pointer to the ArrayBuffer's underlying memory block without
    * externalizing it. If the ArrayBuffer is not externalized, this pointer
@@ -5307,6 +5362,16 @@ class V8_EXPORT SharedArrayBuffer : public Object {
    */
   Contents GetContents();
 
+  /**
+   * Get a shared pointer to the backing store of this array buffer. This
+   * pointer coordinates the lifetime management of the internal storage
+   * with any live ArrayBuffers on the heap, even across isolates. The embedder
+   * should not attempt to manage lifetime of the storage through other means.
+   *
+   * This function replaces both Externalize() and GetContents().
+   */
+  std::shared_ptr<BackingStore> GetBackingStore();
+
   V8_INLINE static SharedArrayBuffer* Cast(Value* obj);
 
   static const int kInternalFieldCount = V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT;

src/api/api.cc

@@ -3725,6 +3725,41 @@ void v8::WasmModuleObject::CheckCast(Value* that) {
                   "Could not convert to wasm module object");
 }
 
+v8::BackingStore::~BackingStore() {
+  auto i_this = reinterpret_cast<const i::BackingStore*>(this);
+  i_this->~BackingStore();  // manually call internal destructor
+}
+
+void* v8::BackingStore::Data() const {
+  return reinterpret_cast<const i::BackingStore*>(this)->buffer_start();
+}
+
+size_t v8::BackingStore::ByteLength() const {
+  return reinterpret_cast<const i::BackingStore*>(this)->byte_length();
+}
+
+std::shared_ptr<v8::BackingStore> v8::ArrayBuffer::GetBackingStore() {
+  i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
+  std::shared_ptr<i::BackingStore> backing_store = self->GetBackingStore();
+  if (!backing_store) {
+    backing_store = i::BackingStore::NewEmptyBackingStore();
+  }
+  i::GlobalBackingStoreRegistry::Register(backing_store);
+  std::shared_ptr<i::BackingStoreBase> bs_base = backing_store;
+  return std::static_pointer_cast<v8::BackingStore>(bs_base);
+}
+
+std::shared_ptr<v8::BackingStore> v8::SharedArrayBuffer::GetBackingStore() {
+  i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
+  std::shared_ptr<i::BackingStore> backing_store = self->GetBackingStore();
+  if (!backing_store) {
+    backing_store = i::BackingStore::NewEmptyBackingStore();
+  }
+  i::GlobalBackingStoreRegistry::Register(backing_store);
+  std::shared_ptr<i::BackingStoreBase> bs_base = backing_store;
+  return std::static_pointer_cast<v8::BackingStore>(bs_base);
+}
+
 void v8::ArrayBuffer::CheckCast(Value* that) {
   i::Handle<i::Object> obj = Utils::OpenHandle(that);
   Utils::ApiCheck(
@@ -7205,10 +7240,10 @@ namespace {
 // The backing store deleter just deletes the indirection, which downrefs
 // the shared pointer. It will get collected normally.
 void BackingStoreDeleter(void* buffer, size_t length, void* info) {
-  auto bs_indirection =
+  std::shared_ptr<i::BackingStore>* bs_indirection =
       reinterpret_cast<std::shared_ptr<i::BackingStore>*>(info);
   if (bs_indirection) {
-    auto backing_store = bs_indirection->get();
+    i::BackingStore* backing_store = bs_indirection->get();
     TRACE_BS("API:delete bs=%p mem=%p (length=%zu)\n", backing_store,
              backing_store->buffer_start(), backing_store->byte_length());
     USE(backing_store);
@@ -7290,6 +7325,15 @@ v8::ArrayBuffer::Contents v8::ArrayBuffer::Externalize() {
   return GetContents(true);
 }
 
+void v8::ArrayBuffer::Externalize(
+    const std::shared_ptr<BackingStore>& backing_store) {
+  i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
+  Utils::ApiCheck(!self->is_external(), "v8_ArrayBuffer_Externalize",
+                  "ArrayBuffer already externalized");
+  self->set_is_external(true);
+  DCHECK_EQ(self->backing_store(), backing_store->Data());
+}
+
 v8::ArrayBuffer::Contents v8::ArrayBuffer::GetContents() {
   return GetContents(false);
 }
@@ -7576,6 +7620,16 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() {
   return GetContents(true);
 }
 
+void v8::SharedArrayBuffer::Externalize(
+    const std::shared_ptr<BackingStore>& backing_store) {
+  i::Handle<i::JSArrayBuffer> self = Utils::OpenHandle(this);
+  Utils::ApiCheck(!self->is_external(), "v8_SharedArrayBuffer_Externalize",
+                  "SharedArrayBuffer already externalized");
+  self->set_is_external(true);
+
+  DCHECK_EQ(self->backing_store(), backing_store->Data());
+}
+
 v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::GetContents() {
   return GetContents(false);
 }

src/d8/d8.cc

@@ -346,7 +346,6 @@ Global<Function> Shell::stringify_function_;
 base::LazyMutex Shell::workers_mutex_;
 bool Shell::allow_new_workers_ = true;
 std::unordered_set<std::shared_ptr<Worker>> Shell::running_workers_;
-std::vector<ExternalizedContents> Shell::externalized_contents_;
 std::atomic<bool> Shell::script_executed_{false};
 base::LazyMutex Shell::isolate_status_lock_;
 std::map<v8::Isolate*, bool> Shell::isolate_status_;
@@ -2586,12 +2585,6 @@ void SourceGroup::JoinThread() {
   thread_->Join();
 }
 
-ExternalizedContents::~ExternalizedContents() {
-  if (data_ != nullptr) {
-    deleter_(data_, length_, deleter_data_);
-  }
-}
-
 void SerializationDataQueue::Enqueue(std::unique_ptr<SerializationData> data) {
   base::MutexGuard lock_guard(&mutex_);
   data_.push_back(std::move(data));
@@ -3171,22 +3164,10 @@ class Serializer : public ValueSerializer::Delegate {
 
   std::unique_ptr<SerializationData> Release() { return std::move(data_); }
 
-  void AppendExternalizedContentsTo(std::vector<ExternalizedContents>* to) {
-    for (auto& contents : externalized_contents_) {
-      auto bs_indirection = reinterpret_cast<std::shared_ptr<i::BackingStore>*>(
-          contents.DeleterData());
-      if (bs_indirection) {
-        auto backing_store = bs_indirection->get();
-        TRACE_BS("d8:append bs=%p mem=%p (length=%zu)\n", backing_store,
-                 backing_store->buffer_start(), backing_store->byte_length());
-        USE(backing_store);
-      }
-    }
-
-    to->insert(to->end(),
-               std::make_move_iterator(externalized_contents_.begin()),
-               std::make_move_iterator(externalized_contents_.end()));
-    externalized_contents_.clear();
+  void AppendBackingStoresTo(std::vector<std::shared_ptr<BackingStore>>* to) {
+    to->insert(to->end(), std::make_move_iterator(backing_stores_.begin()),
+               std::make_move_iterator(backing_stores_.end()));
+    backing_stores_.clear();
   }
 
  protected:
@@ -3206,8 +3187,8 @@ class Serializer : public ValueSerializer::Delegate {
 
     size_t index = shared_array_buffers_.size();
     shared_array_buffers_.emplace_back(isolate_, shared_array_buffer);
-    data_->shared_array_buffer_contents_.push_back(
-        MaybeExternalize(shared_array_buffer));
+    data_->sab_backing_stores_.push_back(
+        shared_array_buffer->GetBackingStore());
     return Just<uint32_t>(static_cast<uint32_t>(index));
   }
 
@@ -3278,17 +3259,6 @@ class Serializer : public ValueSerializer::Delegate {
     }
   }
 
-  template <typename T>
-  typename T::Contents MaybeExternalize(Local<T> array_buffer) {
-    if (array_buffer->IsExternal()) {
-      return array_buffer->GetContents();
-    } else {
-      typename T::Contents contents = array_buffer->Externalize();
-      externalized_contents_.emplace_back(contents);
-      return contents;
-    }
-  }
-
   Maybe<bool> FinalizeTransfer() {
     for (const auto& global_array_buffer : array_buffers_) {
       Local<ArrayBuffer> array_buffer =
@@ -3298,9 +3268,12 @@ class Serializer : public ValueSerializer::Delegate {
         return Nothing<bool>();
       }
 
-      ArrayBuffer::Contents contents = MaybeExternalize(array_buffer);
+      auto backing_store = array_buffer->GetBackingStore();
+      if (!array_buffer->IsExternal()) {
+        array_buffer->Externalize(backing_store);
+      }
+      data_->backing_stores_.push_back(std::move(backing_store));
       array_buffer->Detach();
-      data_->array_buffer_contents_.push_back(contents);
     }
 
     return Just(true);
@@ -3312,7 +3285,7 @@ class Serializer : public ValueSerializer::Delegate {
   std::vector<Global<ArrayBuffer>> array_buffers_;
   std::vector<Global<SharedArrayBuffer>> shared_array_buffers_;
   std::vector<Global<WasmModuleObject>> wasm_modules_;
-  std::vector<ExternalizedContents> externalized_contents_;
+  std::vector<std::shared_ptr<v8::BackingStore>> backing_stores_;
   size_t current_memory_usage_;
 
   DISALLOW_COPY_AND_ASSIGN(Serializer);
@@ -3334,9 +3307,9 @@ class Deserializer : public ValueDeserializer::Delegate {
     }
 
     uint32_t index = 0;
-    for (const auto& contents : data_->array_buffer_contents()) {
-      Local<ArrayBuffer> array_buffer =
-          ArrayBuffer::New(isolate_, contents.Data(), contents.ByteLength());
+    for (const auto& backing_store : data_->backing_stores()) {
+      Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
+          isolate_, backing_store->Data(), backing_store->ByteLength());
       deserializer_.TransferArrayBuffer(index++, array_buffer);
     }
 
@@ -3346,11 +3319,10 @@ class Deserializer : public ValueDeserializer::Delegate {
   MaybeLocal<SharedArrayBuffer> GetSharedArrayBufferFromId(
       Isolate* isolate, uint32_t clone_id) override {
     DCHECK_NOT_NULL(data_);
-    if (clone_id < data_->shared_array_buffer_contents().size()) {
-      const SharedArrayBuffer::Contents contents =
-          data_->shared_array_buffer_contents().at(clone_id);
-      return SharedArrayBuffer::New(isolate_, contents.Data(),
-                                    contents.ByteLength());
+    if (clone_id < data_->sab_backing_stores().size()) {
+      auto backing_store = data_->sab_backing_stores().at(clone_id);
+      return SharedArrayBuffer::New(isolate_, backing_store->Data(),
+                                    backing_store->ByteLength());
     }
     return MaybeLocal<SharedArrayBuffer>();
   }
@@ -3382,9 +3354,6 @@ std::unique_ptr<SerializationData> Shell::SerializeValue(
   if (serializer.WriteValue(context, value, transfer).To(&ok)) {
     data = serializer.Release();
   }
-  // Append externalized contents even when WriteValue fails.
-  base::MutexGuard lock_guard(workers_mutex_.Pointer());
-  serializer.AppendExternalizedContentsTo(&externalized_contents_);
   return data;
 }
 
@@ -3426,7 +3395,6 @@ void Shell::WaitForRunningWorkers() {
   base::MutexGuard lock_guard(workers_mutex_.Pointer());
   DCHECK(running_workers_.empty());
   allow_new_workers_ = true;
-  externalized_contents_.clear();
 }
 
 int Shell::Main(int argc, char* argv[]) {

src/d8/d8.h

@@ -111,68 +111,17 @@ class SourceGroup {
   int end_offset_;
 };
 
-// The backing store of an ArrayBuffer or SharedArrayBuffer, after
-// Externalize() has been called on it.
-class ExternalizedContents {
- public:
-  explicit ExternalizedContents(const ArrayBuffer::Contents& contents)
-      : data_(contents.Data()),
-        length_(contents.ByteLength()),
-        deleter_(contents.Deleter()),
-        deleter_data_(contents.DeleterData()) {}
-  explicit ExternalizedContents(const SharedArrayBuffer::Contents& contents)
-      : data_(contents.Data()),
-        length_(contents.ByteLength()),
-        deleter_(contents.Deleter()),
-        deleter_data_(contents.DeleterData()) {}
-  ExternalizedContents(ExternalizedContents&& other) V8_NOEXCEPT
-      : data_(other.data_),
-        length_(other.length_),
-        deleter_(other.deleter_),
-        deleter_data_(other.deleter_data_) {
-    other.data_ = nullptr;
-    other.length_ = 0;
-    other.deleter_ = nullptr;
-    other.deleter_data_ = nullptr;
-  }
-  ExternalizedContents& operator=(ExternalizedContents&& other) V8_NOEXCEPT {
-    if (this != &other) {
-      data_ = other.data_;
-      length_ = other.length_;
-      deleter_ = other.deleter_;
-      deleter_data_ = other.deleter_data_;
-      other.data_ = nullptr;
-      other.length_ = 0;
-      other.deleter_ = nullptr;
-      other.deleter_data_ = nullptr;
-    }
-    return *this;
-  }
-  ~ExternalizedContents();
-
-  void* DeleterData() { return deleter_data_; }
-
- private:
-  void* data_;
-  size_t length_;
-  ArrayBuffer::Contents::DeleterCallback deleter_;
-  void* deleter_data_;
-
-  DISALLOW_COPY_AND_ASSIGN(ExternalizedContents);
-};
-
 class SerializationData {
  public:
   SerializationData() : size_(0) {}
 
   uint8_t* data() { return data_.get(); }
   size_t size() { return size_; }
-  const std::vector<ArrayBuffer::Contents>& array_buffer_contents() {
-    return array_buffer_contents_;
+  const std::vector<std::shared_ptr<v8::BackingStore>>& backing_stores() {
+    return backing_stores_;
   }
-  const std::vector<SharedArrayBuffer::Contents>&
-  shared_array_buffer_contents() {
-    return shared_array_buffer_contents_;
+  const std::vector<std::shared_ptr<v8::BackingStore>>& sab_backing_stores() {
+    return sab_backing_stores_;
   }
   const std::vector<WasmModuleObject::TransferrableModule>&
   transferrable_modules() {
@@ -186,8 +135,8 @@ class SerializationData {
 
   std::unique_ptr<uint8_t, DataDeleter> data_;
   size_t size_;
-  std::vector<ArrayBuffer::Contents> array_buffer_contents_;
-  std::vector<SharedArrayBuffer::Contents> shared_array_buffer_contents_;
+  std::vector<std::shared_ptr<v8::BackingStore>> backing_stores_;
+  std::vector<std::shared_ptr<v8::BackingStore>> sab_backing_stores_;
   std::vector<WasmModuleObject::TransferrableModule> transferrable_modules_;
 
  private:
@@ -523,7 +472,6 @@ class Shell : public i::AllStatic {
   static base::LazyMutex workers_mutex_;  // Guards the following members.
   static bool allow_new_workers_;
   static std::unordered_set<std::shared_ptr<Worker>> running_workers_;
-  static std::vector<ExternalizedContents> externalized_contents_;
 
   // Multiple isolates may update this flag concurrently.
   static std::atomic<bool> script_executed_;

src/inspector/v8-console.cc

@@ -691,7 +691,7 @@ v8::Local<v8::Object> V8Console::createCommandLineAPI(
 
   v8::Local<v8::ArrayBuffer> data =
       v8::ArrayBuffer::New(isolate, sizeof(CommandLineAPIData));
-  *static_cast<CommandLineAPIData*>(data->GetContents().Data()) =
+  *static_cast<CommandLineAPIData*>(data->GetBackingStore()->Data()) =
       CommandLineAPIData(this, sessionId);
   createBoundFunctionProperty(context, commandLineAPI, data, "dir",
                               &V8Console::call<&V8Console::Dir>,

src/inspector/v8-console.h

@@ -106,14 +106,14 @@ class V8Console : public v8::debug::ConsoleDelegate {
                                     int)>
   static void call(const v8::FunctionCallbackInfo<v8::Value>& info) {
     CommandLineAPIData* data = static_cast<CommandLineAPIData*>(
-        info.Data().As<v8::ArrayBuffer>()->GetContents().Data());
+        info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
     (data->first->*func)(info, data->second);
   }
   template <void (V8Console::*func)(const v8::debug::ConsoleCallArguments&,
                                     const v8::debug::ConsoleContext&)>
   static void call(const v8::FunctionCallbackInfo<v8::Value>& info) {
     CommandLineAPIData* data = static_cast<CommandLineAPIData*>(
-        info.Data().As<v8::ArrayBuffer>()->GetContents().Data());
+        info.Data().As<v8::ArrayBuffer>()->GetBackingStore()->Data());
     v8::debug::ConsoleCallArguments args(info);
     (data->first->*func)(args, v8::debug::ConsoleContext());
   }

src/objects/backing-store.cc

@@ -454,6 +454,18 @@ std::unique_ptr<BackingStore> BackingStore::WrapAllocation(
   return std::unique_ptr<BackingStore>(result);
 }
 
+std::unique_ptr<BackingStore> BackingStore::NewEmptyBackingStore() {
+  auto result = new BackingStore(nullptr,                 // start
+                                 0,                       // length
+                                 0,                       // capacity
+                                 SharedFlag::kNotShared,  // shared
+                                 false,                   // is_wasm_memory
+                                 false,                   // free_on_destruct
+                                 false);                  // has_guard_regions
+
+  return std::unique_ptr<BackingStore>(result);
+}
+
 void* BackingStore::get_v8_api_array_buffer_allocator() {
   CHECK(!is_wasm_memory_);
   auto array_buffer_allocator =
@@ -485,7 +497,7 @@ inline GlobalBackingStoreRegistryImpl* impl() {
 
 void GlobalBackingStoreRegistry::Register(
     std::shared_ptr<BackingStore> backing_store) {
-  if (!backing_store) return;
+  if (!backing_store || !backing_store->buffer_start()) return;
 
   base::MutexGuard scope_lock(&impl()->mutex_);
   if (backing_store->globally_registered_) return;
@@ -501,6 +513,8 @@ void GlobalBackingStoreRegistry::Register(
 void GlobalBackingStoreRegistry::Unregister(BackingStore* backing_store) {
   if (!backing_store->globally_registered_) return;
 
+  DCHECK_NOT_NULL(backing_store->buffer_start());
+
   base::MutexGuard scope_lock(&impl()->mutex_);
   const auto& result = impl()->map_.find(backing_store->buffer_start());
   if (result != impl()->map_.end()) {

src/objects/backing-store.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "include/v8-internal.h"
 #include "src/handles/handles.h"
 
 namespace v8 {
@@ -34,7 +35,7 @@ struct SharedWasmMemoryData;
 // and the destructor frees the memory (and page allocation if necessary).
 // Backing stores can also *wrap* embedder-allocated memory. In this case,
 // they do not own the memory, and upon destruction, they do not deallocate it.
-class V8_EXPORT_PRIVATE BackingStore {
+class V8_EXPORT_PRIVATE BackingStore : public BackingStoreBase {
  public:
   ~BackingStore();
 
@@ -62,6 +63,9 @@ class V8_EXPORT_PRIVATE BackingStore {
                                                       SharedFlag shared,
                                                       bool free_on_destruct);
 
+  // Create an empty backing store.
+  static std::unique_ptr<BackingStore> NewEmptyBackingStore();
+
   // Accessors.
   void* buffer_start() const { return buffer_start_; }
   size_t byte_length() const {

src/objects/js-array-buffer.cc

@@ -42,21 +42,21 @@ void JSArrayBuffer::SetupEmpty(SharedFlag shared) {
   set_byte_length(0);
 }
 
-std::shared_ptr<BackingStore> JSArrayBuffer::Detach(
-    bool force_for_wasm_memory) {
-  if (was_detached()) return nullptr;
+void JSArrayBuffer::Detach(bool force_for_wasm_memory) {
+  if (was_detached()) return;
 
   if (force_for_wasm_memory) {
     // Skip the is_detachable() check.
   } else if (!is_detachable()) {
     // Not detachable, do nothing.
-    return nullptr;
+    return;
   }
 
   Isolate* const isolate = GetIsolate();
-  auto backing_store = isolate->heap()->UnregisterBackingStore(*this);
-  CHECK_IMPLIES(force_for_wasm_memory && backing_store,
-                backing_store->is_wasm_memory());
+  if (backing_store()) {
+    auto backing_store = isolate->heap()->UnregisterBackingStore(*this);
+    CHECK_IMPLIES(force_for_wasm_memory, backing_store->is_wasm_memory());
+  }
 
   if (isolate->IsArrayBufferDetachingIntact()) {
     isolate->InvalidateArrayBufferDetachingProtector();
@@ -67,8 +67,6 @@ std::shared_ptr<BackingStore> JSArrayBuffer::Detach(
   set_backing_store(nullptr);
   set_byte_length(0);
   set_was_detached(true);
-
-  return backing_store;
 }
 
 void JSArrayBuffer::Attach(std::shared_ptr<BackingStore> backing_store) {

src/objects/js-array-buffer.h

@@ -84,19 +84,17 @@ class JSArrayBuffer : public JSObject {
   // (note: this registers it with src/heap/array-buffer-tracker.h)
   V8_EXPORT_PRIVATE void Attach(std::shared_ptr<BackingStore> backing_store);
 
-  // Detach the backing store from this array buffer if it is detachable
-  // and return a reference to the backing store object. This sets the
-  // internal pointer and length to 0 and unregisters the backing store
-  // from the array buffer tracker.
-  // If the array buffer is not detachable, this is a nop.
+  // Detach the backing store from this array buffer if it is detachable.
+  // This sets the internal pointer and length to 0 and unregisters the backing
+  // store from the array buffer tracker. If the array buffer is not detachable,
+  // this is a nop.
   //
   // Array buffers that wrap wasm memory objects are special in that they
   // are normally not detachable, but can become detached as a side effect
   // of growing the underlying memory object. The {force_for_wasm_memory} flag
   // is used by the implementation of Wasm memory growth in order to bypass the
   // non-detachable check.
-  V8_EXPORT_PRIVATE std::shared_ptr<BackingStore> Detach(
-      bool force_for_wasm_memory = false);
+  V8_EXPORT_PRIVATE void Detach(bool force_for_wasm_memory = false);
 
   // Get a reference to backing store of this array buffer, if there is a
   // backing store. Returns nullptr if there is no backing store (e.g. detached

src/wasm/wasm-js.cc

@@ -206,20 +206,20 @@ i::wasm::ModuleWireBytes GetFirstArgumentAsBytes(
   if (source->IsArrayBuffer()) {
     // A raw array buffer was passed.
     Local<ArrayBuffer> buffer = Local<ArrayBuffer>::Cast(source);
-    ArrayBuffer::Contents contents = buffer->GetContents();
+    auto backing_store = buffer->GetBackingStore();
 
-    start = reinterpret_cast<const uint8_t*>(contents.Data());
-    length = contents.ByteLength();
+    start = reinterpret_cast<const uint8_t*>(backing_store->Data());
+    length = backing_store->ByteLength();
     *is_shared = buffer->IsSharedArrayBuffer();
   } else if (source->IsTypedArray()) {
     // A TypedArray was passed.
     Local<TypedArray> array = Local<TypedArray>::Cast(source);
     Local<ArrayBuffer> buffer = array->Buffer();
 
-    ArrayBuffer::Contents contents = buffer->GetContents();
+    auto backing_store = buffer->GetBackingStore();
 
-    start =
-        reinterpret_cast<const uint8_t*>(contents.Data()) + array->ByteOffset();
+    start = reinterpret_cast<const uint8_t*>(backing_store->Data()) +
+            array->ByteOffset();
     length = array->ByteLength();
     *is_shared = buffer->IsSharedArrayBuffer();
   } else {

test/cctest/test-typedarrays.cc

@@ -82,7 +82,7 @@ TEST(AllocateNotExternal) {
       v8::ArrayBuffer::New(env->GetIsolate(), memory, 1024,
                            v8::ArrayBufferCreationMode::kInternalized);
   CHECK(!buffer->IsExternal());
-  CHECK_EQ(memory, buffer->GetContents().Data());
+  CHECK_EQ(memory, buffer->GetBackingStore()->Data());
 }
 
 void TestSpeciesProtector(char* code,

test/inspector/inspector-test.cc

@@ -968,8 +968,8 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
         data->StoreCurrentStackTrace(description_view);
     v8::Local<v8::ArrayBuffer> buffer =
         v8::ArrayBuffer::New(isolate, sizeof(id));
-    *static_cast<v8_inspector::V8StackTraceId*>(buffer->GetContents().Data()) =
-        id;
+    *static_cast<v8_inspector::V8StackTraceId*>(
+        buffer->GetBackingStore()->Data()) = id;
     args.GetReturnValue().Set(buffer);
   }
 
@@ -983,7 +983,7 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     IsolateData* data = IsolateData::FromContext(context);
     v8_inspector::V8StackTraceId* id =
         static_cast<v8_inspector::V8StackTraceId*>(
-            args[0].As<v8::ArrayBuffer>()->GetContents().Data());
+            args[0].As<v8::ArrayBuffer>()->GetBackingStore()->Data());
     data->ExternalAsyncTaskStarted(*id);
   }
 
@@ -997,7 +997,7 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     IsolateData* data = IsolateData::FromContext(context);
     v8_inspector::V8StackTraceId* id =
         static_cast<v8_inspector::V8StackTraceId*>(
-            args[0].As<v8::ArrayBuffer>()->GetContents().Data());
+            args[0].As<v8::ArrayBuffer>()->GetBackingStore()->Data());
     data->ExternalAsyncTaskFinished(*id);
   }
 

test/mjsunit/wasm/shared-arraybuffer-worker-simple-gc.js

+// Copyright 2019 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.
+
+// Flags: --expose-gc
+
+const kNumIterations = 10;
+
+function NewWorker() {
+  let script =
+`onmessage = (msg) => {
+   if (msg.memory) postMessage("ack");
+   if (msg.quit) postMessage("bye");
+   gc();
+}`;
+  return new Worker(script, {type: 'string'});
+}
+
+function PingWorker(worker, memory) {
+  worker.postMessage({memory: memory});
+  assertEquals("ack", worker.getMessage());
+  worker.postMessage({quit: true});
+  assertEquals("bye", worker.getMessage());
+}
+
+function AllocMemory() {
+  return new SharedArrayBuffer(1024);
+}
+
+function RunSingleWorkerSingleMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunSingleWorkerTwoMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory(), second = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    PingWorker(worker, second);
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunSingleWorkerMultipleMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    PingWorker(worker, AllocMemory());
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunMultipleWorkerMultipleMemoryTest() {
+  print(arguments.callee.name);
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    let worker = NewWorker();
+    PingWorker(worker, first);
+    PingWorker(worker, AllocMemory());
+    worker.terminate();
+    gc();
+  }
+}
+
+RunSingleWorkerSingleMemoryTest();
+RunSingleWorkerTwoMemoryTest();
+RunSingleWorkerMultipleMemoryTest();
+RunMultipleWorkerMultipleMemoryTest();

test/mjsunit/wasm/shared-memory-worker-simple-gc.js

+// Copyright 2019 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.
+
+// Flags: --experimental-wasm-threads --expose-gc
+
+const kNumIterations = 10;
+
+function NewWorker() {
+  let script =
+`onmessage = (msg) => {
+   if (msg.memory) postMessage("ack");
+   if (msg.quit) postMessage("bye");
+   gc();
+}`;
+  return new Worker(script, {type: 'string'});
+}
+
+function PingWorker(worker, memory) {
+  worker.postMessage({memory: memory});
+  assertEquals("ack", worker.getMessage());
+  worker.postMessage({quit: true});
+  assertEquals("bye", worker.getMessage());
+}
+
+function AllocMemory() {
+  let pages = 1, max = 1;
+  return new WebAssembly.Memory({initial : pages, maximum : max, shared : true});
+}
+
+function RunSingleWorkerSingleMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunSingleWorkerTwoMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory(), second = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    PingWorker(worker, second);
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunSingleWorkerMultipleMemoryTest() {
+  print(arguments.callee.name);
+  let worker = NewWorker();
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    PingWorker(worker, first);
+    PingWorker(worker, AllocMemory());
+    gc();
+  }
+  worker.terminate();
+}
+
+function RunMultipleWorkerMultipleMemoryTest() {
+  print(arguments.callee.name);
+  let first = AllocMemory();
+  for (let i = 0; i < kNumIterations; i++) {
+    print(`iteration ${i}`);
+    let worker = NewWorker();
+    PingWorker(worker, first);
+    PingWorker(worker, AllocMemory());
+    worker.terminate();
+    gc();
+  }
+}
+
+RunSingleWorkerSingleMemoryTest();
+RunSingleWorkerTwoMemoryTest();
+RunSingleWorkerMultipleMemoryTest();
+RunMultipleWorkerMultipleMemoryTest();

test/unittests/objects/value-serializer-unittest.cc

@@ -1730,7 +1730,7 @@ class ValueSerializerTestWithArrayBufferTransfer : public ValueSerializerTest {
       Context::Scope scope(deserialization_context());
       output_buffer_ = ArrayBuffer::New(isolate(), kTestByteLength);
       const uint8_t data[kTestByteLength] = {0x00, 0x01, 0x80, 0xFF};
-      memcpy(output_buffer_->GetContents().Data(), data, kTestByteLength);
+      memcpy(output_buffer_->GetBackingStore()->Data(), data, kTestByteLength);
     }
   }