[shared-struct] Rework shared value serializer API again

This CL fixes redesigns the current API, which does not correctly
manage lifetimes of the shared object conveyors.

See design doc at
https://docs.google.com/document/d/1TV6agY9dafVJFvdPrUAGbEvos8wL2WDnsmf84n3OJVU/edit?usp=sharing

This CL also removes the incorrect behavior of serializing all shared
strings by sharing instead of copying. Shared strings may be sent to
another process, which should still work.

Bug: v8:12547
Change-Id: I7413abd2d871fd3d52c9b433445cfa1d03e4a732
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3868713
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83044}

BUILD.bazel

@@ -1390,8 +1390,8 @@ filegroup(
         "src/handles/maybe-handles.h",
         "src/handles/persistent-handles.cc",
         "src/handles/persistent-handles.h",
-        "src/handles/shared-object-conveyors.cc",
-        "src/handles/shared-object-conveyors.h",
+        "src/handles/shared-object-conveyor-handles.cc",
+        "src/handles/shared-object-conveyor-handles.h",
         "src/heap/base/active-system-pages.cc",
         "src/heap/base/active-system-pages.h",
         "src/heap/allocation-observer.cc",

BUILD.gn

@@ -3089,7 +3089,7 @@ v8_header_set("v8_internal_headers") {
     "src/handles/maybe-handles-inl.h",
     "src/handles/maybe-handles.h",
     "src/handles/persistent-handles.h",
-    "src/handles/shared-object-conveyors.h",
+    "src/handles/shared-object-conveyor-handles.h",
     "src/heap/allocation-observer.h",
     "src/heap/allocation-result.h",
     "src/heap/allocation-stats.h",
@@ -4497,7 +4497,7 @@ v8_source_set("v8_base_without_compiler") {
     "src/handles/handles.cc",
     "src/handles/local-handles.cc",
     "src/handles/persistent-handles.cc",
-    "src/handles/shared-object-conveyors.cc",
+    "src/handles/shared-object-conveyor-handles.cc",
     "src/heap/allocation-observer.cc",
     "src/heap/array-buffer-sweeper.cc",
     "src/heap/base-space.cc",

include/v8-value-serializer.h

@@ -8,6 +8,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <memory>
 #include <utility>
 
 #include "v8-local-handle.h"  // NOLINT(build/include_directory)
@@ -26,8 +27,37 @@ class Value;
 
 namespace internal {
 struct ScriptStreamingData;
+class SharedObjectConveyorHandles;
+class ValueDeserializer;
+class ValueSerializer;
 }  // namespace internal
 
+/**
+ * A move-only class for managing the lifetime of shared value conveyors used
+ * by V8 to keep JS shared values alive in transit when serialized.
+ *
+ * This class is not directly constructible and is always passed to a
+ * ValueSerializer::Delegate via ValueSerializer::SetSharedValueConveyor.
+ *
+ * The embedder must not destruct the SharedValueConveyor until the associated
+ * serialized data will no longer be deserialized.
+ */
+class V8_EXPORT SharedValueConveyor final {
+ public:
+  SharedValueConveyor(SharedValueConveyor&&) noexcept;
+  ~SharedValueConveyor();
+
+  SharedValueConveyor& operator=(SharedValueConveyor&&) noexcept;
+
+ private:
+  friend class internal::ValueSerializer;
+  friend class internal::ValueDeserializer;
+
+  explicit SharedValueConveyor(Isolate* isolate);
+
+  std::unique_ptr<internal::SharedObjectConveyorHandles> private_;
+};
+
 /**
  * Value serialization compatible with the HTML structured clone algorithm.
  * The format is backward-compatible (i.e. safe to store to disk).
@@ -69,9 +99,20 @@ class V8_EXPORT ValueSerializer {
         Isolate* isolate, Local<WasmModuleObject> module);
 
     /**
-     * Returns whether conveying shared values are supported.
+     * Called when the first shared value is serialized. All subsequent shared
+     * values will use the same conveyor.
+     *
+     * The embedder must ensure the lifetime of the conveyor matches the
+     * lifetime of the serialized data.
+     *
+     * If the embedder supports serializing shared values, this method should
+     * return true. Otherwise the embedder should throw an exception and return
+     * false.
+     *
+     * This method is called at most once per serializer.
      */
-    virtual bool SupportsSharedValues() const;
+    virtual bool AdoptSharedValueConveyor(Isolate* isolate,
+                                          SharedValueConveyor&& conveyor);
 
     /**
      * Allocates memory for the buffer of at least the size provided. The actual
@@ -183,6 +224,12 @@ class V8_EXPORT ValueDeserializer {
      */
     virtual MaybeLocal<SharedArrayBuffer> GetSharedArrayBufferFromId(
         Isolate* isolate, uint32_t clone_id);
+
+    /**
+     * Get the SharedValueConveyor previously provided by
+     * ValueSerializer::Delegate::AdoptSharedValueConveyor.
+     */
+    virtual const SharedValueConveyor* GetSharedValueConveyor(Isolate* isolate);
   };
 
   ValueDeserializer(Isolate* isolate, const uint8_t* data, size_t size);

src/api/api.cc

@@ -62,6 +62,7 @@
 #include "src/execution/vm-state-inl.h"
 #include "src/handles/global-handles.h"
 #include "src/handles/persistent-handles.h"
+#include "src/handles/shared-object-conveyor-handles.h"
 #include "src/heap/embedder-tracing.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/heap-write-barrier.h"
@@ -3330,6 +3331,21 @@ MaybeLocal<String> JSON::Stringify(Local<Context> context,
 
 // --- V a l u e   S e r i a l i z a t i o n ---
 
+SharedValueConveyor::SharedValueConveyor(SharedValueConveyor&& other) noexcept
+    : private_(std::move(other.private_)) {}
+
+SharedValueConveyor::~SharedValueConveyor() = default;
+
+SharedValueConveyor& SharedValueConveyor::operator=(
+    SharedValueConveyor&& other) noexcept {
+  private_ = std::move(other.private_);
+  return *this;
+}
+
+SharedValueConveyor::SharedValueConveyor(Isolate* v8_isolate)
+    : private_(std::make_unique<i::SharedObjectConveyorHandles>(
+          reinterpret_cast<i::Isolate*>(v8_isolate))) {}
+
 Maybe<bool> ValueSerializer::Delegate::WriteHostObject(Isolate* v8_isolate,
                                                        Local<Object> object) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
@@ -3353,7 +3369,14 @@ Maybe<uint32_t> ValueSerializer::Delegate::GetWasmModuleTransferId(
   return Nothing<uint32_t>();
 }
 
-bool ValueSerializer::Delegate::SupportsSharedValues() const { return false; }
+bool ValueSerializer::Delegate::AdoptSharedValueConveyor(
+    Isolate* v8_isolate, SharedValueConveyor&& conveyor) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  i_isolate->ScheduleThrow(*i_isolate->factory()->NewError(
+      i_isolate->error_function(), i::MessageTemplate::kDataCloneError,
+      i_isolate->factory()->NewStringFromAsciiChecked("shared value")));
+  return false;
+}
 
 void* ValueSerializer::Delegate::ReallocateBufferMemory(void* old_buffer,
                                                         size_t size,
@@ -3454,6 +3477,15 @@ ValueDeserializer::Delegate::GetSharedArrayBufferFromId(Isolate* v8_isolate,
   return MaybeLocal<SharedArrayBuffer>();
 }
 
+const SharedValueConveyor* ValueDeserializer::Delegate::GetSharedValueConveyor(
+    Isolate* v8_isolate) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  i_isolate->ScheduleThrow(*i_isolate->factory()->NewError(
+      i_isolate->error_function(),
+      i::MessageTemplate::kDataCloneDeserializationError));
+  return nullptr;
+}
+
 struct ValueDeserializer::PrivateData {
   PrivateData(i::Isolate* i_isolate, base::Vector<const uint8_t> data,
               Delegate* delegate)

src/d8/d8.cc

@@ -2712,23 +2712,6 @@ void Shell::WorkerNew(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
 }
 
-namespace {
-bool GetDisableSharedValuesSupportOption(Isolate* isolate,
-                                         Local<Value> options) {
-  if (options->IsObject()) {
-    Local<Context> context = isolate->GetCurrentContext();
-    Local<Object> options_obj = options->ToObject(context).ToLocalChecked();
-    Local<String> name = String::NewFromUtf8Literal(
-        isolate, "disableSharedValuesSupport", NewStringType::kNormal);
-    Local<Value> value;
-    if (options_obj->Get(context, name).ToLocal(&value)) {
-      return value->BooleanValue(isolate);
-    }
-  }
-  return false;
-}
-}  // namespace
-
 void Shell::WorkerPostMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   HandleScope handle_scope(isolate);
@@ -2747,12 +2730,8 @@ void Shell::WorkerPostMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Local<Value> message = args[0];
   Local<Value> transfer =
       args.Length() >= 2 ? args[1] : Undefined(isolate).As<Value>();
-  Local<Value> options =
-      args.Length() >= 3 ? args[2] : Undefined(isolate).As<Value>();
-  bool supports_shared_values =
-      !GetDisableSharedValuesSupportOption(isolate, options);
   std::unique_ptr<SerializationData> data =
-      Shell::SerializeValue(isolate, message, transfer, supports_shared_values);
+      Shell::SerializeValue(isolate, message, transfer);
   if (data) {
     worker->PostMessage(std::move(data));
   }
@@ -4627,12 +4606,8 @@ void Worker::PostMessageOut(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   Local<Value> message = args[0];
   Local<Value> transfer = Undefined(isolate);
-  Local<Value> options =
-      args.Length() >= 3 ? args[2] : Undefined(isolate).As<Value>();
-  bool supports_shared_values =
-      !GetDisableSharedValuesSupportOption(isolate, options);
   std::unique_ptr<SerializationData> data =
-      Shell::SerializeValue(isolate, message, transfer, supports_shared_values);
+      Shell::SerializeValue(isolate, message, transfer);
   if (data) {
     DCHECK(args.Data()->IsExternal());
     Local<External> this_value = args.Data().As<External>();
@@ -5184,9 +5159,8 @@ bool Shell::HandleUnhandledPromiseRejections(Isolate* isolate) {
 
 class Serializer : public ValueSerializer::Delegate {
  public:
-  Serializer(Isolate* isolate, bool supports_shared_values)
-      : supports_shared_values_(supports_shared_values),
-        isolate_(isolate),
+  explicit Serializer(Isolate* isolate)
+      : isolate_(isolate),
         serializer_(isolate, this),
         current_memory_usage_(0) {}
 
@@ -5277,7 +5251,11 @@ class Serializer : public ValueSerializer::Delegate {
 
   void FreeBufferMemory(void* buffer) override { base::Free(buffer); }
 
-  bool SupportsSharedValues() const override { return supports_shared_values_; }
+  bool AdoptSharedValueConveyor(Isolate* isolate,
+                                SharedValueConveyor&& conveyor) override {
+    data_->shared_value_conveyor_.emplace(std::move(conveyor));
+    return true;
+  }
 
  private:
   Maybe<bool> PrepareTransfer(Local<Context> context, Local<Value> transfer) {
@@ -5337,7 +5315,6 @@ class Serializer : public ValueSerializer::Delegate {
   }
 
   // This must come before ValueSerializer as it caches this value.
-  bool supports_shared_values_;
   Isolate* isolate_;
   ValueSerializer serializer_;
   std::unique_ptr<SerializationData> data_;
@@ -5394,6 +5371,14 @@ class Deserializer : public ValueDeserializer::Delegate {
         isolate_, data_->compiled_wasm_modules().at(transfer_id));
   }
 
+  const SharedValueConveyor* GetSharedValueConveyor(Isolate* isolate) override {
+    DCHECK_NOT_NULL(data_);
+    if (data_->shared_value_conveyor()) {
+      return &data_->shared_value_conveyor().value();
+    }
+    return nullptr;
+  }
+
  private:
   Isolate* isolate_;
   ValueDeserializer deserializer_;
@@ -5428,11 +5413,10 @@ class D8Testing {
 };
 
 std::unique_ptr<SerializationData> Shell::SerializeValue(
-    Isolate* isolate, Local<Value> value, Local<Value> transfer,
-    bool supports_shared_values) {
+    Isolate* isolate, Local<Value> value, Local<Value> transfer) {
   bool ok;
   Local<Context> context = isolate->GetCurrentContext();
-  Serializer serializer(isolate, supports_shared_values);
+  Serializer serializer(isolate);
   std::unique_ptr<SerializationData> data;
   if (serializer.WriteValue(context, value, transfer).To(&ok)) {
     data = serializer.Release();

src/d8/d8.h

@@ -17,6 +17,7 @@
 #include "include/v8-array-buffer.h"
 #include "include/v8-isolate.h"
 #include "include/v8-script.h"
+#include "include/v8-value-serializer.h"
 #include "src/base/once.h"
 #include "src/base/platform/time.h"
 #include "src/base/platform/wrappers.h"
@@ -149,6 +150,9 @@ class SerializationData {
   const std::vector<CompiledWasmModule>& compiled_wasm_modules() {
     return compiled_wasm_modules_;
   }
+  const base::Optional<v8::SharedValueConveyor>& shared_value_conveyor() {
+    return shared_value_conveyor_;
+  }
 
  private:
   struct DataDeleter {
@@ -160,6 +164,7 @@ class SerializationData {
   std::vector<std::shared_ptr<v8::BackingStore>> backing_stores_;
   std::vector<std::shared_ptr<v8::BackingStore>> sab_backing_stores_;
   std::vector<CompiledWasmModule> compiled_wasm_modules_;
+  base::Optional<v8::SharedValueConveyor> shared_value_conveyor_;
 
  private:
   friend class Serializer;
@@ -526,8 +531,7 @@ class Shell : public i::AllStatic {
   static void PostBlockingBackgroundTask(std::unique_ptr<Task> task);
 
   static std::unique_ptr<SerializationData> SerializeValue(
-      Isolate* isolate, Local<Value> value, Local<Value> transfer,
-      bool supports_shared_values);
+      Isolate* isolate, Local<Value> value, Local<Value> transfer);
   static MaybeLocal<Value> DeserializeValue(
       Isolate* isolate, std::unique_ptr<SerializationData> data);
   static int* LookupCounter(const char* name);

src/execution/isolate.cc

@@ -61,7 +61,6 @@
 #include "src/execution/vm-state-inl.h"
 #include "src/handles/global-handles-inl.h"
 #include "src/handles/persistent-handles.h"
-#include "src/handles/shared-object-conveyors.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/heap-verifier.h"
 #include "src/heap/local-heap.h"
@@ -3433,8 +3432,6 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator,
 #endif
       next_module_async_evaluating_ordinal_(
           SourceTextModule::kFirstAsyncEvaluatingOrdinal),
-      shared_object_conveyors_(is_shared ? new SharedObjectConveyors(this)
-                                         : nullptr),
       cancelable_task_manager_(new CancelableTaskManager()) {
   TRACE_ISOLATE(constructor);
   CheckIsolateLayout();

src/execution/isolate.h

@@ -134,7 +134,6 @@ class ReadOnlyArtifacts;
 class RegExpStack;
 class RootVisitor;
 class SetupIsolateDelegate;
-class SharedObjectConveyors;
 class Simulator;
 class SnapshotData;
 class StringForwardingTable;
@@ -2000,14 +1999,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   bool owns_shareable_data() { return owns_shareable_data_; }
 
-  SharedObjectConveyors* GetSharedObjectConveyors() const {
-    if (is_shared()) return shared_object_conveyors_.get();
-    if (shared_isolate()) {
-      return shared_isolate()->shared_object_conveyors_.get();
-    }
-    return nullptr;
-  }
-
   bool log_object_relocation() const { return log_object_relocation_; }
 
   // TODO(pthier): Unify with owns_shareable_data() once the flag
@@ -2400,12 +2391,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   // Otherwise this is populated for all Isolates.
   std::vector<Object> shared_heap_object_cache_;
 
-  // When sharing data among isolates, an isolate can send and receive shared
-  // objects with the ValueSerializer and ValueDeserializer. Shared objects that
-  // are in transit use PersistentHandles owned by this data structure to keep
-  // them alive.
-  std::unique_ptr<SharedObjectConveyors> shared_object_conveyors_;
-
   // Used during builtins compilation to build the builtins constants table,
   // which is stored on the root list prior to serialization.
   BuiltinsConstantsTableBuilder* builtins_constants_table_builder_ = nullptr;

src/handles/shared-object-conveyor-handles.cc

+// Copyright 2022 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.
+
+#include "src/handles/shared-object-conveyor-handles.h"
+
+#include "src/objects/objects-inl.h"
+
+namespace v8 {
+namespace internal {
+
+// TODO(v8:12547): Currently the shared isolate owns all the conveyors. Change
+// the owner to the main isolate once the shared isolate is removed.
+SharedObjectConveyorHandles::SharedObjectConveyorHandles(Isolate* isolate)
+    : persistent_handles_(isolate->shared_isolate()->NewPersistentHandles()) {}
+
+uint32_t SharedObjectConveyorHandles::Persist(HeapObject shared_object) {
+  DCHECK(shared_object.IsShared());
+  uint32_t id = static_cast<uint32_t>(shared_objects_.size());
+  shared_objects_.push_back(persistent_handles_->NewHandle(shared_object));
+  return id;
+}
+
+}  // namespace internal
+}  // namespace v8

src/handles/shared-object-conveyors.h → src/handles/shared-object-conveyor-handles.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_HANDLES_SHARED_OBJECT_CONVEYORS_H_
-#define V8_HANDLES_SHARED_OBJECT_CONVEYORS_H_
+#ifndef V8_HANDLES_SHARED_OBJECT_CONVEYOR_HANDLES_H_
+#define V8_HANDLES_SHARED_OBJECT_CONVEYOR_HANDLES_H_
 
 #include <memory>
 #include <vector>
@@ -22,61 +22,35 @@ class PersistentHandles;
 // The conveyor must be allocated in an isolate that remains alive until the
 // ValueDeserializer in the receiving isolate finishes processing the message.
 //
-// Each conveyor has an id that is stable across GCs. Each shared object that is
-// conveyed is gets an id pair (conveyor_id, object_id). Once all objects in a
-// conveyor are received, the conveyor is deleted and its id may be reused for
-// future conveyance.
+// Each shared object gets an id that is stable across GCs.
 //
-// TODO(v8:12547): Currently the shared isolate owns all the conveyors. Change
-// the owner to the main isolate once the shared isolate is removed.
+// The embedder owns the lifetime of instances of this class. See
+// v8::SharedValueConveyor.
 class SharedObjectConveyorHandles {
  public:
-  SharedObjectConveyorHandles(Isolate* isolate, uint32_t id);
+  explicit SharedObjectConveyorHandles(Isolate* isolate);
 
   SharedObjectConveyorHandles(const SharedObjectConveyorHandles&) = delete;
   SharedObjectConveyorHandles& operator=(const SharedObjectConveyorHandles&) =
       delete;
 
-  // Persist, GetPersisted, and HasPersisted are not threadsafe. A particular
-  // conveyor is used by a single thread at a time, either during sending a
-  // message or receiving a message.
   uint32_t Persist(HeapObject shared_object);
-  HeapObject GetPersisted(uint32_t object_id);
+
   bool HasPersisted(uint32_t object_id) const {
     return object_id < shared_objects_.size();
   }
 
-  // Deleting conveyors is threadsafe and may be called from multiple threads.
-  void Delete();
-
-  const uint32_t id;
+  HeapObject GetPersisted(uint32_t object_id) const {
+    DCHECK(HasPersisted(object_id));
+    return *shared_objects_[object_id];
+  }
 
  private:
   std::unique_ptr<PersistentHandles> persistent_handles_;
   std::vector<Handle<HeapObject>> shared_objects_;
 };
 
-// A class to own and manage conveyors. All methods are threadsafe and may be
-// called from multiple threads.
-class SharedObjectConveyors {
- public:
-  explicit SharedObjectConveyors(Isolate* isolate) : isolate_(isolate) {}
-
-  SharedObjectConveyorHandles* NewConveyor();
-  SharedObjectConveyorHandles* MaybeGetConveyor(uint32_t conveyor_id);
-
- private:
-  friend class SharedObjectConveyorHandles;
-
-  bool HasConveyor(uint32_t conveyor_id) const;
-  void DeleteConveyor(uint32_t conveyor_id);
-
-  Isolate* isolate_;
-  base::Mutex conveyors_mutex_;
-  std::vector<std::unique_ptr<SharedObjectConveyorHandles>> conveyors_;
-};
-
 }  // namespace internal
 }  // namespace v8
 
-#endif  // V8_HANDLES_SHARED_OBJECT_CONVEYORS_H_
+#endif  // V8_HANDLES_SHARED_OBJECT_CONVEYOR_HANDLES_H_

src/handles/shared-object-conveyors.cc

-// Copyright 2022 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.
-
-#include "src/handles/shared-object-conveyors.h"
-
-#include "src/objects/objects-inl.h"
-
-namespace v8 {
-namespace internal {
-
-SharedObjectConveyorHandles::SharedObjectConveyorHandles(Isolate* isolate,
-                                                         uint32_t id)
-    : id(id), persistent_handles_(isolate->NewPersistentHandles()) {}
-
-uint32_t SharedObjectConveyorHandles::Persist(HeapObject shared_object) {
-  DCHECK(shared_object.IsShared());
-  uint32_t id = static_cast<uint32_t>(shared_objects_.size());
-  shared_objects_.push_back(persistent_handles_->NewHandle(shared_object));
-  return id;
-}
-
-HeapObject SharedObjectConveyorHandles::GetPersisted(uint32_t object_id) {
-  DCHECK(HasPersisted(object_id));
-  return *shared_objects_[object_id];
-}
-
-void SharedObjectConveyorHandles::Delete() {
-  persistent_handles_->isolate()->GetSharedObjectConveyors()->DeleteConveyor(
-      id);
-}
-
-SharedObjectConveyorHandles* SharedObjectConveyors::NewConveyor() {
-  base::MutexGuard guard(&conveyors_mutex_);
-
-  uint32_t id;
-  if (conveyors_.empty()) {
-    id = 0;
-  } else {
-    size_t i;
-    for (i = 0; i < conveyors_.size(); i++) {
-      if (conveyors_[i] == nullptr) break;
-    }
-    id = static_cast<uint32_t>(i);
-  }
-
-  auto handles = std::make_unique<SharedObjectConveyorHandles>(isolate_, id);
-  if (id < conveyors_.size()) {
-    conveyors_[id] = std::move(handles);
-  } else {
-    DCHECK_EQ(id, conveyors_.size());
-    conveyors_.push_back(std::move(handles));
-  }
-
-  return conveyors_[id].get();
-}
-
-SharedObjectConveyorHandles* SharedObjectConveyors::MaybeGetConveyor(
-    uint32_t conveyor_id) {
-  base::MutexGuard guard(&conveyors_mutex_);
-  if (HasConveyor(conveyor_id)) return conveyors_[conveyor_id].get();
-  return nullptr;
-}
-
-void SharedObjectConveyors::DeleteConveyor(uint32_t conveyor_id) {
-  base::MutexGuard guard(&conveyors_mutex_);
-  DCHECK(HasConveyor(conveyor_id));
-  conveyors_[conveyor_id].reset(nullptr);
-}
-
-bool SharedObjectConveyors::HasConveyor(uint32_t conveyor_id) const {
-  return conveyor_id < conveyors_.size() &&
-         conveyors_[conveyor_id] != nullptr &&
-         conveyors_[conveyor_id]->id == conveyor_id;
-}
-
-}  // namespace internal
-}  // namespace v8

src/objects/value-serializer.cc

@@ -18,7 +18,7 @@
 #include "src/handles/global-handles-inl.h"
 #include "src/handles/handles-inl.h"
 #include "src/handles/maybe-handles-inl.h"
-#include "src/handles/shared-object-conveyors.h"
+#include "src/handles/shared-object-conveyor-handles.h"
 #include "src/heap/factory.h"
 #include "src/numbers/conversions.h"
 #include "src/objects/heap-number-inl.h"
@@ -171,8 +171,6 @@ enum class SerializationTag : uint8_t {
   kSharedArrayBuffer = 'u',
   // A HeapObject shared across Isolates. sharedValueID:uint32_t
   kSharedObject = 'p',
-  // The SharedObjectConveyor used to get shared objects. conveyorID:uint32_t
-  kSharedObjectConveyor = 'q',
   // A wasm module object transfer. next value is its index.
   kWasmModuleTransfer = 'w',
   // The delegate is responsible for processing all following data.
@@ -264,7 +262,6 @@ ValueSerializer::ValueSerializer(Isolate* isolate,
                                  v8::ValueSerializer::Delegate* delegate)
     : isolate_(isolate),
       delegate_(delegate),
-      supports_shared_values_(delegate && delegate->SupportsSharedValues()),
       zone_(isolate->allocator(), ZONE_NAME),
       id_map_(isolate->heap(), ZoneAllocationPolicy(&zone_)),
       array_buffer_transfer_map_(isolate->heap(),
@@ -472,11 +469,7 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
     }
     default:
       if (InstanceTypeChecker::IsString(instance_type)) {
-        auto string = Handle<String>::cast(object);
-        if (v8_flags.shared_string_table && supports_shared_values_) {
-          return WriteSharedObject(String::Share(isolate_, string));
-        }
-        WriteString(string);
+        WriteString(Handle<String>::cast(object));
         return ThrowIfOutOfMemory();
       } else if (InstanceTypeChecker::IsJSReceiver(instance_type)) {
         return WriteJSReceiver(Handle<JSReceiver>::cast(object));
@@ -1103,21 +1096,26 @@ Maybe<bool> ValueSerializer::WriteWasmMemory(Handle<WasmMemoryObject> object) {
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 Maybe<bool> ValueSerializer::WriteSharedObject(Handle<HeapObject> object) {
-  if (!supports_shared_values_) {
+  if (!delegate_ || isolate_->shared_isolate() == nullptr) {
     return ThrowDataCloneError(MessageTemplate::kDataCloneError, object);
   }
 
   DCHECK(object->IsShared());
 
-  // The first time a shared object is serialized, a new conveyor is made and
-  // its id is written. This conveyor is used for every shared object in this
-  // serialization and subsequent deserialization session. Once deserialization
-  // is complete, the conveyor is deleted.
+  // The first time a shared object is serialized, a new conveyor is made. This
+  // conveyor is used for every shared object in this serialization and
+  // subsequent deserialization sessions. The embedder owns the lifetime of the
+  // conveyor.
   if (!shared_object_conveyor_) {
-    shared_object_conveyor_ =
-        isolate_->GetSharedObjectConveyors()->NewConveyor();
-    WriteTag(SerializationTag::kSharedObjectConveyor);
-    WriteVarint(shared_object_conveyor_->id);
+    v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
+    v8::SharedValueConveyor v8_conveyor(v8_isolate);
+    shared_object_conveyor_ = v8_conveyor.private_.get();
+    if (!delegate_->AdoptSharedValueConveyor(v8_isolate,
+                                             std::move(v8_conveyor))) {
+      shared_object_conveyor_ = nullptr;
+      RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, Nothing<bool>());
+      return Nothing<bool>();
+    }
   }
 
   WriteTag(SerializationTag::kSharedObject);
@@ -1223,11 +1221,6 @@ ValueDeserializer::~ValueDeserializer() {
   if (array_buffer_transfer_map_.ToHandle(&transfer_map_handle)) {
     GlobalHandles::Destroy(transfer_map_handle.location());
   }
-
-  if (shared_object_conveyor_) {
-    shared_object_conveyor_->Delete();
-    shared_object_conveyor_ = nullptr;
-  }
 }
 
 Maybe<bool> ValueDeserializer::ReadHeader() {
@@ -1568,16 +1561,9 @@ MaybeHandle<Object> ValueDeserializer::ReadObjectInternal() {
     case SerializationTag::kHostObject:
       return ReadHostObject();
     case SerializationTag::kSharedObject:
-    case SerializationTag::kSharedObjectConveyor:
-      if (version_ >= 15) {
-        if (tag == SerializationTag::kSharedObject) {
-          return ReadSharedObject();
-        }
-        if (!ReadSharedObjectConveyor()) return MaybeHandle<Object>();
-        return ReadObject();
-      }
-      // If the delegate doesn't support shared values (e.g. older version, or
-      // is for deserializing from storage), treat the tag as unknown.
+      if (version_ >= 15) return ReadSharedObject();
+      // If the data doesn't support shared values because it is from an older
+      // version, treat the tag as unknown.
       V8_FALLTHROUGH;
     default:
       // Before there was an explicit tag for host objects, all unknown tags
@@ -2280,47 +2266,27 @@ MaybeHandle<HeapObject> ValueDeserializer::ReadSharedObject() {
     return MaybeHandle<HeapObject>();
   }
 
-  // The conveyor must have already been gotten via the kSharedObjectConveyor
-  // tag.
-  if (shared_object_conveyor_ == nullptr ||
-      !shared_object_conveyor_->HasPersisted(shared_object_id)) {
+  if (!delegate_) {
     ThrowDeserializationExceptionIfNonePending(isolate_);
     return MaybeHandle<HeapObject>();
   }
 
+  if (shared_object_conveyor_ == nullptr) {
+    const v8::SharedValueConveyor* conveyor = delegate_->GetSharedValueConveyor(
+        reinterpret_cast<v8::Isolate*>(isolate_));
+    if (!conveyor) {
+      RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate_, HeapObject);
+      return MaybeHandle<HeapObject>();
+    }
+    shared_object_conveyor_ = conveyor->private_.get();
+  }
+
   Handle<HeapObject> shared_object(
       shared_object_conveyor_->GetPersisted(shared_object_id), isolate_);
   DCHECK(shared_object->IsShared());
   return shared_object;
 }
 
-bool ValueDeserializer::ReadSharedObjectConveyor() {
-  STACK_CHECK(isolate_, false);
-  DCHECK_GE(version_, 15);
-
-  uint32_t conveyor_id;
-  if (!ReadVarint<uint32_t>().To(&conveyor_id)) {
-    RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate_, false);
-    return false;
-  }
-
-  // This tag appears at most once per deserialization data.
-  if (shared_object_conveyor_ != nullptr ||
-      isolate_->GetSharedObjectConveyors() == nullptr) {
-    ThrowDeserializationExceptionIfNonePending(isolate_);
-    return false;
-  }
-
-  shared_object_conveyor_ =
-      isolate_->GetSharedObjectConveyors()->MaybeGetConveyor(conveyor_id);
-  if (shared_object_conveyor_ == nullptr) {
-    ThrowDeserializationExceptionIfNonePending(isolate_);
-    return false;
-  }
-
-  return true;
-}
-
 MaybeHandle<JSObject> ValueDeserializer::ReadHostObject() {
   if (!delegate_) return MaybeHandle<JSObject>();
   STACK_CHECK(isolate_, MaybeHandle<JSObject>());

src/objects/value-serializer.h

@@ -174,7 +174,6 @@ class ValueSerializer {
   uint8_t* buffer_ = nullptr;
   size_t buffer_size_ = 0;
   size_t buffer_capacity_ = 0;
-  const bool supports_shared_values_;
   bool treat_array_buffer_views_as_host_objects_ = false;
   bool out_of_memory_ = false;
   Zone zone_;
@@ -313,7 +312,6 @@ class ValueDeserializer {
   MaybeHandle<WasmMemoryObject> ReadWasmMemory() V8_WARN_UNUSED_RESULT;
 #endif  // V8_ENABLE_WEBASSEMBLY
   MaybeHandle<HeapObject> ReadSharedObject() V8_WARN_UNUSED_RESULT;
-  bool ReadSharedObjectConveyor() V8_WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadHostObject() V8_WARN_UNUSED_RESULT;
 
   /*
@@ -343,7 +341,7 @@ class ValueDeserializer {
   MaybeHandle<SimpleNumberDictionary> array_buffer_transfer_map_;
 
   // The conveyor used to keep shared objects alive.
-  SharedObjectConveyorHandles* shared_object_conveyor_ = nullptr;
+  const SharedObjectConveyorHandles* shared_object_conveyor_ = nullptr;
 };
 
 }  // namespace internal

test/mjsunit/shared-memory/shared-string.js

@@ -2,34 +2,44 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Flags: --shared-string-table --allow-natives-syntax
+// Flags: --harmony-struct --shared-string-table --allow-natives-syntax
 
 if (this.Worker) {
 
 (function TestSharedStringPostMessage() {
   let workerScript =
-      `postMessage("started");
-       onmessage = function(str) {
-         if (!%IsSharedString(str)) {
+      `let Box = new SharedStructType(['payload']);
+       let b1 = new Box();
+       b1.payload = "started";
+       postMessage(b1);
+       onmessage = function(box) {
+         if (!%IsSharedString(box.payload)) {
            throw new Error("str isn't shared");
          }
-         postMessage(str);
+         let b2 = new Box();
+         b2.payload = box.payload;
+         postMessage(b2);
        };`;
 
+  // Strings referenced in shared structs are serialized by sharing.
+
   let worker = new Worker(workerScript, { type: 'string' });
   let started = worker.getMessage();
-  assertTrue(%IsSharedString(started));
-  assertEquals("started", started);
+  assertTrue(%IsSharedString(started.payload));
+  assertEquals("started", started.payload);
 
   // The string literal appears in source and is internalized, so should
   // already be shared.
-  let str_to_send = 'foo';
-  assertTrue(%IsSharedString(str_to_send));
-  worker.postMessage(str_to_send);
-  let str_received = worker.getMessage();
-  assertTrue(%IsSharedString(str_received));
+  let Box = new SharedStructType(['payload']);
+  let box_to_send = new Box();
+  box_to_send.payload = 'foo';
+  assertTrue(%IsSharedString(box_to_send.payload));
+  worker.postMessage(box_to_send);
+  let box_received = worker.getMessage();
+  assertTrue(%IsSharedString(box_received.payload));
+  assertFalse(box_to_send === box_received);
   // Object.is and === won't check pointer equality of Strings.
-  assertTrue(%IsSameHeapObject(str_to_send, str_received));
+  assertTrue(%IsSameHeapObject(box_to_send.payload, box_received.payload));
 
   worker.terminate();
 })();

test/mjsunit/shared-memory/shared-struct-workers.js

@@ -33,12 +33,6 @@ if (this.Worker) {
   assertEquals("worker", struct.string_field);
   assertEquals(42, struct.struct_field.payload);
 
-  // A serializer that doesn't support shared objects should throw.
-  assertThrows(() => {
-    worker.postMessage(struct, undefined,
-                       { disableSharedValuesSupport: true });
-  });
-
   worker.terminate();
 })();
 

test/mjsunit/shared-memory/value-serializer.js

@@ -6,19 +6,8 @@
 // should throw instead of crash on non-existent shared objects in serialized
 // data.
 
-(function Conveyor() {
-  // Conveyor is 'q', ASCII 113.
-  const data = new Uint8Array([255, 15, 113, 0]);
-  assertThrows(() => { d8.serializer.deserialize(data.buffer); });
-})();
-
-(function SharedObjectNoConveyor() {
+(function SharedObject() {
   // Shared object is 'p', ASCII 112.
   const data = new Uint8Array([255, 15, 112, 0]);
   assertThrows(() => { d8.serializer.deserialize(data.buffer); });
 })();
-
-(function SharedObjectAndConveyor() {
-  const data = new Uint8Array([255, 15, 113, 0, 112, 0]);
-  assertThrows(() => { d8.serializer.deserialize(data.buffer); });
-})();