Skip to content

Commit 8dadd8c

Browse files
jkleinscjkleinsc
committed
src: do not use deprecated V8 API
nodejs/node#53084
1 parent e95fbb0 commit 8dadd8c

6 files changed

Lines changed: 273 additions & 188 deletions

‎patches/node/.patches‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,5 @@ fix_remove_deprecated_errno_constants.patch
4747
build_enable_perfetto.patch
4848
fix_add_source_location_for_v8_task_runner.patch
4949
cherry-pick_src_remove_calls_to_recently_deprecated_v8_apis.patch
50+
src_do_not_use_deprecated_v8_api.patch
51+
src_use_new_v8_api_to_define_stream_accessor.patch

‎patches/node/src_do_not_use_deprecated_v8_api.patch‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: ishell <isheludko@gmail.com>
3+
Date: Mon, 25 Mar 2024 15:45:41 +0100
4+
Subject: src: do not use deprecated V8 API
5+
MIME-Version: 1.0
6+
Content-Type: text/plain; charset=UTF-8
7+
Content-Transfer-Encoding: 8bit
8+
9+
Namely:
10+
- `v8::ObjectTemplate::SetAccessor(v8::Local<v8::String>, ...);`
11+
- `v8::ObjectTemplate::SetNativeDataProperty` with `AccessControl`
12+
13+
Refs: https://github.com/v8/v8/commit/46c241eb99557fe8205acac5c526650c3847d180
14+
Refs: https://github.com/v8/v8/commit/6ec883986bd417e2a42ddb960bd9449deb7e4639
15+
Co-authored-by: Michaël Zasso <targos@protonmail.com>
16+
PR-URL: https://github.com/nodejs/node/pull/53084
17+
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
18+
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
19+
Reviewed-By: James M Snell <jasnell@gmail.com>
20+
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
21+
(cherry picked from commit 26d5cafff76d3a096ebfd7d7a6279d4b5b190230)
22+
23+
diff --git a/src/base_object-inl.h b/src/base_object-inl.h
24+
index da8fed7b3013df10ae02be2070545c74d9a978f0..518b22dabef0974c2e7ecb466669925338524059 100644
25+
--- a/src/base_object-inl.h
26+
+++ b/src/base_object-inl.h
27+
@@ -132,14 +132,14 @@ v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const {
28+
29+
template <int Field>
30+
void BaseObject::InternalFieldGet(
31+
- v8::Local<v8::String> property,
32+
+ v8::Local<v8::Name> property,
33+
const v8::PropertyCallbackInfo<v8::Value>& info) {
34+
info.GetReturnValue().Set(
35+
info.This()->GetInternalField(Field).As<v8::Value>());
36+
}
37+
38+
-template <int Field, bool (v8::Value::* typecheck)() const>
39+
-void BaseObject::InternalFieldSet(v8::Local<v8::String> property,
40+
+template <int Field, bool (v8::Value::*typecheck)() const>
41+
+void BaseObject::InternalFieldSet(v8::Local<v8::Name> property,
42+
v8::Local<v8::Value> value,
43+
const v8::PropertyCallbackInfo<void>& info) {
44+
// This could be e.g. value->IsFunction().
45+
diff --git a/src/base_object.h b/src/base_object.h
46+
index 5968694e8393d8434fb2ffee411dfac4c93aff29..5c16d0d1b32e2d056f4fcfa0e01781292932a0fa 100644
47+
--- a/src/base_object.h
48+
+++ b/src/base_object.h
49+
@@ -111,10 +111,10 @@ class BaseObject : public MemoryRetainer {
50+
51+
// Setter/Getter pair for internal fields that can be passed to SetAccessor.
52+
template <int Field>
53+
- static void InternalFieldGet(v8::Local<v8::String> property,
54+
+ static void InternalFieldGet(v8::Local<v8::Name> property,
55+
const v8::PropertyCallbackInfo<v8::Value>& info);
56+
template <int Field, bool (v8::Value::*typecheck)() const>
57+
- static void InternalFieldSet(v8::Local<v8::String> property,
58+
+ static void InternalFieldSet(v8::Local<v8::Name> property,
59+
v8::Local<v8::Value> value,
60+
const v8::PropertyCallbackInfo<void>& info);
61+
62+
diff --git a/src/node_builtins.cc b/src/node_builtins.cc
63+
index 3f82db324d406e342abee23ab0d7f7c9807ff652..91ee97c0d1308e650730b9977facb924ab361bf5 100644
64+
--- a/src/node_builtins.cc
65+
+++ b/src/node_builtins.cc
66+
@@ -11,7 +11,6 @@ namespace node {
67+
namespace builtins {
68+
69+
using v8::Context;
70+
-using v8::DEFAULT;
71+
using v8::EscapableHandleScope;
72+
using v8::Function;
73+
using v8::FunctionCallbackInfo;
74+
@@ -724,7 +723,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
75+
nullptr,
76+
Local<Value>(),
77+
None,
78+
- DEFAULT,
79+
SideEffectType::kHasNoSideEffect);
80+
81+
target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "builtinIds"),
82+
@@ -732,7 +730,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
83+
nullptr,
84+
Local<Value>(),
85+
None,
86+
- DEFAULT,
87+
SideEffectType::kHasNoSideEffect);
88+
89+
target->SetNativeDataProperty(
90+
@@ -741,7 +738,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
91+
nullptr,
92+
Local<Value>(),
93+
None,
94+
- DEFAULT,
95+
SideEffectType::kHasNoSideEffect);
96+
97+
target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "natives"),
98+
@@ -749,7 +745,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
99+
nullptr,
100+
Local<Value>(),
101+
None,
102+
- DEFAULT,
103+
SideEffectType::kHasNoSideEffect);
104+
105+
SetMethod(isolate, target, "getCacheUsage", BuiltinLoader::GetCacheUsage);
106+
diff --git a/src/node_external_reference.h b/src/node_external_reference.h
107+
index 17c0b2d7e1a440b5280a090ad043858faf99f009..24f0c6771720d8a6cdb684a0d562d01564d433f7 100644
108+
--- a/src/node_external_reference.h
109+
+++ b/src/node_external_reference.h
110+
@@ -63,8 +63,6 @@ class ExternalReferenceRegistry {
111+
V(CFunctionWithBool) \
112+
V(const v8::CFunctionInfo*) \
113+
V(v8::FunctionCallback) \
114+
- V(v8::AccessorGetterCallback) \
115+
- V(v8::AccessorSetterCallback) \
116+
V(v8::AccessorNameGetterCallback) \
117+
V(v8::AccessorNameSetterCallback) \
118+
V(v8::GenericNamedPropertyDefinerCallback) \

‎patches/node/src_use_new_v8_api_to_define_stream_accessor.patch‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Igor Sheludko <isheludko@gmail.com>
3+
Date: Tue, 30 Apr 2024 15:22:06 +0200
4+
Subject: src: use new V8 API to define stream accessor
5+
MIME-Version: 1.0
6+
Content-Type: text/plain; charset=UTF-8
7+
Content-Transfer-Encoding: 8bit
8+
9+
Define XxxStream.prototype.onread as an accessor in JavaScript sense.
10+
11+
Previously it was defined via soon-to-be-deprecated
12+
`v8::ObjectTemplate::SetAccessor(..)` which used to call setter even
13+
for property stores via stream object.
14+
15+
The replacement V8 API `v8::ObjectTemplate::SetNativeDataProperty(..)`
16+
defines a properly behaving data property and thus a store to a stream
17+
object will not trigger the "onread" setter callback.
18+
19+
In order to preserve the desired behavior of storing the value in the
20+
receiver's internal field the "onread" property should be defined as
21+
a proper JavaScript accessor.
22+
23+
PR-URL: https://github.com/nodejs/node/pull/53084
24+
Refs: https://github.com/v8/v8/commit/46c241eb99557fe8205acac5c526650c3847d180
25+
Refs: https://github.com/v8/v8/commit/6ec883986bd417e2a42ddb960bd9449deb7e4639
26+
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
27+
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
28+
Reviewed-By: James M Snell <jasnell@gmail.com>
29+
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
30+
(cherry picked from commit bd151552ef35b0eed415eb1c50d30dafd341cee8)
31+
32+
diff --git a/src/base_object-inl.h b/src/base_object-inl.h
33+
index 518b22dabef0974c2e7ecb466669925338524059..61f30b3cfbdb0f3d21fe8e93dc97c55162b69a69 100644
34+
--- a/src/base_object-inl.h
35+
+++ b/src/base_object-inl.h
36+
@@ -132,19 +132,18 @@ v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const {
37+
38+
template <int Field>
39+
void BaseObject::InternalFieldGet(
40+
- v8::Local<v8::Name> property,
41+
- const v8::PropertyCallbackInfo<v8::Value>& info) {
42+
- info.GetReturnValue().Set(
43+
- info.This()->GetInternalField(Field).As<v8::Value>());
44+
+ const v8::FunctionCallbackInfo<v8::Value>& args) {
45+
+ args.GetReturnValue().Set(
46+
+ args.This()->GetInternalField(Field).As<v8::Value>());
47+
}
48+
49+
template <int Field, bool (v8::Value::*typecheck)() const>
50+
-void BaseObject::InternalFieldSet(v8::Local<v8::Name> property,
51+
- v8::Local<v8::Value> value,
52+
- const v8::PropertyCallbackInfo<void>& info) {
53+
+void BaseObject::InternalFieldSet(
54+
+ const v8::FunctionCallbackInfo<v8::Value>& args) {
55+
+ v8::Local<v8::Value> value = args[0];
56+
// This could be e.g. value->IsFunction().
57+
CHECK(((*value)->*typecheck)());
58+
- info.This()->SetInternalField(Field, value);
59+
+ args.This()->SetInternalField(Field, value);
60+
}
61+
62+
bool BaseObject::has_pointer_data() const {
63+
diff --git a/src/base_object.h b/src/base_object.h
64+
index 5c16d0d1b32e2d056f4fcfa0e01781292932a0fa..ce6277dec5a2b9313ecd3699b39ec17585d5adc5 100644
65+
--- a/src/base_object.h
66+
+++ b/src/base_object.h
67+
@@ -111,12 +111,9 @@ class BaseObject : public MemoryRetainer {
68+
69+
// Setter/Getter pair for internal fields that can be passed to SetAccessor.
70+
template <int Field>
71+
- static void InternalFieldGet(v8::Local<v8::Name> property,
72+
- const v8::PropertyCallbackInfo<v8::Value>& info);
73+
+ static void InternalFieldGet(const v8::FunctionCallbackInfo<v8::Value>& args);
74+
template <int Field, bool (v8::Value::*typecheck)() const>
75+
- static void InternalFieldSet(v8::Local<v8::Name> property,
76+
- v8::Local<v8::Value> value,
77+
- const v8::PropertyCallbackInfo<void>& info);
78+
+ static void InternalFieldSet(const v8::FunctionCallbackInfo<v8::Value>& args);
79+
80+
// This is a bit of a hack. See the override in async_wrap.cc for details.
81+
virtual bool IsDoneInitializing() const;
82+
diff --git a/src/stream_base.cc b/src/stream_base.cc
83+
index af714734f49b836de79ebb25e61f1376b757d5ed..a8f9f98d413c8573eb22960a4cefb47408215469 100644
84+
--- a/src/stream_base.cc
85+
+++ b/src/stream_base.cc
86+
@@ -492,6 +492,29 @@ Local<Object> StreamBase::GetObject() {
87+
return GetAsyncWrap()->object();
88+
}
89+
90+
+void StreamBase::AddAccessor(v8::Isolate* isolate,
91+
+ v8::Local<v8::Signature> signature,
92+
+ enum v8::PropertyAttribute attributes,
93+
+ v8::Local<v8::FunctionTemplate> t,
94+
+ JSMethodFunction* getter,
95+
+ JSMethodFunction* setter,
96+
+ v8::Local<v8::String> string) {
97+
+ Local<FunctionTemplate> getter_templ =
98+
+ NewFunctionTemplate(isolate,
99+
+ getter,
100+
+ signature,
101+
+ ConstructorBehavior::kThrow,
102+
+ SideEffectType::kHasNoSideEffect);
103+
+ Local<FunctionTemplate> setter_templ =
104+
+ NewFunctionTemplate(isolate,
105+
+ setter,
106+
+ signature,
107+
+ ConstructorBehavior::kThrow,
108+
+ SideEffectType::kHasSideEffect);
109+
+ t->PrototypeTemplate()->SetAccessorProperty(
110+
+ string, getter_templ, setter_templ, attributes);
111+
+}
112+
+
113+
void StreamBase::AddMethod(Isolate* isolate,
114+
Local<Signature> signature,
115+
enum PropertyAttribute attributes,
116+
@@ -561,11 +584,14 @@ void StreamBase::AddMethods(IsolateData* isolate_data,
117+
JSMethod<&StreamBase::WriteString<LATIN1>>);
118+
t->PrototypeTemplate()->Set(FIXED_ONE_BYTE_STRING(isolate, "isStreamBase"),
119+
True(isolate));
120+
- t->PrototypeTemplate()->SetAccessor(
121+
- FIXED_ONE_BYTE_STRING(isolate, "onread"),
122+
- BaseObject::InternalFieldGet<StreamBase::kOnReadFunctionField>,
123+
- BaseObject::InternalFieldSet<StreamBase::kOnReadFunctionField,
124+
- &Value::IsFunction>);
125+
+ AddAccessor(isolate,
126+
+ sig,
127+
+ static_cast<PropertyAttribute>(DontDelete | DontEnum),
128+
+ t,
129+
+ BaseObject::InternalFieldGet<StreamBase::kOnReadFunctionField>,
130+
+ BaseObject::InternalFieldSet<StreamBase::kOnReadFunctionField,
131+
+ &Value::IsFunction>,
132+
+ FIXED_ONE_BYTE_STRING(isolate, "onread"));
133+
}
134+
135+
void StreamBase::RegisterExternalReferences(
136+
diff --git a/src/stream_base.h b/src/stream_base.h
137+
index 62a8928e144ad6aa67eeccdbc46d44da22887fb8..ccbd769ceaf3c1e024defb3104e601d037c98b6a 100644
138+
--- a/src/stream_base.h
139+
+++ b/src/stream_base.h
140+
@@ -413,6 +413,13 @@ class StreamBase : public StreamResource {
141+
EmitToJSStreamListener default_listener_;
142+
143+
void SetWriteResult(const StreamWriteResult& res);
144+
+ static void AddAccessor(v8::Isolate* isolate,
145+
+ v8::Local<v8::Signature> sig,
146+
+ enum v8::PropertyAttribute attributes,
147+
+ v8::Local<v8::FunctionTemplate> t,
148+
+ JSMethodFunction* getter,
149+
+ JSMethodFunction* setter,
150+
+ v8::Local<v8::String> str);
151+
static void AddMethod(v8::Isolate* isolate,
152+
v8::Local<v8::Signature> sig,
153+
enum v8::PropertyAttribute attributes,

‎patches/v8/.patches‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
chore_allow_customizing_microtask_policy_per_context.patch
22
deps_add_v8_object_setinternalfieldfornodecore.patch
33
revert_heap_add_checks_position_info.patch
4-
revert_api_fully_deprecated_wrapperdescriptor.patch
5-
revert_api_cleanup_remove_setaccessor_and_setnativedataproperty.patch

0 commit comments

Comments
 (0)