From 0a266c7ba520a20ccec03c8449150c432bb2886b Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Tue, 30 Apr 2024 15:22:06 +0200 Subject: [PATCH] src: use new V8 API to define stream accessor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define XxxStream.prototype.onread as an accessor in JavaScript sense. Previously it was defined via soon-to-be-deprecated `v8::ObjectTemplate::SetAccessor(..)` which used to call setter even for property stores via stream object. The replacement V8 API `v8::ObjectTemplate::SetNativeDataProperty(..)` defines a properly behaving data property and thus a store to a stream object will not trigger the "onread" setter callback. In order to preserve the desired behavior of storing the value in the receiver's internal field the "onread" property should be defined as a proper JavaScript accessor. PR-URL: https://github.com/nodejs/node/pull/53084 Refs: https://github.com/v8/v8/commit/46c241eb99557fe8205acac5c526650c3847d180 Refs: https://github.com/v8/v8/commit/6ec883986bd417e2a42ddb960bd9449deb7e4639 Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- src/base_object-inl.h | 15 +++++++-------- src/base_object.h | 7 ++----- src/stream_base.cc | 36 +++++++++++++++++++++++++++++++----- src/stream_base.h | 7 +++++++ 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 518b22dabef097..61f30b3cfbdb0f 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -132,19 +132,18 @@ v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const { template void BaseObject::InternalFieldGet( - v8::Local property, - const v8::PropertyCallbackInfo& info) { - info.GetReturnValue().Set( - info.This()->GetInternalField(Field).As()); + const v8::FunctionCallbackInfo& args) { + args.GetReturnValue().Set( + args.This()->GetInternalField(Field).As()); } template -void BaseObject::InternalFieldSet(v8::Local property, - v8::Local value, - const v8::PropertyCallbackInfo& info) { +void BaseObject::InternalFieldSet( + const v8::FunctionCallbackInfo& args) { + v8::Local value = args[0]; // This could be e.g. value->IsFunction(). CHECK(((*value)->*typecheck)()); - info.This()->SetInternalField(Field, value); + args.This()->SetInternalField(Field, value); } bool BaseObject::has_pointer_data() const { diff --git a/src/base_object.h b/src/base_object.h index fd6d62a99ce011..546d968e5ca424 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -111,12 +111,9 @@ class BaseObject : public MemoryRetainer { // Setter/Getter pair for internal fields that can be passed to SetAccessor. template - static void InternalFieldGet(v8::Local property, - const v8::PropertyCallbackInfo& info); + static void InternalFieldGet(const v8::FunctionCallbackInfo& args); template - static void InternalFieldSet(v8::Local property, - v8::Local value, - const v8::PropertyCallbackInfo& info); + static void InternalFieldSet(const v8::FunctionCallbackInfo& args); // This is a bit of a hack. See the override in async_wrap.cc for details. virtual bool IsDoneInitializing() const; diff --git a/src/stream_base.cc b/src/stream_base.cc index af714734f49b83..a8f9f98d413c85 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -492,6 +492,29 @@ Local StreamBase::GetObject() { return GetAsyncWrap()->object(); } +void StreamBase::AddAccessor(v8::Isolate* isolate, + v8::Local signature, + enum v8::PropertyAttribute attributes, + v8::Local t, + JSMethodFunction* getter, + JSMethodFunction* setter, + v8::Local string) { + Local getter_templ = + NewFunctionTemplate(isolate, + getter, + signature, + ConstructorBehavior::kThrow, + SideEffectType::kHasNoSideEffect); + Local setter_templ = + NewFunctionTemplate(isolate, + setter, + signature, + ConstructorBehavior::kThrow, + SideEffectType::kHasSideEffect); + t->PrototypeTemplate()->SetAccessorProperty( + string, getter_templ, setter_templ, attributes); +} + void StreamBase::AddMethod(Isolate* isolate, Local signature, enum PropertyAttribute attributes, @@ -561,11 +584,14 @@ void StreamBase::AddMethods(IsolateData* isolate_data, JSMethod<&StreamBase::WriteString>); t->PrototypeTemplate()->Set(FIXED_ONE_BYTE_STRING(isolate, "isStreamBase"), True(isolate)); - t->PrototypeTemplate()->SetAccessor( - FIXED_ONE_BYTE_STRING(isolate, "onread"), - BaseObject::InternalFieldGet, - BaseObject::InternalFieldSet); + AddAccessor(isolate, + sig, + static_cast(DontDelete | DontEnum), + t, + BaseObject::InternalFieldGet, + BaseObject::InternalFieldSet, + FIXED_ONE_BYTE_STRING(isolate, "onread")); } void StreamBase::RegisterExternalReferences( diff --git a/src/stream_base.h b/src/stream_base.h index 62a8928e144ad6..ccbd769ceaf3c1 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -413,6 +413,13 @@ class StreamBase : public StreamResource { EmitToJSStreamListener default_listener_; void SetWriteResult(const StreamWriteResult& res); + static void AddAccessor(v8::Isolate* isolate, + v8::Local sig, + enum v8::PropertyAttribute attributes, + v8::Local t, + JSMethodFunction* getter, + JSMethodFunction* setter, + v8::Local str); static void AddMethod(v8::Isolate* isolate, v8::Local sig, enum v8::PropertyAttribute attributes,