[shared-struct] Rework ValueSerializer::Delegate::SupportsSharedValues

This CL has two changes:

1. Remove ValueDeserializer::Delegate::SupportsSharedValues. Only
   ValueSerializer::Delegate needs to report whether it supports
   serializing shared values. The ValueDeserializer::Delegate should
   DCHECK if it gets a shared object tag but it doesn't support it.
   This better mirrors what happens with SharedArrayBuffer transfers
   currently.

2. When attempting to serialize a shared object (shared struct, shared
   array, Atomics.Mutex, or Atomics.Condition) when
   !SupportsSharedValues(), throw instead of assert. This is for better
   ergonomics.

Bug: v8:12547
Change-Id: I2bb66830393526578016813c4e3488859dd07073
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3866302
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82870}
This commit is contained in:
Shu-yu Guo 2022-08-30 18:46:38 -07:00 committed by V8 LUCI CQ
parent f03dd79562
commit d855d7f7b1
7 changed files with 49 additions and 30 deletions

View File

@ -183,11 +183,6 @@ class V8_EXPORT ValueDeserializer {
*/
virtual MaybeLocal<SharedArrayBuffer> GetSharedArrayBufferFromId(
Isolate* isolate, uint32_t clone_id);
/**
* Returns whether conveying shared values are supported.
*/
virtual bool SupportsSharedValues() const;
};
ValueDeserializer(Isolate* isolate, const uint8_t* data, size_t size);

View File

@ -3444,8 +3444,6 @@ MaybeLocal<WasmModuleObject> ValueDeserializer::Delegate::GetWasmModuleFromId(
return MaybeLocal<WasmModuleObject>();
}
bool ValueDeserializer::Delegate::SupportsSharedValues() const { return false; }
MaybeLocal<SharedArrayBuffer>
ValueDeserializer::Delegate::GetSharedArrayBufferFromId(Isolate* v8_isolate,
uint32_t id) {

View File

@ -2711,6 +2711,23 @@ 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);
@ -2729,8 +2746,12 @@ 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);
Shell::SerializeValue(isolate, message, transfer, supports_shared_values);
if (data) {
worker->PostMessage(std::move(data));
}
@ -4605,8 +4626,12 @@ 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);
Shell::SerializeValue(isolate, message, transfer, supports_shared_values);
if (data) {
DCHECK(args.Data()->IsExternal());
Local<External> this_value = args.Data().As<External>();
@ -5158,8 +5183,9 @@ bool Shell::HandleUnhandledPromiseRejections(Isolate* isolate) {
class Serializer : public ValueSerializer::Delegate {
public:
explicit Serializer(Isolate* isolate)
: isolate_(isolate),
Serializer(Isolate* isolate, bool supports_shared_values)
: supports_shared_values_(supports_shared_values),
isolate_(isolate),
serializer_(isolate, this),
current_memory_usage_(0) {}
@ -5250,7 +5276,7 @@ class Serializer : public ValueSerializer::Delegate {
void FreeBufferMemory(void* buffer) override { base::Free(buffer); }
bool SupportsSharedValues() const override { return true; }
bool SupportsSharedValues() const override { return supports_shared_values_; }
private:
Maybe<bool> PrepareTransfer(Local<Context> context, Local<Value> transfer) {
@ -5309,6 +5335,8 @@ class Serializer : public ValueSerializer::Delegate {
return Just(true);
}
// This must come before ValueSerializer as it caches this value.
bool supports_shared_values_;
Isolate* isolate_;
ValueSerializer serializer_;
std::unique_ptr<SerializationData> data_;
@ -5365,8 +5393,6 @@ class Deserializer : public ValueDeserializer::Delegate {
isolate_, data_->compiled_wasm_modules().at(transfer_id));
}
bool SupportsSharedValues() const override { return true; }
private:
Isolate* isolate_;
ValueDeserializer deserializer_;
@ -5401,10 +5427,11 @@ class D8Testing {
};
std::unique_ptr<SerializationData> Shell::SerializeValue(
Isolate* isolate, Local<Value> value, Local<Value> transfer) {
Isolate* isolate, Local<Value> value, Local<Value> transfer,
bool supports_shared_values) {
bool ok;
Local<Context> context = isolate->GetCurrentContext();
Serializer serializer(isolate);
Serializer serializer(isolate, supports_shared_values);
std::unique_ptr<SerializationData> data;
if (serializer.WriteValue(context, value, transfer).To(&ok)) {
data = serializer.Release();

View File

@ -526,7 +526,8 @@ 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);
Isolate* isolate, Local<Value> value, Local<Value> transfer,
bool supports_shared_values);
static MaybeLocal<Value> DeserializeValue(
Isolate* isolate, std::unique_ptr<SerializationData> data);
static int* LookupCounter(const char* name);

View File

@ -1103,10 +1103,11 @@ Maybe<bool> ValueSerializer::WriteWasmMemory(Handle<WasmMemoryObject> object) {
#endif // V8_ENABLE_WEBASSEMBLY
Maybe<bool> ValueSerializer::WriteSharedObject(Handle<HeapObject> object) {
if (!supports_shared_values_) {
return ThrowDataCloneError(MessageTemplate::kDataCloneError, object);
}
DCHECK(object->IsShared());
DCHECK(supports_shared_values_);
DCHECK_NOT_NULL(delegate_);
DCHECK(delegate_->SupportsSharedValues());
// 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
@ -1203,7 +1204,6 @@ ValueDeserializer::ValueDeserializer(Isolate* isolate,
delegate_(delegate),
position_(data.begin()),
end_(data.end()),
supports_shared_values_(delegate && delegate->SupportsSharedValues()),
id_map_(isolate->global_handles()->Create(
ReadOnlyRoots(isolate_).empty_fixed_array())) {}
@ -1213,7 +1213,6 @@ ValueDeserializer::ValueDeserializer(Isolate* isolate, const uint8_t* data,
delegate_(nullptr),
position_(data),
end_(data + size),
supports_shared_values_(false),
id_map_(isolate->global_handles()->Create(
ReadOnlyRoots(isolate_).empty_fixed_array())) {}
@ -1570,7 +1569,7 @@ MaybeHandle<Object> ValueDeserializer::ReadObjectInternal() {
return ReadHostObject();
case SerializationTag::kSharedObject:
case SerializationTag::kSharedObjectConveyor:
if (version_ >= 15 && supports_shared_values_) {
if (version_ >= 15) {
if (tag == SerializationTag::kSharedObject) {
return ReadSharedObject();
}
@ -2260,9 +2259,6 @@ MaybeHandle<WasmMemoryObject> ValueDeserializer::ReadWasmMemory() {
MaybeHandle<HeapObject> ValueDeserializer::ReadSharedObject() {
STACK_CHECK(isolate_, MaybeHandle<HeapObject>());
DCHECK_GE(version_, 15);
DCHECK(supports_shared_values_);
DCHECK_NOT_NULL(delegate_);
DCHECK(delegate_->SupportsSharedValues());
uint32_t shared_object_id;
if (!ReadVarint<uint32_t>().To(&shared_object_id)) {
@ -2281,9 +2277,6 @@ MaybeHandle<HeapObject> ValueDeserializer::ReadSharedObject() {
bool ValueDeserializer::ReadSharedObjectConveyor() {
STACK_CHECK(isolate_, false);
DCHECK_GE(version_, 15);
DCHECK(supports_shared_values_);
DCHECK_NOT_NULL(delegate_);
DCHECK(delegate_->SupportsSharedValues());
// This tag appears at most once per deserialization data.
DCHECK_NULL(shared_object_conveyor_);
uint32_t conveyor_id;

View File

@ -333,7 +333,6 @@ class ValueDeserializer {
v8::ValueDeserializer::Delegate* const delegate_;
const uint8_t* position_;
const uint8_t* const end_;
const bool supports_shared_values_;
uint32_t version_ = 0;
uint32_t next_id_ = 0;
bool version_13_broken_data_mode_ = false;

View File

@ -33,6 +33,12 @@ 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();
})();